Interpret content-type header for 'application/x-www-form-urlencoded' correctly

[caseyamcl]

Sometimes the Content-Type header can have additional information, such as charset. Also, there should only be one Content-Type header in a HTTP Request.

This PR fixes interpretation of the application/x-www-form-urlencoded by comparing only the beginning of the Content-Type header value instead of testing for equality. So, now both of these headers will now be interpreted correctly:

Worked before:

Content-Type: application/x-www-form-urlencoded

Works now:

Content-Type: application/x-www-form-urlencoded; charset=utf-8

[daltones]

Have a look at these lines:

$headers = $psrRequest->getHeaders();
array_walk($headers, function(&$val) {
    if (1 === count($val)) {
        $val = $val[0];
    }
});

PSR-7 says that getHeaders() MUST return an array of string arrays. So, those lines could make a mess while we're considering a single string elsewhere.

Shouldn't Content-Type: application/x-www-form-urlencoded; charset=utf-8 be parsed as 'Content-Type' => ['application/x-www-form-urlencoded', 'charset=utf-8'] by GuzzleHttp\Psr7\parse_request()? I actually didn't understand PSR-7 just mentioning commas…

(a little refactoring: what about stripos($headers['Content-Type'] instead of strpos(strtolower($headers['Content-Type'])?)

[greevex]

What about this pull request? At this moment it's really a bug in react/http usage.

[WyriHaximus]

Why did you choose to go for === 0over !== false? My preference would be the latter, that leaves room for an client messing up and adding a space in front of it. (Just nitpicking a bit here.)

[caseyamcl]

Either way is fine, and I can change it if you want.

I can't find anywhere in the the official documents that specifies that the header name must appear as the first character on a line, although I've never seen white space before a header in practice.

Perhaps this may be the least ambiguous:

strpos(strtolower(trim($headers['Content-Type'])), 'application/x-www-form-urlencoded') === 0

[WyriHaximus]

Fair enough, leave it as is. Was mostly nitpicking because I've seen clients (and servers) pull really odd stunts so trying to be a bit more defensive and prepared for such occasions.

[WyriHaximus]

@daltones It is something that has been on my mind for a while. Tbh I want to convert the whole thing to PSR-7 so it is something to keep in mind.

@greevex Yes it is, my current goal is to get this and the multipart streaming into the next tag.

[clue]

Afaict this has been filed to fix the previously broken master branch. Now that #59 is in, is this still relevant?

[clue]

Thanks for filing this PR, I can assure you we're working hard on this 👍

This PR currently targets an unstable development feature that is no longer part of the master branch, so I've just filed #105 to keep track of the underlying feature request here. Please bear with us, we'll keep this ticket updated 👍