Ability to unset a default header added by HttpKernelBrowser / Client

[teohhanhui]

Symfony version(s) affected: 3.4+

Description
There is currently no way to unset a HTTP request header through the BrowserKit AbstractBrowser / Client. This was requested before in #20306, but actually implementing an unsetServerParameter method would not help in our case.

The problem we're facing here (in API Platform) is that when we send a request using HttpKernelBrowser / Client:

symfony/src/Symfony/Component/HttpKernel/Client.php

Lines 136 to 138 in f1edae0

	protected function filterRequest(DomRequest $request)
	{
	$httpRequest = Request::create($request->getUri(), $request->getMethod(), $request->getParameters(), $request->getCookies(), $request->getFiles(), $request->getServer(), $request->getContent());

it calls (HttpFoundation) Request::create which adds some default request headers:

symfony/src/Symfony/Component/HttpFoundation/Request.php

Lines 309 to 324 in f1edae0

	public static function create($uri, $method = 'GET', $parameters = [], $cookies = [], $files = [], $server = [], $content = null)
	{
	$server = array_replace([
	'SERVER_NAME' => 'localhost',
	'SERVER_PORT' => 80,
	'HTTP_HOST' => 'localhost',
	'HTTP_USER_AGENT' => 'Symfony',
	'HTTP_ACCEPT' => 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8',
	'HTTP_ACCEPT_LANGUAGE' => 'en-us,en;q=0.5',
	'HTTP_ACCEPT_CHARSET' => 'ISO-8859-1,utf-8;q=0.7,*;q=0.7',
	'REMOTE_ADDR' => '127.0.0.1',
	'SCRIPT_NAME' => '',
	'SCRIPT_FILENAME' => '',
	'SERVER_PROTOCOL' => 'HTTP/1.1',
	'REQUEST_TIME' => time(),
	], $server);

This includes the Accept header, which we need to be able to remove in order to test content negotiation scenarios where the client doesn't send the Accept header, thereby resulting in the use of the default media type.

How to reproduce
We had been using a workaround to unset the Accept header in our test suite, basically:

$client->setServerParameter('HTTP_ACCEPT', null);

The end result in the HeaderBag is:

array(1) {
  ["accept"]=>
  array(1) {
    [0]=>
    NULL
  }
}

However, fixes in HeaderBag::get, specifically the change made to this line:

symfony/src/Symfony/Component/HttpFoundation/HeaderBag.php

Line 124 in f1edae0

return \count($headers[$key]) ? (string) $headers[$key][0] : $default;

means that we get an empty string now instead of null.

Possible Solution
So this breaks our test suite. We could find new workarounds, including checking for the case where the Accept header is an empty string (weird!), or overriding / extending some parts of the chain which sends the request in our test suite.

However, we're wondering if there might be a better way of fixing this for everyone. It was an ugly workaround in the first place.

This problem also affects ApiTestCase in API Platform, as the user would be unable to send a request without any of the default headers.

Additional context

[nicolas-grekas]

However, fixes in HeaderBag::get, specifically the change made to this line [...] means that we get an empty string now instead of null.

this behavior change has been fixed, so that API Platform tests should now be alright, isn't it?
If confirmed, I think we can close as the use case is covered.

[teohhanhui]

Our test suite is fixed, yes, as we've implemented further workarounds.

However, the design problem remains. There is no way for the user to unset any of these default headers when they're using the HttpKernelBrowser / Client (from the HttpKernel component). It's a problem with our Client class in API Platform too, as it extends from the HttpKernel one.

It's very bad for DX when you don't have full control over the HTTP client you're using. The limitations are understandable if you're working with an actual browser, but it's not the case here. We should aim for feature parity with curl or a mobile application doing HTTP requests.

[nicolas-grekas]

"Very bad" is very subjective. What you can use today is the best the community achieved so far, thanks to them. About the future: PR welcome as usual :)

[teohhanhui]

Sorry if my comment came off as diminishing the work of the community. That was not my intention.

What I mean is, this is a feature request. 😄

[dunglas]

For the record, API Platform is bite by this problem again. Our workaround doesn't work anymore because typehints have been added: https://github.com/api-platform/core/blob/89c0f30d188b0ff4f15302d1ee901353417663e3/features/bootstrap/HttpHeaderContext.php#L35