enforce header value as string ? about psr7 HOT 9 CLOSED

I wouldn’t mind throwing an exception for non-string values, the same way we do in withHeader:

In essence I guess this is what is happening now, when the anonymous function within trimHeaderValues throws a TypeError. But it may be better if we throw it ourselves?

Did you prefer any specific option, @ncou?

from psr7.

yes it may be better to throw it ourself, the controls are already presents in the withHeader and withAddedHeader so the controls should also be applyed on the setHeaders(), IMHO.

perhaps you could try to add a new function to do the checks, something like : "assertHeaderValues(string $header, array $values)" and this function could do the following asserts/verifications :

        if (empty($values)) {
            throw new \InvalidArgumentException('Header values must be non-empty strings');
        }
        if (!is_string($header) || empty($header)) {
            throw new \InvalidArgumentException('Header name must be non-empty strings');
        }
        foreach ( array_values($values) as $value) {
            if (!is_string($value)) {
                throw new \InvalidArgumentException('Header values must be non-empty strings');
            }
        }

and you keep the convertion from string to array for the value variable before calling this assert method. and after you call the trimHeaderValues. This new function could be used/called in setHeaders() / withHeader() and withAddedHeader() and avoid some duplication code.

What do you think ?

from psr7.

@Zegnat : so what is your opinion ?

from psr7.

I do think we should use the same exception throwing in those 3 methods, for sure.

I am ambivalent on creating an extra private method for it. It is good DRY to abstract it. On the other hand, I like it when I am looking at an exception and it originates from the method I called rather than having to go further up in the stack trace. But that is a personal quirk.

from psr7.

@Nyholm : and your opinion ?

from psr7.

no news ?

from psr7.

Sorry for being late on replying.
I agree with Martin, lets duplicate the exception throwing for now.

I agree that doing new Response(200, ['Age' => 60]); should throw an exception just like (new Response(200))->withHeader('Age', 60).

Could you send a PR?

from psr7.

When I closer inspected this. We use the same validation 3(!) times. I used a separate function. See #48

from psr7.

Wohoo. This is all fixed now.

from psr7.