If you specify a nonexistent/unreadable Xero cert or key file in xero.module's admin interface, the user gets a WSOD when attempting to post hours (because the instantiation of the Xero class returns false, but xero.module doesn't detect it).

The attached patch validates that the cert and key files are readable.

CommentFileSizeAuthor
#8 959240-combined-fork-admin-3.patch5.54 KBShiny
#6 959240-combined-fork-admin-2.patch5.16 KBmradcliffe
#5 959240-combined-fork-admin.patch3.6 KBmradcliffe
#3 xero-object-validation-959240.patch5.13 KBmradcliffe
#3 php-xero.patch429 bytesmradcliffe
xero-validate.patch1.22 KBsmokris

Comments

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented
Assigned: Unassigned » mradcliffe
Status: Needs review » Needs work

Hmm... xero_query should return NULL if it cannot create a proper $xero object. My error checking in xero_query is bad. $xero is always going to be an object, but I should really be checking the oauth variables.

The private variables consumer, token, and signature_method should be objects and the private format variable should be a string. If it cannot create a valid oauth connection those will be NULL.

This also affects the simpletest connection test.

I do think element validate would be good, but it won't fix the entire issue.

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented

Currently I can't properly check if the $xero object is valid because its member variables are private. I've written a patch for PHP-Xero, which this will depend on.

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented
StatusFileSize
new php-xero.patch429 bytes
new xero-object-validation-959240.patch5.13 KB

Forgot to attach some patches... Won't work until php-xero is patched.

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented

Closed #1108706: validate files on admin form submit as a duplicate.

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented
StatusFileSize
new 959240-combined-fork-admin.patch3.6 KB

Combining patches from local commits I just made:

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented
Status: Needs work » Needs review
StatusFileSize
new 959240-combined-fork-admin-2.patch5.16 KB

Another one to adjust install/status requirements.

Shiny’s picture

Shiny commented
Status: Needs review » Needs work

Combined patch results in:
Parse error: syntax error, unexpected ';' in /home/brenda/workspace/drupal/modules/xero/xero.install on line 47

Shiny’s picture

Shiny commented
Status: Needs work » Needs review
StatusFileSize
new 959240-combined-fork-admin-3.patch5.54 KB

re-rolled with that parse error in .install gone

Shiny’s picture

Shiny commented

The php-xero.patch includes this line:

+ if ( !is_object($this->consumer) || !is_object($this->token) || !is_object($thi

the end of the line is missing.

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented
Status: Needs review » Fixed

Ugh, sorry for the delay.

http://drupalcode.org/project/xero.git/commit/4d54ab2

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented

Okay, fixed and tested. That was a quick version :(

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.