Bug #88420

HTTP Headers are incorrectly send when using PSR-15 middleware

Added by Markus Poerschke over 2 years ago. Updated almost 2 years ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
-
Target version:
-
Start date:
2019-05-23
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
9
PHP Version:
Tags:
Complexity:
Is Regression:
Sprint Focus:

Description

When sending HTTP headers, the values of the headers are combined with a comma, if multiple headers exists.

For example this hinders for example, that multiple cookies can be set at the same time.

Example:

Given you have the following code in a middleware

$response = $response->withAddedHeader('Set-Cookie', 'my_session_a=1');
$response = $response->withAddedHeader('Set-Cookie', 'my_session_a=2');

return $response;

The expected output is, that two headers are set:

Set-Cookie: my_session_a=1
Set-Cookie: my_session_b=2

But instead the values are combined:

Set-Cookie: my_session_a=1, my_session_b=2

This behaviors has the impact, that only the "my_session_a" cookie is set. The other is ignored.


Related issues

Related to TYPO3 Core - Bug #88457: Revert "Allow to send multiple HTTP headers with the same name"ClosedBenni Mack2019-05-29

Actions
Related to TYPO3 Core - Bug #88482: Allow multi line headers for "Set-Cookie" headersClosed2019-06-03

Actions
#1

Updated by Gerrit Code Review over 2 years ago

  • Status changed from New to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/60800

#2

Updated by Jonas Eberle over 2 years ago

Interesting. I did a little research.

For most headers, this is semantically identical and also allowed https://www.ietf.org/rfc/rfc2616.txt

Under 4.2:

   Multiple message-header fields with the same field-name MAY be present 
   in a message if and only if the entire field-value for that header 
   field is defined as a comma-separated list [i.e., #(values)]. It
   MUST be possible to combine the multiple header fields into one 
   "field-name: field-value" pair, without changing the semantics of the 
   message, by appending each subsequent field-value to the first, each
   separated by a comma. The order in which header fields with the same 
   field-name are received is therefore significant to the interpretation 
   of the combined field value, and thus a proxy MUST NOT change the 
   order of these field values when a message is forwarded.

But for cookies, it is not possible https://www.ietf.org/rfc/rfc6265.txt

Under 3:

   Origin servers SHOULD NOT fold multiple Set-Cookie header fields into
   a single header field.  The usual mechanism for folding HTTP headers
   fields (i.e., as defined in [RFC2616]) might change the semantics of
   the Set-Cookie header field because the %x2C (",") character is used
   by Set-Cookie in a way that conflicts with such folding.
#3

Updated by Gerrit Code Review over 2 years ago

Patch set 1 for branch 9.5 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/60815

#4

Updated by Anonymous over 2 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
#5

Updated by Benni Mack over 2 years ago

  • Related to Bug #88457: Revert "Allow to send multiple HTTP headers with the same name" added
#6

Updated by Markus Poerschke over 2 years ago

Since the new behavior broke I see the following options, if the goal should be to be able to send multiple cookies within the PSR-Middleware:

1. Always override existing headers with the first call:

$headerSent = [];
foreach ($response->getHeaders() as $name => $values) {
    $headerSent[$name] = true;
    foreach ($values as $value) {
        header($name . ': ' . $value, !$headerSent[$name]);
    }
}

The benefit of this version is that it will behave like before unless only the headers are only available with a single value within the PSR-7 response object. If someone relied on the "implode" when adding multiple values to the header, then this might break the application again.

2. Only send cookie without overriding the already sent headers:

foreach ($response->getHeaders() as $name => $values) {
    if (strtolower($name) === 'set-cookie') {
        foreach ($values as $value) {
            header($name . ': ' . $value, false);
        }
    } else {
        header($name . ': ' . implode(', ', $values));
    }
}

I think for Version 9.5 the option number 2 will not be a breaking change. Therefore adding option 1 for v10 (but needs to be along fixing all usages of "header()" which can take a while) and use option 9.5 until that time.

#7

Updated by Markus Poerschke over 2 years ago

  • % Done changed from 100 to 50
#8

Updated by Markus Poerschke over 2 years ago

  • % Done changed from 50 to 100
#9

Updated by Markus Poerschke over 2 years ago

  • Related to Bug #88482: Allow multi line headers for "Set-Cookie" headers added
#10

Updated by Benni Mack almost 2 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF