Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

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

Open
2 tasks done
FalkHe opened this issue Oct 24, 2019 · 1 comment
Open
2 tasks done

Comments

@FalkHe
Copy link

FalkHe commented Oct 24, 2019

  • I was not able to find an open or closed issue matching what I'm seeing.
  • This is not a question. (Questions should be asked on chat (Signup here) or our forums.)

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
Copy link
Member

This repository has been closed and moved to laminas/laminas-http; a new issue has been opened at laminas/laminas-http#2.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants