Unnecessary post type check in wp_get_attachment_url

[jfarthing84]

Not sure why this needs to be done. You wouldn't be calling this function on a post if it wasn't an attachment. If you are, then you must have a good reason to be doing so, like I do.

Remove post type check from wp_get_attachment_url.

If you are, then you must have a good reason to be doing so, like I do.

And the reason is?

Well, I have a custom post type for user uploads. This post type should be treated exactly like an attachment. As far as I can tell so far, wp_get_attachment_url is the only "media" function that does this check, and it hinders other functions from working on a post type other than "attachment" (like image_downsize).

Why don't I just use the attachment post type? Well, there's a few reasons for that. The main reason is because you can't assign any post status to an attachment. WordPress forces it to be "inherit" or "private".

Any reason we can't get this done?

Replying to jfarthing84:

Well, I have a custom post type for user uploads. This post type should be treated exactly like an attachment. As far as I can tell so far, wp_get_attachment_url is the only "media" function that does this check, and it hinders other functions from working on a post type other than "attachment" (like image_downsize).

Attachments are designed to be a special type in WordPress. We do 'attachment' checks frequently (especially on endpoints — look at some recent functions added to ajax-actions.php) to avoid (in part) potential information disclosure based on someone requesting an "attachment ID" of a non-attachment.

Why don't I just use the attachment post type? Well, there's a few reasons for that. The main reason is because you can't assign any post status to an attachment. WordPress forces it to be "inherit" or "private".

Any other reasons? That's not true, it can also be public, and it can be done programmatically otherwise. User uploads should probably go through the attachment post type, or they should be a post type that stores your information and then the attachment post type holds the information for you.

I'm not saying this itself is a bad idea. I don't think it is. But if we do this, we are essentially saying we will always support attachment-specific functions to work on non-attachments, and that is just not something I think we should be guaranteeing. It requires us to support something we don't really care to support. We'll either find a situation where we can't, or we'll break the paradigm accidentally later on. So it's not really fair to either of us.

I just don't like the idea of needing two rows in the database per each upload. That's really not needed. Of course, I could also just write my own versions of the WP attachment functions, but that is not really needed also. The image_downsize function provides a handy filter before it runs, allowing plugins to short-circuit it and return their own image. Perhaps something like this can be added there instead?

That's not true, it can also be public, and it can be done programmatically otherwise.

So, what about this check in wp_insert_attachment?

if ( ! in_array( $post_status, array( 'inherit', 'private' ) ) )
	$post_status = 'inherit';