Schema error breaks XMLRPC image uploading

[Otto42]

Issue is described in greater detail here:
​http://ardentdev.com/fix-for-wordpress-xmlrpc-500-internal-server-error/

Short version: Uploading an image or other attachment via XMLRPC tries to set the post_parent of the upload to -1, and then changes the value to the correct ID after the post is created and the ID is known.

In WordPress 2.8 and up, this fails, because the schema changed to define the post_parent as unsigned, so -1 becomes invalid. This disables the capability for image attachments to work via XMLRPC uploads.

Fix: Either change the way uploads work (use a 0 for post_parent perhaps?) or change the schema back to not being unsigned on post_parent.

[Otto42]

Change that broke this was here: [10852] , for ticket #9422.

[Denis-de-Bernardy]

Why is it not trying to insert it using a post_parent of 0 instead?

[Otto42]

Replying to Denis-de-Bernardy:

Why is it not trying to insert it using a post_parent of 0 instead?

That was my suggestion as well. Seems to me that zero would work okay.

You'd have to change xmlrpc.php. At a glance, I'd say to alter mw_newMediaObject and attach_uploads to make them both use zero instead of -1. Needs testing to be sure it doesn't interfere with anything else.

[Otto42]

Alternatively, if zero does turn out to be a problem, we can just be sooper clever about it and set it to the max bigint (2^{64, or 18446744073709551615).}

If somebody can get that many posts, they deserve to have their database kersplode. ;)

[Denis-de-Bernardy]

It shouldn't be any problem. It might be the software that's using the XML-RPC interface. But still, we could catch a negative parent and turn that into a 0 instead...

[Otto42]

Replying to Denis-de-Bernardy:

It shouldn't be any problem. It might be the software that's using the XML-RPC interface. But still, we could catch a negative parent and turn that into a 0 instead...

Oh, no, the client software is not sending it. It's in the xmlrpc.php file, definitely.

Line 2877 is where it gets set to -1 when the image is uploaded.

Line 2272 is where it searches for the -1 entries and corrects their parent id's (after the actual post comes in and gets created, since uploads happen first).

My worry is that we're using zero in that field somewhere else for something else and using that could cause interference. But I don't think that will be an issue, just that it's possible.

[Denis-de-Bernardy]

you're kidding me. you're meaning there's potential for akward behavior when race conditions occur?

[Denis-de-Bernardy]

when do we switch to using UUIDs all over for this kind of stuff?

[Otto42]

Replying to Denis-de-Bernardy:

you're kidding me. you're meaning there's potential for akward behavior when race conditions occur?

No need for that now. :-P

Replying to Denis-de-Bernardy:

when do we switch to using UUIDs all over for this kind of stuff?

Agreed, proper UUIDs would eliminate this issue, more or less. However, few problems with using them for IDs.

• People are already annoyed that their post_id's are no longer sequential. No, I don't get it either.
• Lots and lots and lots of code that uses ints for ID's. All of that going to change?
• Database bloat. UUIDs are twice the size of bigints.

Plus you're always going to get the whole "where's the advantages" argument, of which I can only think of one, which is that UUID identifiers on everything makes it a lot easier to merge/split databases.

Right now, UUIDs have a lot of downsides. Unless we made them entirely internal (no exposure to the outside world) and left some of the int id remnants for people to look at, it would be awkward.

[Denis-de-Bernardy]

oh, I was meaning something similar to what's discussed here: #9471

[josephscott]

I've been trying to reproduce this but so far haven't had any luck. Given the conditions though it seems like this would be a problem, so it's worth looking at changes to address it. Perhaps there's a strict MySQL setting that I don't have enabled, or a difference between MySQL versions or table types.

I'm going to dig through the code path more and run a few more tests. Switching to a post_id of zero sounds like it would be safe, so I'll focus on testing that.

[josephscott]

Another confirmation on this issue: ​http://iphone.forums.wordpress.org/topic/attachments-post_parent?replies=7#post-873

[c0nstruct]

Patch used to fix my xml-rpc.php

[c0nstruct]

Replying to josephscott:

Another confirmation on this issue: ​http://iphone.forums.wordpress.org/topic/attachments-post_parent?replies=7#post-873

