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

Allow multi value headers #35

Closed
hollodotme opened this issue Apr 29, 2019 · 3 comments · Fixed by #38
Closed

Allow multi value headers #35

hollodotme opened this issue Apr 29, 2019 · 3 comments · Fixed by #38
Assignees
Milestone

Comments

@hollodotme
Copy link
Owner

hollodotme commented Apr 29, 2019

Expected Behavior

  1. A call to Response#getHeaders() will return a two-dimensional array like this:
$expectedHeaders = [
    'Set-Cookie' => [
        'yummy_cookie=choco',
        'tasty_cookie=strawberry',
    ],
    'Status' => [
        'HTTP/2.0 200 OK'
    ],
    'Content-type' => [
        'text/html; charset=utf-8',
    ],
];

$this->assertSame($expectedHeaders, $response->getHeaders());
  1. A call to Response#getHeader($headerKey) will return a flat array with all values for the given key.
$expectedHeaderValues = [
    'yummy_cookie=choco',
    'tasty_cookie=strawberry',
];

$this->assertSame($expectedHeaderValues, $response->getHeader('Set-Cookie'));
  1. A call to a new method Response#getHeaderLine($headerKey) will return all values, separated by comma, for the given key.

  2. The results of Response#getHeader($headerKey) and Response#getHeaderLine($headerKey) will be the same for given keys in different case. (case-insensitive keys are accepted)

Actual Behavior

  1. Currently a call to Response#getHeaders() will return a flat key-value array and provide only the last value of a multi-value header.
[
    'Set-Cookie' => 'tasty_cookie=strawberry',
    'Status' => 'HTTP/2.0 200 OK',
    'Content-type' => 'text/html; charset=utf-8',
];
  1. Currently a call to Response#getHeader($headerKey) will return a string containing the last value for the given key.

  2. A method Response#getHeaderLine($headerKey) does not exist.

  3. Header keys are currently case-sensitive in Response#getHeader($headerKey).

Steps to Reproduce the Problem

  1. Create a worker script that sends the following headers:
    header('Status: HTTP/2.0 200 OK');
    header('Set-Cookie: yummy_cookie=choco');
    header('Set-Cookie: tasty_cookie=strawberry');
    header('Content-type: text/html; charset=utf-8');
  2. Issue a GET request to the worker script via the FastCGI client.
  3. Read the headers using Response#getHeaders() or Response#getHeader('Set-Cookie') respectively.

Specifications

  • Package version: <=2.7.0
  • PHP version: ALL
  • Platform/OS: ALL

Further comments

Changing the return type of Response#getHeaders() and Response#getHeader($headerKey) to array is a BC break and requires a new major version.

Proposed version: 3.0.0

@mnapoli
Copy link
Contributor

mnapoli commented Apr 29, 2019

👍

Do you think it could make sense to add a new method like getMultiHeader($key) to keep BC? Maybe it's not as clean as you want it to be though, in any case both options are fine for Bref since it's still wip ^^

@hollodotme
Copy link
Owner Author

@mnapoli I think I'll go with the PSR standard and also implement Response#getHeaderLine($headerKey) that returns a string with all header values.

So in the end:

public function getHeaders() : array # two-dimensional as state above

public function getHeader(string $headerKey) : array # 1-dimensional containing all values of the header

public function getHeaderLine(string $headerKey) : string # all values separated with comma

@mnapoli
Copy link
Contributor

mnapoli commented Apr 29, 2019

👍 makes sense!

hollodotme added a commit that referenced this issue Apr 29, 2019
* Add Response#getHeaderLine() to retrieve headers as string
* Change return type of Response#getHeader() to array
* Change return value of Response#getHeaders() to 2-dimensional array
* Add & adjust tests
@hollodotme hollodotme mentioned this issue Apr 29, 2019
2 tasks
hollodotme added a commit that referenced this issue Apr 29, 2019
If headers with multiple values use header keys of different letter-case
they will also be grouped when retrieved via getHeader()
or getHeaderLine(), but the original headers (case-sensitive) are kept
in the result of getHeaders().
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants