Make WordPress Core

Opened 6 years ago

Closed 5 years ago

#44303 closed defect (bug) (fixed)

`media_handle_sideload()` may unexpectedly return 0 on error

Reported by: jirihon's profile jirihon Owned by: joemcgill's profile joemcgill
Milestone: 5.1 Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch commit
Focuses: Cc:


This is closely related to #41726. The documentation of media_handle_sideload() (same as for media_handle_upload()) states that it will always return the attachment ID on success, or a WP_Error object on failure. However, it currently may return 0 as well if something fails with the attachment post insertion. That is because wp_insert_attachment() and wp_insert_post() do not return WP_Error objects by default, and media_handle_sideload() for some reason does not change this like it should.

This requires a simple fix: call wp_insert_attachment() with the fourth parameter set to true as it was recently fixed for the media_handle_upload() in #41726.

Attachments (1)

44303.diff (557 bytes) - added by subrataemfluence 6 years ago.
Proposed patch

Download all attachments as: .zip

Change History (6)

6 years ago

Proposed patch

#1 @subrataemfluence
6 years ago

  • Keywords has-patch added
  • Version set to trunk

#2 @joemcgill
6 years ago

  • Keywords commit added
  • Owner set to joemcgill
  • Status changed from new to accepted

Thanks for the writeup, @jirihon and for the patch, @subrataemfluence. It looks good to me.

One quick note on ticket process, the version property on a ticket is meant to refer to the version of WordPress where a bug was introduced and/or the earliest version where you can reproduce this bug. I don't really see a need to go back through WP history to identify that moment in this instance.


#3 @joemcgill
5 years ago

  • Milestone changed from Awaiting Review to 5.1

#4 @pento
5 years ago

  • Version trunk deleted

#5 @pento
5 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 44634:

Media: media_handle_sideload() expectes wp_insert_attachment() to return a WP_Error.

For wp_insert_attachment() to do that, we need to be setting the $wp_error parameter to true.

Props subrataemfluence, jirihon.
Fixes #44303.

Note: See TracTickets for help on using tickets.