If it's of any use, I am having trouble with the same issue. I have changed the xml-rpc.php file in my dev instance to use '0' instead of '-1' for the orphan parent post_id and it now works just like expected.

I am attaching the patch I have applied to xml-rpc.php.

[leenewton]

I had put a ticket in over on the ​wpmu side after I'd debugged this on our setup. Should have looked here first :) Anyway regarding setting the post_parent to 0 rather than -1; the problem I see here is wp installations with a lot of activity most likely have very many unattached images. The default behaviour is to query for all post_parents = -1 where the guid matches what's in the post_content for the post added after the image upload. If this was changed to 0, a great many posts would potentially be looped through.

[brianlayman]

I decided to get some actual numbers from 10 of b5's blogs to see how many string compares attach_uploads() would have to do with the current patch via: "SELECT count(*) FROM wp_#_posts WHERE post_parent = '0' AND post_type = 'attachment'"

The counts I came up with were 6758, 5027, 671, 1200, 3684, 4377, 3095, 21776, 15885, & 9846.

It looks like many of our individual entertainment blogs will be up in the 5k-10k range. So looping through ALL of those records, for every new attachment when publishing a 5 attachment post, would probably produce noticeable lag.

Heck, for that one blog it would mean 65,328 if statements before the loop was exited, for just one attachment.

Plus it would be better if we didn't make our db throw around that many records around the network when we know the overwhelming majority don't match. If we must keep this index unsigned, we could consider throwing an index on the quid column and allowing the comparison with $post_content to occur there. I'd have to do some testing to see which method would be more costly.

[Otto42]

It wouldn't loop through them every time. It'd attach them all to the first post made via XML-RPC. Then they wouldn't have a post_parent of zero any more.

Clearly, using zero instead of -1 isn't a viable option, because having attachment posts with a zero parent is possible using the normal media uploader, which creates "unattached" attachment posts.

[leenewton]

Replying to Otto42:

It wouldn't loop through them every time. It'd attach them all to the first post made via XML-RPC. Then they wouldn't have a post_parent of zero any more.

Not to nitpick but that's not how I read the xmlrpc, mw_newPost calls attach_uploads which will only change the post_parent if( strpos( $post_content, $file->guid ) !== false ) if an img src match isn't found while processing the post uploaded via xmlrpc then nothing happens and all those images will be involved in subsequent post creation. So while this will capture and reassign the image you uploaded, all other post_parent = 0 rows will remain untouched.

Clearly, using zero instead of -1 isn't a viable option, because having attachment posts with a zero parent is possible using the normal media uploader, which creates "unattached" attachment posts.

What he said.

[Otto42]

Replying to leenewton:

Not to nitpick but that's not how I read the xmlrpc, mw_newPost calls attach_uploads which will only change the post_parent if( strpos( $post_content, $file->guid ) !== false ) if an img src match isn't found while processing the post uploaded via xmlrpc then nothing happens and all those images will be involved in subsequent post creation. So while this will capture and reassign the image you uploaded, all other post_parent = 0 rows will remain untouched.

Ahh, yes. Missed that bit. Fair enough.

Still would be a problem of speed if you've got a few thousand zero'd attachments.

[ryan]

Introduce an "unattached" post_status?

[Otto42]

Replying to ryan:

Introduce an "unattached" post_status?

If you do that, and it doesn't get changed back to attachment (when the post is added), then the attachment won't show up in the media section. Unless you modify all that jazz as well.

[ryan]

We could modify the attachment query to look for inherit|unattached. Since there is no parent to inherit from, unattached kinda makes sense.

[ryan]

Go with 0 for now, see how it goes, mess with post_status if need be?

[Otto42]

Zero will fix the problem for now, even if it is not optimal for sites with lots of unattached images.

I say to do zero now simply to get it working *at all*, work on a better way for a long term fix.

[ryan]

Refreshed patch

[ryan]

Looks like the detached check in upload.php and elsewhere looks for < 1 so we're still good there.

[ryan]

(In [11969]) Use a post_parent of 0 instead of -1 to indicate unattached posts. Props c0nstruct. see #10521