Make WordPress Core

Opened 10 years ago

Closed 10 years ago

#27802 closed defect (bug) (fixed)

Notice: Illegal member variable name in media.php on line 2418 due to invalid attachment

Reported by: bobbingwide's profile bobbingwide Owned by: nacin's profile nacin
Milestone: 3.9 Priority: normal
Severity: normal Version: 3.9
Component: Media Keywords: has-patch
Focuses: Cc:

Description

WordPress 3.9-RC1

Attachment has a status of 'private', post_content = upload file name and post_mime_type set to ( blank ).

Each time I attempt to edit a post I see this notify message.
Similar explanation to #19706.

The problem was created when Installing a theme from a ZIP file.
You have to attempt to upload the theme twice.
Resulting in a Destination folder already exists on the second upload.

I am able to reproduce the problem.

Attachments (3)

27802.diff (586 bytes) - added by jeremyfelt 10 years ago.
27802.2.diff (641 bytes) - added by jeremyfelt 10 years ago.
27802.3.diff (541 bytes) - added by bobbingwide 10 years ago.
Cast the result from wp_count_attachments() to an array

Download all attachments as: .zip

Change History (15)

This ticket was mentioned in IRC in #wordpress-dev by bobbingwide. View the logs.


10 years ago

#2 @bobbingwide
10 years ago

Workaround to avoid the notify message is to set the post_mime_type field to a non-blank value.

#3 @bobbingwide
10 years ago

Problem also applies to plugin uploads.

#4 @bobbingwide
10 years ago

In class-wp-upgrader.php wp_handle_upload() is not setting $filetype?

#5 @bobbingwide
10 years ago

The fact that the post still exists is not new. But the logic isn't there to ignore it.

#6 @jeremyfelt
10 years ago

  • Component changed from Upgrade/Install to Media
  • Milestone changed from Awaiting Review to 3.9

Confirmed.

  1. Upload theme zip file.
  2. Upload same theme zip file.
  3. Click New Post

Introduced in [27788]

@jeremyfelt
10 years ago

This ticket was mentioned in IRC in #wordpress-dev by jeremyfelt. View the logs.


10 years ago

@jeremyfelt
10 years ago

#8 @jeremyfelt
10 years ago

When a theme upload fails due to a duplicate, its mime type is not stored in the database. wp_count_attachments() looks for all attachments and groups them using mime type as a key. When a mime type is not available, this key is empty.

27802.2.diff checks for those empty requests and stores them under the 'unknown' key. This preserves a total count and avoids the issue of an unknown key. It also introduces 'unknown', which may be weird.

Another option is to just skip that attachment all together when counting, which 27802.diff does. This could (?) cause some issues with counts being off, though it seems like the original issue would still be present.

#9 @bobbingwide
10 years ago

  • Component changed from Media to Upgrade/Install
  • Keywords has-patch added

The fact that the post still exists is not new. But the logic isn't there to ignore it.
A simpler fix, duplicating the method used in class-wp-media-list-table.php, is to cast the object back to an array.

 $counts = (array) wp_count_attachments();

@bobbingwide
10 years ago

Cast the result from wp_count_attachments() to an array

#10 @nacin
10 years ago

This is fun. So:

  • Uploading a theme (and probably a plugin) doesn't actually set a mime type. This appears broken in general, not just when you upload a theme a second time. I only needed to upload it once to trigger it.
  • wp_count_attachments() should ignore empty mime types to avoid returning an object with an invalid member name.
  • This notice doesn't actually occur until we attempt to access the member name, in this case by iterating over it with a foreach.

I am perfectly fine with papering over this for now by converting it back to an array. I just wanted to make sure I understood what was going on.

#11 @SergeyBiryukov
10 years ago

  • Component changed from Upgrade/Install to Media

#12 @nacin
10 years ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In 28139:

Cast wp_count_attachments() before iterating it to avoid a notice when items exist without a mime type.

props bobbingwide.
fixes #27802.

Note: See TracTickets for help on using tickets.