Attachments inserted with NULL value for GUID

[docfish]

This is the error I get when trying to post via xml-rpc interface:

Empty delimiter in .../wp-includes/class-wp-xmlrpc-server.php on line 2469 Warning: Cannot modify header information - headers already sent by (output started at .../wp-includes/class-wp-xmlrpc-server.php:2469) in .../webraces/wp-includes/class-IXR.php on line 471

The line 2469 of class-wp-xmlrpc-server.php looks like:

if ( strpos( $post_content, $file->guid ) !== false )

Apparently $file->guid has invalid value as this is fixed by inserting

if($file->guid && !($file->guid == NULL))

right above the line in question. See also ​http://wordpress.org/support/topic/windows-live-writer-no-go-with-wp-312

[ericmann]

I'd want to look at that a bit deeper. Your fix would remove the error, but wouldn't fix the underlying problem.

In this case, $file is an element returned directly from $wpdb->get_results(). If it contains a null value, then something else is going wrong when attachments are being inserted into the database. I'd want to see what exactly get_results() is returning when this error happens and fix the cause of that rather than apply a bandaid.

$file->guid shouldn't be empty ...

[solarissmoke]

Can you give us a sample XML-RPC request that triggers this error? I can't reproduce it and like ericmann says, $file->guid shouldn't be empty.

[jcox]

I encountered this error after using wp_insert_attachment() without including a guid value, because guid is not listed as a required key/value for the $attachment array in the function reference at ​http://codex.wordpress.org/Function_Reference/wp_insert_attachment.

[ericmann]

True, guid is not a required key/value for that $attachment array in that function. And that is where the problem is, not in XML-RPC.

By default, the GUID is set to if nothing is passed in the $attachment array. Unfortunately, that's one of the only fields not sanitized/reset by WordPress before actually inserting the attachment.

In my opinion, this is a defect. $file->guid shouldn't be empty. If no guid is passed to wp_insert_attachment(), then a guid should be created along the same lines as for wp_insert_post().

I'm attaching a patch that updates the attachment to set the GUID if it wasn't originally sent in the $attachment array.

This should fix the real problem of a null GUID for attachments, which will also fix the observed issue with XML-RPC.

[ericmann]

Automatically inserts a GUID if none was passed to wp_insert_attachment()

[iandunn]

Thanks for writing the patch Eric. I agree that a GUID should be generated if one isn't passed to wp_insert_attachment(), but that doesn't solve the problem reported here by docfish ​and also on the Android app forums.

This line in attach_uploads() in wp-includes/class-wp-xmlrpc-server.php will still generate a PHP warning when $file->guid is empty because of the invalid paramater.

if ( strpos( $post_content, $file->guid ) !== false )

That warning causes some XMLRPC clients to freak out and display an error to the user, even though the post content and upload are successfully posted.

So, before calling strpos(), attach_uploads() also needs to check for a valid GUID, and if necessary, generate one.

Maybe the code that generates the GUID in both functions (and possibly others) should be refactored into its own function so that it's DRY.

[ericmann]

Patch refresh, and also check that $file->guid is actually set before using it.

[ericmann]

New patch refreshes the existing one (setting the GUID if it wasn't previously set). But it also throws an isset() check on $file->guid inside attach_uploads() to fix the existing errors.

[julien.stlaurent]

The attach_uploads function also makes a similar query, at line 3955, and would require the same 'isset' check.

[markoheijnen]

Moving to 3.6 since this issue is causing apps to stop working. the isset wouldn't work. It should just be ! $file->guid ?

Also not sure why wp-includes/post.php should be fixed.

[markoheijnen]

never mind on the post.php fix. Understand it now after reading comment 4.

[markoheijnen]

In 1278/tests:

Add test to check if guid isn't empty when not provided. See #18310

[markoheijnen]

No isset but empty check. Also change arrray to array

[markoheijnen]

It's ready to be committed. Only need a pair of eyes on setting guid when it was empty.

[nacin]

If no guid is passed to wp_insert_attachment(), then a guid should be created along the same lines as for wp_insert_post().

Sure, I guess. But that's not really part of the bug fix here. That's a separate discussion (which probably involves #21963).

[nacin]

In 24639:

Avoid notice in XML-RPC when attaching uploads, when attachments do not have a guid in the DB. props ericmann, markoheijnen. fixes #18310.

[nacin]

The notice is for an empty delimiter. isset( $guid ) is therefore not sufficient; if it comes back as an empty string or the like, the strpos() call will fail and emit a warning. Note, #23811 more or less fixes this anyway.

I'm going to suggest a new ticket for wp_insert_attachment()... I'm leaving a comment in #21963 as well.

[nacin]

Code in wp_insert_attachment() should probably model wp_insert_post() here:

if ( !$update && '' == $current_guid )
	$wpdb->update( $wpdb->posts, array( 'guid' => get_permalink( $post_ID ) ), $where );

The ! $update is key. Anyway, #21963 or a new ticket.