improve `MessageTrait::SetHeaders()` about psr7 HOT 6 CLOSED

Hey @vicb, long time no see. How are you doing?

I don't see how you want to improve setHeaders actually. withAddedHeader and withHeader keep immutability. And setHeaders doesn't and it is basically only used in the constructor. Can you create a PR with what you had in mind?

from psr7.

@Tobion I'm doing very, working on the client side ATM - happy to hear about / see some of the PHP community members from time to time.

I mean SetHeaders should not concat but "Set" the header

So something along the line

    public function withHeader($header, $value)
    {
        return $this->withHeaderClone($header, $value, /* clone */ true);
    }

    private function setHeaders(array $headers)
    {
        foreach ($headers as $header => $value) {
            $this->withHeaderClone($header, $value, /* clone */ false);
        }
    }

An other thing I notice (actually why I look at the code first) is that:

// sends Content-Type: b
->withHeader('content-type', 'a')
->withHeader('Content-Type', 'b')

// sends content-type: b
->withHeader('Content-Type', 'a')
->withHeader('content-type', 'b')

It seems weird to me that we use the last name - and the spec is not deterministic on this point.
why the last, why not the first ?

I was looking at node.js and it seems that it normalizes the names

/ref angular/angular#11624

from psr7.

A possible refactoring would always mean to introduce more methods to the trait, which makes it slightly slower and adds more extension points that need to be maintainted (because you cannot hide methods in a trait). I think that is way worse than a few lines of duplicated code.

It seems weird to me that we use the last name - and the spec is not deterministic on this point.

True, AFAIK it's not clearly specified. But I think it makes more sense the way it is. There might a use-case to actually change the header case because of legacy clients or whatever. So this allows to do that. If you really need to keep the original case is kinda strange because that would mean you rely on the fact that the header has been specified before at all. So either you care about the header case, then you write it the way you want. Or you don't care (because it shouldn't matter), then you also do not care what the case was before.

from psr7.

A possible refactoring would always mean to introduce more methods to the trait, which makes it slightly slower and adds more extension points that need to be maintainted (because you cannot hide methods in a trait). I think that is way worse than a few lines of duplicated code.

Couldn't you call a function outside of the trait (oh... long time no PHP...)

But I think it makes more sense the way it is

The original bug report on Angular could be triggered with

->withHeader('authorization', 'b') // Apache will ignore this

so I think normalization is slighty less worse...

from psr7.

Well, automatic normalization is a completely different topic and definitely not implied in PSR-7.

Couldn't you call a function outside of the trait

That would be an extension point as well.

from psr7.

Makes sense, closing then.

from psr7.