Make WordPress Core

Opened 13 years ago

Closed 9 years ago

Last modified 9 years ago

#16026 closed enhancement (fixed)

Stop using ereg() and eregi()

Reported by: norwayfun's profile NorwayFun Owned by: dd32's profile dd32
Milestone: 4.4 Priority: normal
Severity: minor Version:
Component: External Libraries Keywords: has-patch needs-testing dev-feedback
Focuses: Cc:

Description

see attached file. ereg(), eregi() and so on deprecated functions was replaced by mb_ suffix ones. also, checker for set_magic_quotes_runtime
disabler for 5.2 and 5.3 series PHP included

Attachments (3)

wp_php_5.3.patch (6.5 KB) - added by NorwayFun 13 years ago.
16026.diff (3.6 KB) - added by aaroncampbell 13 years ago.
Change all ereg to preg
16026.2.diff (3.6 KB) - added by aaroncampbell 13 years ago.
Use strlower & == where possible

Download all attachments as: .zip

Change History (17)

#1 @mtekk
13 years ago

Should probably use

version_compare(PHP_VERSION, '5.3.0', '<')

rather than

strpos($_SERVER['SERVER_SOFTWARE'], 'PHP/5.3') !== false

in wp-settings.php Regardless, it looks like we should be able to use

ini_set('magic_quotes_runtime', 0);

as a drop in replacement of

set_magic_quotes_runtime(0);

#2 @NorwayFun
13 years ago

thing is, that your solution will be removed in php 6.0 so patch will be switched to "case", because disabling magic quotes looks different between PHP 5.2.16>5.3.0>6.0.0

#3 @dd32
13 years ago

  • Component changed from General to Warnings/Notices
  • Keywords 3.2-early added; php 5.3 removed
  • Milestone changed from Awaiting Review to Future Release

#4 @nacin
13 years ago

For clarification, we cannot modify the magic quotes setting or multibyte. But we do need to move ereg to pcre.

#5 @scribu
13 years ago

  • Keywords 3.2-early removed
  • Milestone changed from Future Release to 3.2
  • Summary changed from patch for more support for php 5.3 to Stop using ereg() and eregi()

#6 @aaroncampbell
13 years ago

Are we maintaining pemFTP? http://www.phpclasses.org/package/1743-PHP-FTP-client-in-pure-PHP.html :

  • wp-admin/include/class-ftp.php
  • wp-admin/include/class-ftp-pure.php
  • wp-admin/include/class-ftp-sockets.php

It looks like the only things we've done to it in three years is replace a ;; with a ; in [12548], fix IP address validation (#12387) and add some documentation in [8645]

Also, I'm pretty sure we're NOT maintaining phpMailer or the SquirrelMail pop3 implementations. I know that the one instance in phpMailer would be fixed if we upgraded it (#15912), and upgrading the SquirrelMail POP3 class would fix the ones there as well (#17064)

#7 @dd32
13 years ago

  • Component changed from Warnings/Notices to External Libraries
  • Keywords needs-patch added
  • Milestone changed from 3.2 to Future Release
  • Severity changed from normal to minor

Are we maintaining pemFTP?

Anything which affects WordPress's usage of it, we're going to have to update.

Looking at the changes, a number of them can probably be done with plain string comparisons instead, others should use preg_*() functions.

I'm marking this for future release pending a patch

#8 @aaroncampbell
13 years ago

  • Keywords has-patch dev-feedback 2nd-opinion needs-testing added; needs-patch removed

That patch just converts all ereg to preg (and could use some testing). However, it looks like I can improve (or possibly just remove) the glob_regexp() function. It's only job is to match a string (case insensitive for everything but WIN32). It seems like:

if ( PHP_OS != 'WIN32' ) {
	return ( strtolower( $pattern ) == strtolower( $probe ) );
}
return ( $pattern == $probe );

Since the function is only used once I'd prefer to just put the code inline rather than have the function at all, but I wasn't sure how important it was to keep around. Would anyone be trying to use it directly?

#9 @aaroncampbell
13 years ago

The .2 diff uses strtolower & == where possible. None of my setups ever seem to trigger that code though, so it definitely still needs plenty of testing no matter which patch we choose.

#10 @dd32
13 years ago

  • Keywords dev-feedback 2nd-opinion removed

In both patches, the closing delimiter of wp-admin/includes/class-ftp-pure.php is wrong (in the wrong string)

and in .2 the function change looks ok, but it wasn't updated in the spot where it was called.

my string comparison remark was mainly related to the changes in wp-includes/class-pop3.php it seems after re-reading the patch.

I'd go with the s/eregi/preg_*/g approach, all the individual regex's just need to be tested properly.

@aaroncampbell
13 years ago

Change all ereg to preg

@aaroncampbell
13 years ago

Use strlower & == where possible

#11 @chriscct7
9 years ago

  • Keywords dev-feedback added

#12 @wonderboymusic
9 years ago

  • Milestone changed from Future Release to 4.4
  • Owner changed from NorwayFun to dd32
  • Status changed from new to assigned

should we do something? should we close?

#13 @dd32
9 years ago

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

In 34281:

Updates: FTP/PemFTP Library: Remove the usage of deprecated regular expression functions (ereg replaced by PCRE).

Props enshrined, aaroncampbell
Fixes #16026, #33432

#14 @dd32
9 years ago

In 34282:

Updates: FTP: Add a missing PCRE modifer in [34281].

See #16026, #33432x

Note: See TracTickets for help on using tickets.