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

Change in symfony/http-client 4.3.4 breaks headers in Test Client #3043

Closed
weaverryan opened this issue Sep 3, 2019 · 4 comments
Closed

Comments

@weaverryan
Copy link
Contributor

Hi friends!

We're using the new testing Client/Response, etc in our SymfonyCasts tutorial and we've had really nice success. However, one part stopped working with symfony/http-client 4.3.4. The problematic change is this one: https://github.com/symfony/symfony/pull/32823/files#diff-1ab846e0995199d7db8d975a9225e818

Basically, this changes the format of the $options['headers'] key in HttpClient::prepareRequest(), which the Client class calls

For example, suppose this request:

class CheeseListingResourceTest extends ApiTestCase
{
    public function testCreateCheeseListing()
    {
        $client = self::createClient();
        $client->request('POST', '/api/cheeses', [
            'headers' => ['Content-Type' => 'application/json'],
            'json' => [],
        ]);
        $this->assertResponseStatusCodeSame(401);
    }
}

Before 4.3.4, $options (if you dump right after

[$url, $options] = self::prepareRequest($method, $url, $options, $this->defaultOptions);
) looks like this:

array:7 [
  "headers" => array:2 [
    "content-type" => array:1 [
      0 => "application/json"
    ]
    "accept" => array:1 [
      0 => "application/ld+json"
    ]
  ]
...

With symfony/http-client 4.3.4, it now looks like:

array:7 [
  "headers" => array:2 [
    0 => "Content-Type: application/json"
    1 => "accept: application/ld+json"
  ]
  "normalized_headers" => array:2 [
    "content-type" => array:1 [
      0 => "Content-Type: application/json"
    ]
    "accept" => array:1 [
      0 => "accept: application/ld+json"
    ]
  ]

This causes the Client class from the new testing tools to incorrectly read those headers in this block:

foreach ($options['headers'] as $key => $value) {
if ('content-type' === $key) {
$server['CONTENT_TYPE'] = $value[0] ?? '';
continue;
}
// BrowserKit doesn't support setting several headers with the same name
$server['HTTP_'.strtoupper(str_replace('-', '_', $key))] = $value[0] ?? '';
}

It's probably an easy fix - sorry I don't have time at this moment to make a PR. But I hope it will be an easy pick :).

Cheers!

@alanpoulain
Copy link
Member

Thanks for reporting this @weaverryan, we came to the same conclusion with @teohhanhui.
That's why this PR is red: #3038.
It seems it's a BC break from Symfony (if HttpClient is no more experimental): https://symfony.com/doc/current/contributing/code/bc.html#using-our-traits
This method has changed: https://github.com/symfony/http-client/blob/v4.3.4/HttpClientTrait.php#L190

@teohhanhui
Copy link
Contributor

IMO, @experimental should mean "can break in the next minor release", not "can break in the next patch release".

What is Symfony's actual policy on that?

@weaverryan
Copy link
Contributor Author

It seems it's a BC break from Symfony (if HttpClient is no more experimental): https://symfony.com/doc/current/contributing/code/bc.html#using-our-traits

It IS still experimental (won't be in 4.4)

IMO, @experimental should mean "can break in the next minor release", not "can break in the next patch release".
What is Symfony's actual policy on that?

BC breaks are allowed in a minor, but not in a patch. I've opened an issue :) symfony/symfony#33450

@dunglas
Copy link
Member

dunglas commented Sep 26, 2019

Fixed by #3038

@dunglas dunglas closed this as completed Sep 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants