Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[6.x] Fix for issue Content Type not specified #31533

Merged
merged 1 commit into from Feb 20, 2020
Merged

Conversation

@adam1010
Copy link
Contributor

adam1010 commented Feb 19, 2020

This fixes issue #31518 where Laravel isn't setting a Content-Type and so Symfony is just setting it to whatever the request is asking for. This is poisoning our cache, randomly serving up our website HTML pages as "text/xml" (which causes Chrome rendering errors).

This is a regression caused by a recent update to the underlying Symfony framework, but affects Laravel because we chose not to specify the default Content-Type.

This fixes issue #31518 where Laravel isn't setting a Content-Type and so Symfony is just setting it to whatever the request is asking for.  This is poisoning our cache, serving up our website HTML pages as "XML".
@driesvints

This comment has been minimized.

Copy link
Member

driesvints commented Feb 20, 2020

You can't know that the response is text/html. It could be a binary file, text/plain, literally anything.

@taylorotwell taylorotwell merged commit 3f185c7 into laravel:6.x Feb 20, 2020
2 checks passed
2 checks passed
continuous-integration/styleci/pr The analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@taylorotwell

This comment has been minimized.

Copy link
Member

taylorotwell commented Feb 20, 2020

@driesvints if this is fixing a broken behavior I think we need to do it. It sounds like Symfony was adding this automatically in the past so this is bringing us back to that behavior.

@GrahamCampbell

This comment has been minimized.

Copy link
Member

GrahamCampbell commented Feb 20, 2020

I think Symfony considered it to be a bad idea to automatically set the content type, and actively decided to remove it.

@adam1010

This comment has been minimized.

Copy link
Contributor Author

adam1010 commented Feb 20, 2020

Sure, trying to guess (or assume) the content type isn't the best idea, but it's much better than having the content-type header just echoed back out based on whatever the client asked for when the content itself isn't actually changing.

For endpoints that deliver non-HTML, devs are used to specifying that manually (because until this recent regression, you didn't have to explicitly set text/html)

return response()->view('sitemap')->header('Content-Type', 'text/xml');

or

return response($bytes, 200,
        ['Content-Type' => 'application/octet-stream', 'Content-Disposition' => 'attachment']
  );
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.