#61681 closed defect (bug) (fixed)
PHP Fatal error: Uncaught Error: Object of class WP_Comment could not be converted to string
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Milestone: | 6.6.1 | Priority: | high |
| Severity: | normal | Version: | 6.6 |
| Component: | Comments | Keywords: | has-patch has-unit-tests commit dev-reviewed fixed-major |
| Focuses: | Cc: |
Description
Error message is: PHP Fatal error: Uncaught Error: Object of class WP_Comment could not be converted to string in /var/web/site/public_html/wp-includes/comment-template.php:27
The problem appears to be that in some cases, code in comment-template.php is calling get_comment_author() in comment-template.php which expects an optional parameter that is used as the comment_ID but is being passed a WP_Comment object instead at times. Discovered the issue when trying to edit a Divi template and the builder hung, and found the errors in the log. I do have a proposed patch attached.
Attachments (1)
Change History (25)
#2
@
18 months ago
Oh wait, what if an instance of WP_Comment is instantiated before it has an assigned ID?
The ID is auto-assigned in the database. So if a new comment is instantiated but not yet saved in the database, the comment_ID property should be empty.
Testing this theory and writing tests to show it.
This ticket was mentioned in PR #7053 on WordPress/wordpress-develop by @hellofromTonya.
18 months ago
#3
- Keywords has-patch has-unit-tests added
If an instance of WP_Comment is passed to get_comment_author() without an ID (either before the comment is saved in the database or somehow the ID is reset), a fatal error can happen.
Trac ticket: https://core.trac.wordpress.org/ticket/61681
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
18 months ago
#5
follow-up:
↓ 6
@
18 months ago
Patch: https://github.com/WordPress/wordpress-develop/pull/7053
This patch:
- First adds tests to show the fatal error happening in
get_comment_author()when the incomingWP_Commentobject does not have a comment ID (for whatever reason) or that property is empty. - Second adds the fix, which (a) defensively guards the string type casting
(string) $comment_idfor when$comment_idis scalar and (b) if neither path is true, sets it an empty string as a default.
The $comment_id is not used for getting the comment author; rather it was added to pass to the filter.
This patch fixes the fatal error. But what is passed to the filter would no longer be an object. I don't think it's a BC break in that the $comment_id passing in the filter is documented to a string data type, not an instance of WP_Comment.
@ambrosiawt are you able to test the patch to see if resolves the issue?
#6
in reply to:
↑ 5
@
18 months ago
Replying to hellofromTonya:
Patch: https://github.com/WordPress/wordpress-develop/pull/7053
...
@ambrosiawt are you able to test the patch to see if resolves the issue?
Yes, I will test it shortly. Thanks!
#7
@
18 months ago
For what it's worth, I did add some logging before submitting the ticket to validate that sometimes there is an object present, and in those situations you are correct, it did have an empty comment_ID in the object.
#9
@
18 months ago
- Priority changed from normal to high
That's awesome @ambrosiawt. Thank you for confirming.
I'm elevating the priority on this ticket to high for better tracking as it's one of the drivers for 6.6.1.
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
18 months ago
18 months ago
#11
Since I needed to search for them, noting that the test failures from before the second commit are in: https://github.com/WordPress/wordpress-develop/actions/runs/9980970593
#12
@
18 months ago
- Keywords commit added
- Owner set to hellofromTonya
- Status changed from new to reviewing
Patch: https://github.com/WordPress/wordpress-develop/pull/7053
@jorbin reviewed and approved.
Marking for commit.
@hellofromTonya commented on PR #7053:
18 months ago
#15
Committed via https://core.trac.wordpress.org/changeset/58755. Thank you everyone 🎉
#16
@
18 months ago
- Keywords dev-feedback added
- Resolution fixed deleted
- Status changed from closed to reopened
Hello @ambrosiawt,
Welcome to WordPress Core Trac. Thank you for reporting this issue.
[58335] introduced the
(string)type casting change. It triggers when theWP_Commentinstance'scomment_IDis not empty (see the code here).function get_comment_author( $comment_id = 0 ) { $comment = get_comment( $comment_id ); $comment_id = ! empty( $comment->comment_ID ) ? $comment->comment_ID : (string) $comment_id;At first glance:
comment_IDproperty isn't initialized (not empty) at this point.$comment_idis an instance ofWP_Comment, passing it toget_comment()should also return the object.?branch.So how and why is it falling into the
elsebranch (i.e. running this(string) $comment_id)?Hmm.
Thinking the place to start is in understanding how, when, and why an object passed into
get_comment()may not have itscomment_IDassigned / initialized.Once that's understood, then the root cause may be elsewhere.
I'm pulling this into 6.6.1 milestone for awareness and investigation.