http_fopen_wrapper.c - Handle HTTP headers with varying white space#1902
http_fopen_wrapper.c - Handle HTTP headers with varying white space#1902IMSoP wants to merge 3 commits intophp:PHP-5.6from
Conversation
The stream handler assumed all HTTP headers contained exactly one space, but the standard says there may be zero or more. Should fix Bug #47021, and any other edge cases caused by a web server sending unusual spacing, e.g. the MIME type discovered from Content-Type: can no longer contain leading whitespace.
ext/standard/http_fopen_wrapper.c
Outdated
| > Each header field consists of a case-insensitive field name followed | ||
| > by a colon (":"), optional leading whitespace, the field value, and | ||
| > optional trailing whitespace. */ | ||
| char *http_header_value = http_header_line + 1; |
There was a problem hiding this comment.
Declarations need to go at the top of the block in C89.
|
Thanks for looking over this so quickly! I'll hopefully have time to look into your feedback properly tomorrow, and update the patch. :) |
- fix removal of trailing whitespace (and add more test cases) - more obvious detection if there is no : in the header - coding style / C89 compliance
|
hey all, i'd just like to check the status of this - it looks like we're seeing an issue that this patch will solve. |
|
@nikic Since you reviewed before, does this look OK now? I'm pretty sure the Travis error was just a false report. |
|
awaits nikitas response |
|
Question: What should happen with the headers we add to the The latter would certainly be easier to implement. Some of the APIs used there don't accept explicit lengths :/ |
|
The spec indicates that the trailing white space is allowed, however since it serves no practical purpose, I see no harm in removing it - the vast majority of the time, the programmer is surely going to do that anyway, and if they don't their code may exhibit unexpected behaviour. |
5 participants
The stream handler assumed all HTTP headers contained exactly one space,
but the standard says there may be zero or more. Should fix Bug #47021,
and any other edge cases caused by a web server sending unusual spacing,
e.g. the MIME type discovered from Content-Type: can no longer contain
leading whitespace.