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

[11.x] Ensure headers are only attached to illuminate responses #52789

Merged
merged 1 commit into from
Sep 16, 2024

Conversation

timacdonald
Copy link
Member

The base symfony response does not have the header method. This can cause issues when you are returning a download, which uses the Symfony\Component\HttpFoundation\BinaryFileResponse class.

I think we can safely only attach the Vite headers to Illuminate responses considering this is the first report of the issue.

fixes #52784

@Jubeki
Copy link
Contributor

Jubeki commented Sep 15, 2024

Maybe adding a test, that it also is not attached to RedirectResponse or JsonResponse would also be good. (They both don't extend the illuminate response class, but a Symfony based response class). These both classes don't need any vite assets in my opinion, because they should not render a viewable html response.

But if you are talking about an illuminate response, I would assume you are including RedirectResponse and JsonResponse too.

@taylorotwell taylorotwell merged commit 1c52ec4 into laravel:11.x Sep 16, 2024
33 checks passed
@timacdonald timacdonald deleted the vite-fix branch September 18, 2024 23:20
@devinfd
Copy link
Contributor

devinfd commented Oct 2, 2024

Can this bug fix be ported into 10.x as well?

@timacdonald
Copy link
Member Author

Can do #53019

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

Successfully merging this pull request may close these issues.

Using @vite in a view that is rendered but a file response is sent back causes an exception
4 participants