Nyholm / psr7

A super lightweight PSR-7 implementation
MIT License
1.16k stars 75 forks source link

issue: InvalidArgumentException: Header values must be RFC 7230 compatible strings. #167

Closed DominicGBauer closed 2 years ago

DominicGBauer commented 3 years ago

Detailed description

I keep getting this error: InvalidArgumentException: Header values must be RFC 7230 compatible strings.

It seems like I'm getting this error because of a Null value in the access-control-allow-origin header of the baseResponse. Is this supposed to happen and/or am I missing something?

Here's the trace:

/var/www/vendor/nyholm/psr7/src/MessageTrait.php:201
/var/www/vendor/nyholm/psr7/src/MessageTrait.php:80
/var/www/vendor/symfony/psr-http-message-bridge/Factory/PsrHttpFactory.php:163
/var/www/vendor/osteel/openapi-httpfoundation-testing/src/HttpFoundation/HttpFoundationResponseAdapter.php:33
/var/www/vendor/osteel/openapi-httpfoundation-testing/src/ResponseValidator.php:57
/var/www/tests/Feature/SensorAPI/GatewayControllerTest.php:113

This is an example of the test I am trying to run:

        $response = $this->get(
            'api/v1/sensor/gateways/gateway_name',
            [
                'Accept' => 'application/json',
                'Content-Type' => 'application/json',
                'Authorization' => 'Bearer token'
            ]
        );

        $validator = ResponseValidatorBuilder::fromJson(storage_path('api-docs/api-docs.json'))->getValidator();

        $result = $validator->validate('/v1/sensor/gateways/gateway_name', 'get', $response->baseResponse);

        $this->assertTrue($result);

The baseResponse header looks like this:

(
    [headers] => Symfony\Component\HttpFoundation\ResponseHeaderBag Object
        (
            [computedCacheControl:protected] => Array
                (
                    [no-cache] => 1
                    [private] => 1
                )

            [cookies:protected] => Array
                (
                )

            [headerNames:protected] => Array
                (
                    [cache-control] => Cache-Control
                    [date] => Date
                    [content-type] => Content-Type
                    [access-control-allow-origin] => Access-Control-Allow-Origin
                    [vary] => Vary
                    [access-control-allow-credentials] => Access-Control-Allow-Credentials
                )

            [headers:protected] => Array
                (
                    [cache-control] => Array
                        (
                            [0] => no-cache, private
                        )

                    [date] => Array
                        (
                            [0] => Wed, 06 Jan 2021 11:07:53 GMT
                        )

                    [content-type] => Array
                        (
                            [0] => application/json
                        )

                    [access-control-allow-origin] => Array
                        (
                            [0] => 
                        )

                    [vary] => Array
                        (
                            [0] => Origin
                        )

                    [access-control-allow-credentials] => Array
                        (
                            [0] => true
                        )

                )

        )
)

The $header and $values I get when I dump them in the validateAndTrimHeader() in MessageTrait class are:

string(13) "cache-control"
array(1) {
  [0]=>
  string(17) "no-cache, private"
}
string(4) "date"
array(1) {
  [0]=>
  string(29) "Wed, 06 Jan 2021 11:07:53 GMT"
}
string(12) "content-type"
array(1) {
  [0]=>
  string(16) "application/json"
}
string(27) "access-control-allow-origin"
array(1) {
  [0]=>
  NULL
}

My environment

PHP 7.3.20 Laravel 8 nyholm/psr7 : ^1.3

Zegnat commented 3 years ago

Can you help me create a minimal test case reproducing this? I am not sure I fully understand what dependencies I all need to get the test code you are trying to run working.

An InvalidArgumentException is exactly what PSR-7 tells us to throw when an invalid header value is provided. We have a specific test to make sure we accept empty strings, but a header must be a string value according to PSR-7.

If somehow we are converting an empty string into a null, we need to fix that. But I am having trouble recreating this issue and figuring out the code path. If any of the steps above us in the stack trace (Symfony's PsrHttpFactory, OpenAPI HttpFoundation Testing, ...) is providing the null value directly to a PSR-7 instance then the bug is a PSR-7 integration error in that package.

Integrations with PSR-7 must make sure to either validate the data they are passing into the objects themselves, or handle the exceptions that are expected out of different PSR-7 methods.

Happy to help debug this and figure out where the error is, but I am going to need help reproducing, sorry!

Nyholm commented 3 years ago

This library is strictly following the specification for HTTP and PSR7. I know (because I worked on it myself) that the PsrHttpFactory may have some edge cases where it does not covert things correctly.

Arkitecht commented 2 years ago

@DominicGBauer I was having a similar issue while using the Intercom API library, and it turned it I was missing a config variable to set the correct API version in the request. Might you be missing something similar? The issue is probably in the null response you are getting, and not in how the library is handling it.

nuryagdym commented 2 years ago

Hi, I had analyzed this issue. This error is just not caused by only this library, I have also tried laminas/laminas-diactoros, slim/psr7 libraries. They are also throwing the same error.

The error was caused because of a leading space on the response header name " x-xss-protection" this should be "x-xss-protection".

I also checked nyholm/psr7 (also laminas/laminas-diactoros, slim/psr7) in combination with following PSR-18 clients: php-http/curl-client, symfony/http-client, guzzlehttp/guzzle.

This error is thrown only when symfony/http-client is used as a PSR-18 client implementation. On the others (php-http/curl-client, guzzlehttp/guzzle) I did not get this issue, because they are trimming header names:

guzzlehttp/guzzle: GuzzleHttp\Handler\CurlFactory::createHeaderFn()

php-http/curl-client: Http\Client\Curl\Client::prepareRequestOptions()

but symfony/http-client does not do trimming.

So to fix this error for now you can use either php-http/curl-client or guzzlehttp/guzzle

I also opened PR on https://github.com/symfony/symfony/pull/47415 to address this issue.

nicolas-grekas commented 2 years ago

Now that https://github.com/symfony/symfony/pull/47415 is merged, this can be closed. Thanks @nuryagdym for the patch.

nicolas-grekas commented 2 years ago

I've opened https://github.com/symfony/psr-http-message-bridge/pull/107 on symfony/psr-http-message-bridge which is also affected.