WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 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 Owned by: 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 6 years ago.
27802.2.diff (641 bytes) - added by jeremyfelt 6 years ago.
27802.3.diff (541 bytes) - added by bobbingwide 6 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.


6 years ago

#2 @bobbingwide
6 years ago

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

#3 @bobbingwide
6 years ago

Problem also applies to plugin uploads.

#4 @bobbingwide
6 years ago

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

#5 @bobbingwide
6 years ago

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

#6 @jeremyfelt
6 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
6 years ago

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


6 years ago

@jeremyfelt
6 years ago

#8 @jeremyfelt
6 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
6 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
6 years ago

Cast the result from wp_count_attachments() to an array

#10 @nacin
6 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
6 years ago

  • Component changed from Upgrade/Install to Media

#12 @nacin
6 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.