zendframework / zend-http

Http component from Zend Framework
BSD 3-Clause "New" or "Revised" License
134 stars 85 forks source link

Zend\Http\Headers::addHeaderLine() doesn't check MultipleHeaderInterface #196

Open FalkHe opened 5 years ago

FalkHe commented 5 years ago

Adding a Header not implementing MultipleHeaderInterface via addHeaderLine, will add the Header multiple times. I'd expect it to be/stay unique.

Background: I'm trying to overwrite an already (multiple times) set ContentType Header. Because the Header is set twice via addHeaderLine(), it is present multiple times. But via Headers->get('Content-Type') i only get the first instead of all Content-Type headers. Of cause: because it's not implementing MultipleHeaderInterface. :/

In general i'd expect addHeader() and addHeaderLine() to behave the same way. Or am I wrong? ... Of cause Instanciating the Header for the multiple-check would destrory the "lazy-loading" argument in the docs. :/

If i'm wrong, all ZF-internal Header modifications should use addHeader() instead of addHeaderLine() to prevent this behaviour. In this particular Case Zend\View\Strategy\JsonStrategy adds a Content-Type Header via addHeaderLine() and therefore doesn't re-use/overwrite an already set ContentType Header. May be, this is a bug in zend-view, too.

I'd prefer to implement the "multiple-check" into addHeaderLine().

I'd like to contribute some code but i'm nut sure, what the best/right solution is. ... any thoughts/suggestion?

Code to reproduce the issue

$headers = new Headers();
$headers->addHeader(new ContentType(null, 'text/plain')); // i.e. in Controller
$headers->addHeaderLine('Content-Type', 'text/javascript'); // i.e. in JsonStrategy:134
echo $headers->count().PHP_EOL; 
echo $headers->get('Content-Type)->getMediaType(); 

Expected results

1
text/javascript

Actual results

2
text/plain
weierophinney commented 4 years ago

This repository has been closed and moved to laminas/laminas-http; a new issue has been opened at https://github.com/laminas/laminas-http/issues/2.