WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 5 years ago

#26265 closed defect (bug) (fixed)

safe_mode error when uploading audio/video (stuck on crunching)

Reported by: tomsommer Owned by: wonderboymusic
Milestone: 4.4 Priority: normal
Severity: normal Version: 3.6
Component: Media Keywords: has-patch commit
Focuses: Cc:

Description

There is a bug in the ID3 library, where it fails to calculate the correct tmp directory and throws realpath errors when the webserver is running safe_mode.

This patch fixes this.

Attachments (2)

id3.patch (1.4 KB) - added by tomsommer 7 years ago.
26265.patch (1.3 KB) - added by chriscct7 5 years ago.

Download all attachments as: .zip

Change History (10)

@tomsommer
7 years ago

#1 @SergeyBiryukov
7 years ago

  • Component changed from Upload to External Libraries
  • Version changed from 3.7.1 to 3.6

#2 @tomsommer
7 years ago

  • Summary changed from safe_mode error when uploading audio/video to safe_mode error when uploading audio/video (stuck on crunching)

#3 @nacin
7 years ago

  • Component changed from External Libraries to Media
  • Milestone changed from Awaiting Review to Future Release

I've asked wonderboymusic to look into this.

#4 @wonderboymusic
7 years ago

Few things:

  • http://www.php.net/manual/en/ini.sect.safe-mode.php#ini.safe-mode = This feature has been DEPRECATED as of PHP 5.3.0 and REMOVED as of PHP 5.4.0. Not sure that we even care to support this, even though it is in the scope of our minimum requirements.
  • The patch will result in the value ALWAYS being set by WordPress, so when getid3.php is loaded, there will be a warning for trying to reset GETID3_TEMP_DIR
  • We don't want to patch the getID3 library directly, so suppressing errors in the realpath() call isn't ideal. Even so, GETID3_TEMP_DIR isn't set conditionally, getid3.php ALWAYS attempts to set it, without checking if it is already defined.

This one is not keeping me up at night

#5 @tomsommer
7 years ago

So 5.3 support is dropped from WordPress core now? While everything else works fine, this is the one thing broken.

I don't really care how you fix it, but it should be fixed - and the fix is simple, so why not?

#6 @wonderboymusic
7 years ago

safe_mode isn't supported by PHP anymore. It is deprecated since PHP 5.3. WordPress makes changes with every new version of PHP when necessary but provides backwards compatibility back to PHP 5.2.4.

The only reason to address this issue would be for versions >= PHP 5.2.4 and < PHP 5.3. Even then, suppressing the error for realpath() and allowing the override of GETID3_TEMP_DIR should be filed upstream: https://github.com/jamesheinrich/getid3 - we don't typically patch external libraries directly.

I like the direction of your patch, thanks for alerting us to the issue.

@chriscct7
5 years ago

#7 @chriscct7
5 years ago

  • Keywords commit added
  • Milestone changed from Future Release to 4.4
  • Severity changed from major to normal

Refreshed patch. Note part of the original patch was implemented in #29627

#8 @wonderboymusic
5 years ago

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

In 34866:

Media: in wp_read_video|audio_metadata(), set GETID3_TEMP_DIR to get_temp_dir() if it is not defined. This is a workaround for when safe_mode is enabled pre-PHP 5.3.

Props chriscct7, tomsommer.
Fixes #26265.

Note: See TracTickets for help on using tickets.