Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Urgent - v2.1.1 - MessageTrait assertValue broken #489

Closed
thomas-alrek opened this issue Mar 20, 2022 · 16 comments
Closed

Urgent - v2.1.1 - MessageTrait assertValue broken #489

thomas-alrek opened this issue Mar 20, 2022 · 16 comments

Comments

@thomas-alrek
Copy link

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
Copy link

mbabker commented Mar 20, 2022

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
Copy link

kissifrot commented Mar 20, 2022

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
Copy link

it-can commented Mar 20, 2022

guzzle/guzzle#2998

@it-can
Copy link

it-can commented Mar 20, 2022

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
Copy link

holtkamp commented Mar 20, 2022

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

Here are the changes: 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
Copy link
Member

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

@GrahamCampbell
Copy link
Member

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

@GrahamCampbell
Copy link
Member

cc @ibrasho

@it-can
Copy link

it-can commented Mar 20, 2022

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

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

@GrahamCampbell
Copy link
Member

1.8.5, 2.1.2, 2.2.1 released.

@TimWolla
Copy link
Contributor

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
Copy link

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
Copy link
Member

@samizdam can you submit a failing test please?

@bullder
Copy link

bullder commented Apr 27, 2022

@samizdam can you confirm in #502 that you have met same control symbol in value of google analytics cookie?

@ablyanant

This comment was marked as outdated.

@GrahamCampbell
Copy link
Member

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

@guzzle guzzle locked as resolved and limited conversation to collaborators Aug 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants