Opened 13 years ago
Closed 13 years ago
#19914 closed defect (bug) (fixed)
wp-app.php assumes elements exist
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 3.4 | Priority: | normal |
Severity: | normal | Version: | 2.3 |
Component: | AtomPub | Keywords: | has-patch |
Focuses: | Cc: |
Description
First bug found by Gorilla, yay!
So, submitting a very sparse entry gives errors.
Submitted document:
<?xml version="1.0" encoding="utf-8"?> <entry> <title>From a Gorilla!</title> <id>tag:ryanmccue.info,2012:016540021841</id> <summary type="text">Summary from a <strong>&lt;Gorilla&gt;</strong> at 2012-01-28T23:30:20+10:00</summary> </entry>
Gives the following errors:
Notice: Trying to get property of non-object in wp-app.php on line 403 Warning: Invalid argument supplied for foreach() in wp-app.php on line 403 Notice: Trying to get property of non-object in wp-app.php on line 426 Notice: Trying to get property of non-object in wp-app.php on line 427 Notice: Trying to get property of non-object in wp-app.php on line 428 Notice: Trying to get property of non-object in wp-app.php on line 429
Attachments (3)
Change History (18)
#2
@
13 years ago
Some extra ones from after creation:
Notice: Undefined property: AtomEntry::$content in wp-app.php on line 427 Notice: Undefined property: AtomEntry::$published in wp-app.php on line 429
#4
follow-up:
↓ 5
@
13 years ago
Oh, another good idea would be to do error_reporting(0)
to avoid outputting any errors at all, regardless of WP_DEBUG, as it's not a HTML endpoint.
#5
in reply to:
↑ 4
;
follow-up:
↓ 6
@
13 years ago
- Cc kpayne@… added
- Keywords has-patch added
Hi rmccue,
I've submitted a patch for the issues you've pointed out. I was unable to recreate the problems you originally reported using Gorilla. Can you please post the instructions on how to recreate? I'm using this command:
gorilla --uri=http://localhost/wp-app.php --user=admin --pass=******** EntryPostsTest
Replying to rmccue:
Oh, another good idea would be to do
error_reporting(0)
to avoid outputting any errors at all, regardless of WP_DEBUG, as it's not a HTML endpoint.
Perhaps it would be better to disable error output and send errors to an error log.
#6
in reply to:
↑ 5
@
13 years ago
Replying to kurtpayne:
I've submitted a patch for the issues you've pointed out. I was unable to recreate the problems you originally reported using Gorilla. Can you please post the instructions on how to recreate?
Apologies Kurt, I didn't realise my version of PHPUnit was so out-of-date. If you pull again now, it should spit out the errors properly. (If you're still seeing nothing, try -v
as well to enable debug mode.
Perhaps it would be better to disable error output and send errors to an error log.
Agreed.
#7
@
13 years ago
By the way, patch looks good, but I think isset( $entry->categories )
should probably be !empty( $entry->categories )
instead. For the strings, I think that should check $var[1]
rather than just $var
.
#8
follow-ups:
↓ 9
↓ 13
@
13 years ago
- Keywords needs-patch removed
Replying to rmccue:
I think
isset( $entry->categories )
should probably be!empty( $entry->categories )
instead.
Agreed.
For the strings, I think that should check
$var[1]
rather than just$var
.
Disagree. We should definitely check for $var
before trying to access any sub elements. Switched these to
empty()` calls and reversed the order of the ternary to be in-line with the coding standards.
Perhaps it would be better to disable error output and send errors to an error log.
Agreed.
This is already possible using wp-config.php:
define('WP_DEBUG_LOG', true); define('WP_DEBUG_DISPLAY', false);
Also, Gorilla worked beautifully this time!
#9
in reply to:
↑ 8
@
13 years ago
Replying to kurtpayne:
Disagree. We should definitely check for
$var
before trying to access `any sub elements.
I meant to check both, sorry. Otherwise $var
could be an empty array and then cause it to error.
Switched these to
empty()
calls and reversed the order of the ternary to be in-line with the coding standards.
I think you actually want isset there, since it could be an empty string (which would give the same result of course, but less efficiently).
This is already possible using wp-config.php:
Not separately for APP/XMLRPC endpoints though. Whether that actually warrants a patch is a different story though.
Also, Gorilla worked beautifully this time!
Awesome, glad to hear it.
#10
@
13 years ago
I think 19914.3.patch combines all of the error checking we've discussed. Gorilla passes with this patch, and there are no errors in the logs.
#12
@
13 years ago
Looks good to me. (Minor style nitpick for whoever commits: =(string)
should be = (string)
on line 441. No real need for another patch though, IMHO.)
(To be fair though, some of those are caused by my missing XMLNS, but the errors are still valid. wp-app.php shouldn't have PHP errors with invalid input)