Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 6 years ago

#9395 closed defect (bug) (fixed)

WP_Http: add support for headers split over multiple lines

Reported by: wnorris Owned by: ryan
Milestone: 2.8 Priority: normal
Severity: normal Version: 2.8
Component: HTTP API Keywords: has-patch tested commit
Focuses: Cc:


rfc2616 section 2.2 defines rules for splitting HTTP headers across multiple lines. This patch adds support for this to WP_Http.

Attachments (4)

http.diff (1.2 KB) - added by wnorris 7 years ago.
9395.diff (1.3 KB) - added by Denis-de-Bernardy 7 years ago.
9395.patch (939 bytes) - added by hakre 7 years ago.
The RFC is not that complex. (Fixed)
9395.2.patch (941 bytes) - added by hakre 7 years ago.
The RFC is not that complex. (RegEx Fixed)

Download all attachments as: .zip

Change History (19)

7 years ago

#1 @ryan
7 years ago

DD32, jacobsantos, review? Need to lose the CamelCase in the var names. WP uses underscores.

#2 @Denis-de-Bernardy
7 years ago

  • Keywords commit added; needs-review removed

Updated the patch to use underscores, and added a check on the header array's size.

@Ryan: His point is valid. I've used similar code to parse email headers in a separate app a few years back.

#3 @DD32
7 years ago

Sorry, Didnt see this ticket for review.

First code chunk: (status location changed)

  • Not sure if this is going to work correctly with proxies, I think it was in the loop for a reason, To catch the last status header rather than the first one (Which is often a proxy header)

Second code chunk: (Multiline headers)

  • Looks good to me, Pretty sure it was just something jacob didnt feel like doing, no technical reason as such
  • I'd get rid of the temp var myself, and use if ( in_array($headers[$i]{0}, array(' ', "\t")) ) { instead, but thats just me. s/$c/$headers[$i]{0}/g is another option
  • How's the trimming going to affect headers which have whitespace? (Or is that against spec?
    Header: Some Value.. Now 4 spaces:  
       Now some more text
    • It might be better to base that off a .= substr($string, 1) ?

#4 @Denis-de-Bernardy
7 years ago

  • Keywords needs-patch added; has-patch commit removed

needs a new patch then

#5 @hakre
7 years ago

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

new patch. the RFC is not _that_ complex. i just decided to go into the normalization of the headers upfront before splitting. that makes handling easier afterwards.

i was very strict to the rfc and therefore changed to propper CRLF tolerant parsing. that might be questionable but since this ticket was dedicated to the RFC voila.

unfolding upfront makes much more sense to me. just for the quote from 2.2:

HTTP/1.1 header field values can be folded onto multiple lines if the continuation line begins with a space or horizontal tab. All linear white space, including folding, has the same semantics as SP. A recipient MAY replace any linear white space with a single SP before interpreting the field value or forwarding the message downstream.

because DD32 questioned the location status logic i kept it out.

7 years ago

The RFC is not that complex. (Fixed)

#6 @hakre
7 years ago

regex was broken in patched. fixed.

#7 @Denis-de-Bernardy
7 years ago

  • Keywords tested commit added; 2nd-opinion removed


#8 @ryan
7 years ago

wnorris, do these patches work for you?

#9 @Denis-de-Bernardy
7 years ago

@ryan: in case wnorris doesn't reply back soon, I ran into similar issues in a separate piece of php software. I needed to parse emails and stick their content into a blog.

If you open an email's headers, you'll occasionally see things such as:

header name: values go a first line

more values go on a second line

the patch deals with those cases.

#10 @ryan
7 years ago

  • Owner set to ryan
  • Status changed from new to accepted

7 years ago

The RFC is not that complex. (RegEx Fixed)

#11 @hakre
7 years ago

now this time the broken regex was fixed. forgotten regex delimiters have been added. rest of patch unchanged.

#12 @ryan
7 years ago

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

(In [11351]) Support headers split over multiple lines. Props hakre, Denis-de-Bernardy, wnorris. fixes #9395

#13 @Lazy79
7 years ago

Warning: Wrong parameter count for str_replace() in /opt/lampp/htdocs/web11/html/wp-includes/http.php on line 402

Warning: Wrong parameter count for str_replace() in /opt/lampp/htdocs/web11/html/wp-includes/http.php on line 402

Warning: Wrong parameter count for str_replace() in /opt/lampp/htdocs/web11/html/wp-includes/http.php on line 402

After Updating i got these lines in the Dashboard - after a reload, they are gone.

I am not sure if i am doing right in posting it here.. please be patient :)

#14 @ryan
7 years ago

(In [11355]) Fix bad call to str_replace. see #9395

#15 @hakre
6 years ago

Related: #13513

Note: See TracTickets for help on using tickets.