feat: add Message::addHeader() to add header with the same name

[kenjis]

Description
For #5041

• add Message::addHeader()
• implement Message::header() "return an array of header objects":

  CodeIgniter4/system/HTTP/MessageTrait.php

  Lines 120 to 121 in 0ab4d67

  	* Returns a single Header object. If multiple headers with the same
  	* name exist, then will return an array of header objects.

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[michalsn]

I don't get it. Multiple headers with the same name should be handled with a single Header class that has multiple values.

Am I missing something?

[kenjis]

The question is not strange. It seems PSR-7 specification is like that.

But I don't think these are the same:

1. a header with multiple values
2. multiple headers with the same name

If we handle multiple headers with the same name with a single Header class that has multiple values,
we cannot distinguish them.

If we were to send three Set-Cookie headers, how would we handle them?
We need to know that in the case of Set-Cookies, one value is sent with one header.

[michalsn]

The main issue I have with this is that code like this:

$this->request->header('NAME')?->getValueLine();

is no longer guaranteed to work.

If we handle multiple headers with the same name with a single Header class that has multiple values,
we cannot distinguish them.

I don't see much difference when it comes to distinguishing between values, because in both cases we are dealing with a simple array. There are no methods that make it easy to work with multiple objects of the Header class. On the other hand, there are plenty that make it easier to work with multiple values in the Header class.

As you said, the PSR-7 seems to implement it just like we do right now: https://www.php-fig.org/psr/psr-7/#headers-with-multiple-values

Note: Not all header values can be concatenated using a comma (e.g., Set-Cookie). When working with such headers, consumers of MessageInterface-based classes SHOULD rely on the getHeader() method for retrieving such multi-valued headers.

In our case, it would be:

$this->request->header('NAME')?->getValue();

[kenjis]

If $this->request->header('NAME')?->getValueLine() works now, it will work.

        $this->message->setHeader('accept', ['json', 'html']);
        $this->message->appendHeader('Accept', 'xml');
        dd($this->message->header('accept')?->getValueLine());
        // "json, html, xml"

But the following code does not make sense, because we cannot use logged_in=no; Path=/, sessid=123456; Path=/.

        $this->message->appendHeader(
            'Set-Cookie',
            'logged_in=no; Path=/'
        );
        $this->message->appendHeader(
            'Set-Cookie',
            'sessid=123456; Path=/'
        );
        dd($this->message->header('Set-Cookie')?->getValueLine());
        // string (43) "logged_in=no; Path=/, sessid=123456; Path=/"

In PSR-7, we also cannot use getHeaderLine() as you pointed out.

require __DIR__ . '/vendor/autoload.php';

$response = new \Laminas\Diactoros\Response();

$cookie1 = 'user_id=123; Path=/; Expires='
    . gmdate('D, d M Y H:i:s T', strtotime('+1 day'));
$cookie2 = 'session_token=abc123; Path=/; Secure; HttpOnly; SameSite=None; Expires='
    . gmdate('D, d M Y H:i:s T', strtotime('+7 days'));

$response = $response->withAddedHeader('Set-Cookie', $cookie1);
$response = $response->withAddedHeader('Set-Cookie', $cookie2);

var_dump($response->getHeader('Set-Cookie'));
var_dump($response->getHeaderLine('Set-Cookie'));

$ php test.php 
array(2) {
  [0] =>
  string(58) "user_id=123; Path=/; Expires=Wed, 15 Nov 2023 05:09:03 GMT"
  [1] =>
  string(100) "session_token=abc123; Path=/; Secure; HttpOnly; SameSite=None; Expires=Tue, 21 Nov 2023 05:09:03 GMT"
}
string(159) "user_id=123; Path=/; Expires=Wed, 15 Nov 2023 05:09:03 GMT,session_token=abc123; Path=/; Secure; HttpOnly; SameSite=None; Expires=Tue, 21 Nov 2023 05:09:03 GMT"

[kenjis]

After all, our question is:
How do we get and forward all headers with CURLRequest (and Response)?
See #5041

[michalsn]

Yes... you are right. I clearly missed a lot. There is no other way around it.

However, I still see some issues.

If $this->request->header('NAME')?->getValueLine() works now, it will work.

And that's the problem. If we call:

$this->request->addHeader('NAME', 'value1');
$this->request->addHeader('NAME', 'value2');

then it will not work. We will receive an error when we try to use it. This should be at least documented in the changelog. Or even better handled just like you showed in Laminas.

// Edit
The very last sentence obviously applies to the $this->request->getHeaderLine('NAME').

[kenjis]

Thanks. To get multiple headers and make them into one line does not make sense.
So it should cause an exception. I did so.

[michalsn]

Hopefully, others can wave in with a comment about this.
Seems like only a changelog/docs is missing.

[MGatner]

I agree with the thread around the confusion and I think this is probably the best that we can do. I'm not entirely sure PSR-7 handles this correctly but that's our guide.

[kenjis]

PSR-7 cannot handle multiple headers with the same name correctly.

[kenjis]

Rebased and added docs.