zendframework/zend-http

ContentSecurityPolicy headers overwrite each other

markushausammann opened this issue · 6 comments

markushausammann commented
  • 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.)

It's often not allowed or recommended to have several headers with the same name. But there are situations where it is allowed or even necessary. The CSP is one of these.

https://w3c.github.io/webappsec-csp/#multiple-policies

and

https://www.w3.org/TR/CSP2/#content-security-policy-header-field

Imagine a case where a main application creates a CSP and different modules also independently add their own CSPs. The framework MUST render them all separately OR do a preemptive union merge which is what the client would otherwise do. It will (hopefully) quickly become standard practice that modules provide their own CSPs.

Code to reproduce the issue

$headers = $controller->getResponse()->getHeaders();
$headers->addHeader(new ContentSecurityPolicy($someDirectives));
$headers->addHeader(new ContentSecurityPolicy($someOtherDirectives));

Expected results

The expected result is a response with two CSP headers (OR a union merged CSP).

Actual results

The second addition overwrites the first, the response only contains that one CSP.

markushausammann commented

PS: It's not enough to have ContentSecurityPolicy implement MultipleHeaderInterface I think. The framework also needs to make sure to check for existing policies and append them automatically.

markushausammann commented

Also I'm trying to understand the MultipleHeaderInterface and I've tried to add such a multi header string via addHeaderLine but that makes Headers throw an exception on line 192: A field name was provided without a field value.

:-o

markushausammann commented

Am I even in the right repository? Seems this one hasn't really been touched for quite some time.

Ocramius commented

Yeah, the repo is correct, but unless someone picks up a failing test case + patch, it's not gonna be solved. If you can manage to create a failing test, that already simplifies things enormously.

markushausammann commented

Ok, I'll try to find time for something... not that I'm the right person to do that.

Ocramius commented

Yes you are: you know exactly what the bug looks like 👍

  • 👍2
  • 🎉1