Make WordPress Core

Opened 13 years ago

Closed 12 years ago

#20660 closed feature request (invalid)

wp_get_attachment_url() ignores filter for unrecognized IDs

Reported by: roamfree's profile RoamFree Owned by:
Milestone: Priority: normal
Severity: minor Version: 2.1
Component: General Keywords:
Focuses: Cc:

Description

wp_get_attachment_url() fails to invoke its filters when it encounters an unrecognized post ID. It would be helpful to allow a filter to deal with the situation.

The plugin NextGEN Gallery uses image IDs like "ngg-1", while WP's image IDs are integers. A filter could deal with the situation.

wp_get_attachment_url() begins like this:

$post_id = (int) $post_id;
if ( !$post =& get_post( $post_id ) )
  return false;

Obviously, a nonstandard post ID will ignore the filters. If the filters were invoked, nonstandard IDs can be dealt with as desired by themes or plugins.

The filter should be given a "false" value instead of $url, and the original $post_id value (before forcing to (int)). If existing filters are already dealing with null or empty URLs, they can handle this situation.

Suggestion:

$post_id_original = $post_id;
$post_id = (int) $post_id;
if ( !$post =& get_post( $post_id ) )
  $url = apply_filters( 'wp_get_attachment_url', false, $post_id_original );

  if ( empty( $url ) )
     return false;

  return $url;

Change History (4)

#1 @RoamFree
13 years ago

Of course, modify the last part above to have {braces} around the section after the "if".

#2 @c3mdigital
12 years ago

  • Resolution set to invalid
  • Status changed from new to closed
  • Version changed from 3.3.2 to 2.1

Anything that calls wp_get_attachment_url() should make sure they are sending an actual post id or do a check on the return value to make sure it's not false. We shouldn't be supporting bad coding habits in core.

#3 @markoheijnen
12 years ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

I reopening this for second thoughts. The use case described is maybe not the best one. I had a use case where core uses this function to request the attachment url for a CPT I created. This because I was combining it in the new media dialog. In this case there was another filter I could use but I still think this was the best location to change it.

Also I do believe when you filter on the output that you should try to do that always. Even when the output is false.

#4 @nacin
12 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from reopened to closed

It is standard for us to bail early, sans filter, when a function expects a post and the post does not exist.

The filter would receive $post which would be invalid, or it would not be passed $post. Either way, all existing usage of the original filter by plugins could possibly break. The alternative would be for a new, specific filter to be added — solely to be issued when something unexpected occurred.

Your CPT would have a valid ID. But unrecognized post IDs are invalid. One should be lucky we are not issuing a WP_Error or throwing an exception.

Note: See TracTickets for help on using tickets.