Make WordPress Core

Opened 13 years ago

Closed 12 years ago

Last modified 12 years ago

#16325 closed defect (bug) (fixed)

kses uses deprecated (PHP 5.3.0) split

Reported by: duck_'s profile duck_ Owned by: duck_'s profile duck_
Milestone: 3.4 Priority: normal
Severity: normal Version: 3.0
Component: General Keywords: has-patch
Focuses: Cc:

Description

wp-includes/kses.php line 1407, part of safecss_filter_attr.

split is deprecated in PHP as of 5.3.0, we should probably replace with explode here.

Attachments (3)

16325.diff (437 bytes) - added by duck_ 13 years ago.
garyc40.16325.diff (5.7 KB) - added by garyc40 13 years ago.
fix deprecated split() in other files
rss-#16325.diff (465 bytes) - added by j_schumann 12 years ago.
replace split() in wp-includes/rss.php, all other occurences seem to be coverd in the previous attachments

Download all attachments as: .zip

Change History (15)

@duck_
13 years ago

#1 @duck_
13 years ago

Actually used in approximately 20 places overall (probably should have checked this first).

@garyc40
13 years ago

fix deprecated split() in other files

#2 follow-up: @garyc40
13 years ago

I didn't replace split() in 3 places:

  • atomlib
  • rss (magpieRSS)
  • phpmailer

These are external packages. They probably have upstream changes already.

I also checked each instances whether to use explode() or preg_split(). Seems like all of them can be explode(), but we should be careful if there's any additional replacement we need to make.

#3 @nacin
13 years ago

magpieRSS is dead upstream and deprecated here, so not touching it is fine.

atomlib.php looks dead upstream as well -- DOA in fact. Only change other than whitespace and docs is actually downstream (r4892). We should probably take ownership of it again formally, considering it was originally a contribution to WordPress, rather than an external package we brought in. I'm also noting that it is licensed under Apache upstream, but was originally contributed to WordPress as GPL. (Not sure why it was even treated as an external library due to the way it was contributed.)

Reference: http://code.google.com/p/phpatomlib/source/browse/trunk/lib/atomlib.php

phpmailer, I believe there exists a PHP5 version upstream, so we should update to that for 3.2.

Last edited 13 years ago by nacin (previous) (diff)

#4 in reply to: ↑ 2 @duck_
13 years ago

Replying to garyc40:

I also checked each instances whether to use explode() or preg_split(). Seems like all of them can be explode(), but we should be careful if there's any additional replacement we need to make.

Yeah that's why I didn't rush into it last night. I imagine explode will be fine in these cases, but do need to make sure.

#5 @westi
13 years ago

I wouldn't worry about atomlib we will like stop using it soon - https://core.trac.wordpress.org/attachment/ticket/7652/7652-wpapp-done.diff

#6 @nacin
13 years ago

  • Version set to 3.0

#7 @aaroncampbell
13 years ago

As a quick note, these are different:

list($domain, $video_id) = split(".com/", $url);
list($domain, $video_id) = explode(".com/", $url);

Technically in split (being a POSIX Regex) the . is a single-character wildcard. I actually think that the way explode works is what was intended for this particular bit of code, but I thought I'd point it out.

#8 follow-up: @j_schumann
12 years ago

+1

I would highly appreciate seeing this fixed in 3.3, this bug is open for 10 months and unchanged, with patches available that do not change functionality or break b/c so it should not be of any problems.

@j_schumann
12 years ago

replace split() in wp-includes/rss.php, all other occurences seem to be coverd in the previous attachments

#9 @knutsp
12 years ago

  • Cc knut@… added

#10 @duck_
12 years ago

  • Milestone changed from Awaiting Review to 3.4

#11 @duck_
12 years ago

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

In [19726]:

split was deprecated in PHP 5.3.0, so use explode instead. Props garyc40. Fixes #16325.

#12 in reply to: ↑ 8 @duck_
12 years ago

Replying to j_schumann:

replace split() in wp-includes/rss.php, all other occurences seem to be coverd in the previous attachments

See nacin's comment above for why it wasn't removed from wp-includes/rss.php.

Note: See TracTickets for help on using tickets.