guzzle / psr7

PSR-7 HTTP message library
MIT License
7.88k stars 3 forks source link

Urgent - v2.1.1 - MessageTrait assertValue broken #489

Closed thomas-alrek closed 2 years ago

thomas-alrek commented 2 years ago

PHP version: 7.4.27 (hint: php --version)

Description I updated my dependencies, and guzzlehttp/psr7 was updated to v2.1.1. This broke an integration with a third party API that I'm working with. (Largest credit card payment processor in Scandiavia).

I have traced it down to the changes introduced in MessageTrait for the method assertValue. When the response (which I am not in control over), contains the following header, I get an InvalidArgumentException XXX is not a valid header value.

X-Iinfo: 12-34567890-123456789 AAAA BC(12 34 5) DE(1234567890123 123) a(1 2 3 4) b(1 2) A1

I have changed the actual values from the header, as I'm not sure if it contains confidential information. The relevant part here is the whitespaces, which failes the parsing introduced in 2.1.1.

How to reproduce Add the above header to a response.

Possible Solution Revert the changes done in 2.1.1 back to the behavior in 2.1.0

Additional context

mbabker commented 2 years ago

I've got a second case that these changes are breaking.

$request->setHeaders(['User-Agent' => 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_6) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/14.0.3 Safari/605.1.15');

kissifrot commented 2 years ago

I'm also getting it with the header x-generator: Drupal 7 (https://www.drupal.org) - value retrieved using curl CLI -, getting error "Drupal 7 (https://www.drupal.org)" is not valid header value)

it-can commented 2 years ago

https://github.com/guzzle/guzzle/issues/2998

it-can commented 2 years ago

To reproduce, tested on php 8.1 and guzzle 7.4

<?php
$header = 'Linux f0f489981e90 5.10.104-linuxkit 1 SMP Wed Mar 9 19:05:23 UTC 2022 x86_64';

$client = new \GuzzleHttp\Client();
$response = $client->request('GET', 'https://enswyqyojtxm.x.pipedream.net', [
    'headers' => [
        'User-Agent' => 'testing/1.0',
        'Accept'     => 'application/json',
        'X-Foo'      => ['Bar', 'Baz'],
        'X-TEST'     => $header,
    ]
]);

This will throw an error: "Linux f0f489981e90 5.10.104-linuxkit #1 SMP Wed Mar 9 19:05:23 UTC 2022 x86_64" is not valid header value

holtkamp commented 2 years ago

Seems to also occur when upgrading guzzle/psr7 1.8.3 => 1.8.4

Here are the changes: https://github.com/guzzle/psr7/compare/1.8.3...1.8.4

Forcing composer to install 1.8.3 using "guzzlehttp/psr7": "1.8.3" resolves the problem.

The string that was used as header value on a Heroku dyno running PHP 8.0.17:

Linux 2a6d8020-bdd7-4fd2-9341-dfbe8ac2506c 4.4.0-1101-aws #106-Ubuntu SMP Tue Mar 1 10:51:49 UTC 2022 x86_64

GrahamCampbell commented 2 years ago

If you'd like to propose a fix, please do, since this is urgent for you.

GrahamCampbell commented 2 years ago

I have prepared a fix. Please can you try this out @thomas-alrek @mbabker @kissifrot @it-can @holtkamp.

GrahamCampbell commented 2 years ago

cc @ibrasho

it-can commented 2 years ago

@GrahamCampbell yes the fix is working for me... Thanks for the quick-fix!

https://requestbin.com/r/en23lcyrh3o2p/26fRMDWJ8k32KhF1EAqZvPJU0vU

GrahamCampbell commented 2 years ago

1.8.5, 2.1.2, 2.2.1 released.

TimWolla commented 2 years ago

Hi folks. Sorry for breaking this. When creating the validation regex I've faithfully transcribed the ABNF given in RFC 7230#3.2. Double checking the ABNF it looks like the validation in guzzle/psr7 was technically correct and the header values are indeed not valid with the current spec. But of course this is not useful to you.

I've checked with the experts in #curl on libera.chat and it appears the error in the specification is already fixed in the latest draft: https://httpwg.org/http-core/draft-ietf-httpbis-semantics-latest.html#fields.values

samizdam commented 2 years ago

I am get InvalidArgumentException: "___utmvayaufcDoB=TphsWoW; path=/; Max-Age=900; Secure; SameSite=None" is not valid header value now in

/var/www/vendor/guzzlehttp/psr7/src/MessageTrait.php:267
/var/www/vendor/guzzlehttp/psr7/src/MessageTrait.php:203
/var/www/vendor/guzzlehttp/psr7/src/MessageTrait.php:206
/var/www/vendor/guzzlehttp/psr7/src/MessageTrait.php:175
/var/www/vendor/guzzlehttp/psr7/src/MessageTrait.php:148
/var/www/vendor/guzzlehttp/psr7/src/Response.php:107

with

$ php -v
PHP 7.4.9 (cli) ....

$ composer info guzzlehttp/psr7
name     : guzzlehttp/psr7
descrip. : PSR-7 message implementation that also provides common utility methods
keywords : http, message, psr-7, request, response, stream, uri, url
versions : * 1.8.5
....
GrahamCampbell commented 2 years ago

@samizdam can you submit a failing test please?

bullder commented 2 years ago

@samizdam can you confirm in https://github.com/guzzle/psr7/pull/502 that you have met same control symbol in value of google analytics cookie?

ablyanant commented 2 years ago

Changing the version from 1.9 to 1.8.3 worked for me.

In your root composer.json file add

"require": { "guzzlehttp/psr7": "1.8.3" },

and then run the command: composer require

and then check the version by command: composer info guzzlehttp/psr7

GrahamCampbell commented 2 years ago

Hidden above comment because it opens things up to major vulnerabilities. Locking down this whole thread, too. Everyone should only use the latest version.