Skip to content

[9.x] do not resolve already set headers#42760

Merged
taylorotwell merged 1 commit intolaravel:9.xfrom
rodrigopedra:9.x
Jun 15, 2022
Merged

[9.x] do not resolve already set headers#42760
taylorotwell merged 1 commit intolaravel:9.xfrom
rodrigopedra:9.x

Conversation

@rodrigopedra
Copy link
Copy Markdown
Contributor

Currently Illuminate\Filesystem\FilesystemAdapter@response, will implicitly calculate the Content-Type, Content-Length, and Content-Disposition default headers values, even when the user already provided those headers.

Also, the calculated default values end up not being used, as the default headers are merged to the user provided headers using the + operator, which preserve keys already present.

$response->headers->replace($headers + [
    'Content-Type' => $this->mimeType($path),
    'Content-Length' => $this->size($path),
    'Content-Disposition' => $disposition,
]);

As outlined in issue #42758 , this can create an issue, for example, when a developer is trying to send a custom file extension, even when they provide a custom Content-Type header, as the FilesystemAdapter@mimeType method would be called anyways.

This PR:

  • Checks if each of the Content-Type, Content-Length, and Content-Disposition headers are not present before calculating their corresponding default
  • Adds test cases to ensure the methods are not called when those headers are provided

Note

I previouslu sent PR #42759 to the 8.x branch, but after further inspection as 8.x branch used a previous version of league/flysystem it doesn't thrown an error on an unknown mime-type.

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.

2 participants