WordPress.org

Make WordPress Core

Opened 12 years ago

Closed 12 years ago

Last modified 11 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:

Description

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 12 years ago.
9395.diff (1.3 KB) - added by Denis-de-Bernardy 12 years ago.
9395.patch (939 bytes) - added by hakre 12 years ago.
The RFC is not that complex. (Fixed)
9395.2.patch (941 bytes) - added by hakre 12 years ago.
The RFC is not that complex. (RegEx Fixed)

Download all attachments as: .zip

Change History (19)

@wnorris
12 years ago

#1 @ryan
12 years ago

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

#2 @Denis-de-Bernardy
12 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
12 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
12 years ago

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

needs a new patch then

#5 @hakre
12 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.

@hakre
12 years ago

The RFC is not that complex. (Fixed)

#6 @hakre
12 years ago

regex was broken in patched. fixed.

#7 @Denis-de-Bernardy
12 years ago

  • Keywords tested commit added; 2nd-opinion removed

+1

#8 @ryan
12 years ago

wnorris, do these patches work for you?

#9 @Denis-de-Bernardy
12 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
12 years ago

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

@hakre
12 years ago

The RFC is not that complex. (RegEx Fixed)

#11 @hakre
12 years ago

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

#12 @ryan
12 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
12 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
12 years ago

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

#15 @hakre
11 years ago

Related: #13513

Note: See TracTickets for help on using tickets.