tuupola / cors-middleware

PSR-7 and PSR-15 CORS middleware
MIT License
132 stars 16 forks source link

Issue with comma-separated headers value in MS Edge #40

Closed morgan-tr closed 5 years ago

morgan-tr commented 5 years ago

Microsoft Edge requires the access-control-expose-headers headers to be lowercase and have no spaces between the comma-separated values. The CORS-middleware package (v0.5.2) automatically adds spaces to the the values being returned in the response headers.

This results in an issue preventing JavaScript to read the exposed headers when the browser is MS Edge.

Reference to the issue: https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/12046299/

Edit: fixed the link to the issue.

tuupola commented 5 years ago

Seems the link requires registration?

tuupola commented 5 years ago

Probably related:

https://github.com/neomerx/cors-psr7/issues/31

https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/10584749/ https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/12046299/

tuupola commented 5 years ago

With example code:

$app->add(new Tuupola\Middleware\CorsMiddleware([
    "headers.expose" => ["Content-Length", "Etag", "X-Foo"],
]));

Current behaviour with latest release (1.0.0) is separate headers

$ curl --include http://0.0.0.0:8080  \
     --request PUT \
     --include \
     --header "Origin: http://www.example.com"

HTTP/1.1 200 OK
Host: 0.0.0.0:8080
Date: Wed, 02 Oct 2019 09:21:18 GMT
Connection: close
X-Powered-By: PHP/7.3.9
Access-Control-Allow-Origin: http://www.example.com
Vary: Origin
Access-Control-Expose-Headers: Content-Length
Access-Control-Expose-Headers: Etag
Access-Control-Expose-Headers: X-Foo
Content-type: text/html; charset=UTF-8

Apparently Edge cannot handle multiple headers. After PR #42 values are combined into one header.

curl --include http://0.0.0.0:8080  \
     --request PUT \
     --include \
     --header "Origin: http://www.example.com"
HTTP/1.1 200 OK
Host: 0.0.0.0:8080
Date: Wed, 02 Oct 2019 09:22:48 GMT
Connection: close
X-Powered-By: PHP/7.3.9
Access-Control-Allow-Origin: http://www.example.com
Vary: Origin
Access-Control-Expose-Headers: Content-Length, Etag, X-Foo
Content-type: text/html; charset=UTF-8

Could you check if this fixes your problem?

$ composer require tuupola/cors-middleware:dev-edge-bug-40
tuupola commented 5 years ago

@morgan-tr Did this fix the problem you had with MS Edge?

morgan-tr commented 5 years ago

@tuupola First of all, thank you for all the responses and I really apologize for not commenting on this thread sooner. I actually didn't see the updates in my inbox until now.

I'll ask my colleague, who's the developer in charge of this, to test your code sample, and will report back.

Thanks again,

Morgan

morgan-tr commented 5 years ago

@tuupola One more comment about the fix you've introduced - we've yet to test it, but I'm not sure if the it would work.

Your example shows that the new expose header would be: Access-Control-Expose-Headers: Content-Length, Etag, X-Foo

However, Edge requires the header to be in the following format: Access-Control-Expose-Headers: content-length,etag, x-foo

That is, lowercase, no spaces between the values.

In any case, we'll give it a try.

tuupola commented 5 years ago

Middleware does not alter the parameters which are passed in. To get lowercase header values you can use:

$app->add(new Tuupola\Middleware\CorsMiddleware([
    "headers.expose" => ["content-Length", "etag", "x-foo"],
]));

The space thing I missed for some reason. Will fix.

morgan-tr commented 5 years ago

@tuupola Thanks, removing the spaces should do the trick.

tuupola commented 5 years ago

Released as 1.1.1.