WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#16325 closed defect (bug) (fixed)

kses uses deprecated (PHP 5.3.0) split

Reported by: duck_ Owned by: 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_ 3 years ago.
garyc40.16325.diff (5.7 KB) - added by garyc40 3 years ago.
fix deprecated split() in other files
rss-#16325.diff (465 bytes) - added by j_schumann 2 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_3 years ago

comment:1 duck_3 years ago

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

garyc403 years ago

fix deprecated split() in other files

comment:2 follow-up: garyc403 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.

comment:3 nacin3 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 than 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.

Version 0, edited 3 years ago by nacin (next)

comment:4 in reply to: ↑ 2 duck_3 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.

comment:5 westi3 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

comment:6 nacin3 years ago

  • Version set to 3.0

comment:7 aaroncampbell3 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.

comment:8 follow-up: j_schumann2 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_schumann2 years ago

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

comment:9 knutsp2 years ago

  • Cc knut@… added

comment:10 duck_2 years ago

  • Milestone changed from Awaiting Review to 3.4

comment:11 duck_2 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.

comment:12 in reply to: ↑ 8 duck_2 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.