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.

Sign up for GitHub

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

Jump to bottom

Fixing overwrite Blob/File type as Content-Type in browser. #1773

Merged
merged 4 commits into from May 28, 2020
Merged

Fixing overwrite Blob/File type as Content-Type in browser. #1773

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1b33e70
Fixing overwrite Blob/File type as Content-Type in browser.
Gerhut Jun 29, 2018
5d496e1
Merge branch 'master' into content-type
Gerhut Jan 20, 2020
0c88769
Merge branch 'master' into content-type
Gerhut May 27, 2020
cc177b2
Merge branch 'master' into content-type
jasonsaayman May 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions lib/adapters/xhr.js
View file
Open in desktop
Expand Up @@ -18,6 +18,13 @@ module.exports = function xhrAdapter(config) {
delete requestHeaders['Content-Type']; // Let the browser set it
}

if (
(utils.isBlob(requestData) || utils.isFile(requestData)) &&
requestData.type
) {
delete requestHeaders['Content-Type']; // Let the browser set it
}
Comment on lines +21 to +26
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks allowing the developer to explicitly set the Content-Type header. For example, if I need to send Content-Type of application/octet-stream for a PNG image, the browser overrides this and instead sends image/png.

For a use-case, this breaks sending attachments to the Microsoft Azure Rest API, which requires a Content-Type of application/octet-stream

Copy link
Contributor Author

@Gerhut Gerhut Sep 10, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @adamdehaven, thanks for the report.

Although I think this is a breaking change and not sure why this was landed in a minor version (just missed the release version, I think a major version release is OK for this), in your case, I think actually you need to build a blob with type application/octet-stream, which could be done by something like blob = blob.slice(0, blob.size, newType)

Here is a codepen: https://codepen.io/Gerhut/pen/YzqaOQY?editors=0012

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why I'd need to build a blob instead when this worked fine in v0.19.x?

Why delete the Content-Type value set in the code by the dev? If it's being explicitly set, that value should be respected. Axios should not "decide" which one to use.

Copy link
Contributor Author

@Gerhut Gerhut Sep 10, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

“Theoretically” speaking, the dev also "set" a content-type in the body if they put a blob in body, which goes after headers and should overwrite the former definition.

Why the body changes header? Because header is used to define the body.

OK, I was just going to remove default content type header for the blob, not thought that there are many users use content type header to overwrite the blob type. I am OK with reverting it now.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Gerhut, when will #3289 be merged? The solution you mentioned in #1773 (comment) is not working for IE11.


var request = new XMLHttpRequest();

// HTTP basic authentication
Expand Down