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

S3 upload should not set an empty content-type #519

Closed
stof opened this issue Oct 14, 2016 · 9 comments
Closed

S3 upload should not set an empty content-type #519

stof opened this issue Oct 14, 2016 · 9 comments

Comments

@stof
Copy link

stof commented Oct 14, 2016

When the mime type is not known by the mime-types gem, it would be better to set a application/octet-stream mime type than an empty string.

@BanzaiMan
Copy link
Contributor

Thanks for opening the issue. What are effects of not setting the Content-Type header?

@stof
Copy link
Author

stof commented Nov 7, 2016

if you don't set it when uploading, I think S3 automatically sets it to application/octet-stream (which means that browsers will download the file, while an empty content-type gets treated like a text content at least in Firefox and so displayed on screen even when it is not textual)

BanzaiMan added a commit that referenced this issue Nov 9, 2016
If MIME::Types can't figure out the type, skip setting the Content-Type
header altogether.
When downloading, Amazon should specify 'application/octet-stream'.

Resolves #519
@BanzaiMan
Copy link
Contributor

Please test it with:

deploy:
  provider: s3
  edge:
    branch: 's3-content-type'# rest

BanzaiMan added a commit that referenced this issue Nov 9, 2016
If MIME::Types can't figure out the type, skip setting the Content-Type
header altogether.
When downloading, Amazon should specify 'application/octet-stream'.

Resolves #519
@stof
Copy link
Author

stof commented Nov 11, 2016

Actually, it does not set it to application/octet-stream (this may be a feature of s3cmd for unknown types).

But omitting the content-type makes the file be downloaded by Firefox instead of being displayed as text as previously (which was very confusing when the file was not a text file). So I would vote +1 on your fix.

@stale
Copy link

stale bot commented Apr 12, 2018

Thanks for contributing to this issue. As it has been 90 days since the last activity, we are automatically closing the issue. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If that's not the case, please do feel free to either reopen this issue or open a new one. We'll gladly take a look again! You can read more here: https://blog.travis-ci.com/2018-03-09-closing-old-issues

@BanzaiMan
Copy link
Contributor

@stof @sylveon @peternewman You all have expressed interest in resolving this issue. Could you test the PR one last time, so that we can merge and release the fix?

Thanks.

@peternewman
Copy link
Contributor

Not guilty sorry @BanzaiMan aside from pointing out it's a duplicate to try and keep the issue list clean.

@BanzaiMan
Copy link
Contributor

@peternewman Sorry for the noise!

@stale
Copy link

stale bot commented Jan 22, 2019

Thanks for contributing to this issue. As it has been 90 days since the last activity, we are automatically closing the issue. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If that's not the case, please do feel free to either reopen this issue or open a new one. We'll gladly take a look again! You can read more here: https://blog.travis-ci.com/2018-03-09-closing-old-issues

@stale stale bot added the stale label Jan 22, 2019
@stale stale bot closed this as completed Jan 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants