Skip to content
This repository has been archived by the owner on Apr 4, 2022. It is now read-only.

Fix #119: Ensure a multipart Content-Type header is used when sending attachments. #120

Merged
merged 2 commits into from
Jul 19, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/collection.js
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,10 @@ export default class Collection {
const {permissions} = reqOptions;
const id = record.id || uuid.v4();
const path = endpoint("attachment", this.bucket.name, this.name, id);
const addAttachmentRequest = requests.addAttachmentRequest(path, dataURI, {data: record, permissions}, reqOptions);
const addAttachmentRequest = requests.addAttachmentRequest(path, dataURI, {
data: record,
permissions
}, reqOptions);
return this.client.execute(addAttachmentRequest, {stringify: false})
.then(() => this.getRecord(id));
}
Expand Down
5 changes: 5 additions & 0 deletions src/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ export default class HTTP {
let response, status, statusText, headers, hasTimedout;
// Ensure default request headers are always set
options.headers = {...HTTP.DEFAULT_REQUEST_HEADERS, ...options.headers};
// If a multipart body is provided, remove any custom Content-Type header as
// the fetch() implementation will add the correct one for us.
if (options.body && typeof options.body.append === "function") {
delete options.headers["Content-Type"];
}
Copy link
Contributor

@leplatrem leplatrem Jul 19, 2016

Choose a reason for hiding this comment

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

It is not worse than the previous undefined we had...

options.mode = this.requestMode;
return new Promise((resolve, reject) => {
const _timeoutId = setTimeout(() => {
Expand Down
2 changes: 0 additions & 2 deletions src/requests.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,6 @@ export function addAttachmentRequest(path, dataURI, {data, permissions}={}, opti
headers: {
...headers,
...safeHeader(safe, last_modified),
// Setting content type as undefined so that it does not use default "application/json".
"Content-Type": undefined
},
body: formData
};
Expand Down
9 changes: 9 additions & 0 deletions test/http_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,15 @@ describe("HTTP class", () => {

expect(fetch.firstCall.args[1].headers.Foo).eql("Bar");
});

it("should drop custom content-type header for multipart body", () => {
http.request("/", {
headers: {"Content-Type": "application/foo"},
body: new FormData()
});

expect(fetch.firstCall.args[1].headers["Content-Type"]).to.be.undefined;
});
});

describe("Request CORS mode", () => {
Expand Down
8 changes: 4 additions & 4 deletions test/integration_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -991,7 +991,7 @@ describe("Integration tests", function() {
beforeEach(() => {
return coll
.addAttachment(dataURL, {foo: "bar"}, {
permissions: {write: ["github:n1k0"]}
permissions: {write: ["github:n1k0"]},
})
.then(res => result = res);
});
Expand All @@ -1017,8 +1017,9 @@ describe("Integration tests", function() {
});

describe("Without filename", () => {
const dataURL = "data:text/plain;base64," + btoa("blah");

it("should default filename to 'untitled' if not specified", () => {
const dataURL = "data:text/plain;base64," + btoa("blah");
return coll
.addAttachment(dataURL)
.should.eventually
Expand All @@ -1028,9 +1029,8 @@ describe("Integration tests", function() {
});

it("should allow to specify a filename in options", () => {
const dataURL = "data:text/plain;base64," + btoa("blah");
return coll
.addAttachment(dataURL, {}, {filename: "MYFILE.DAT"})
.addAttachment(dataURL, undefined, {filename: "MYFILE.DAT"})
.should.eventually
.have.property("data")
.have.property("attachment")
Expand Down
2 changes: 1 addition & 1 deletion test/requests_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ describe("requests module", () => {

it("should accept a headers option", () => {
expect(requests.addAttachmentRequest("/foo", dataURL, {}, {headers: {Foo: "Bar"}}))
.to.have.property("headers").eql({"Content-Type": undefined, Foo: "Bar"});
.to.have.property("headers").eql({Foo: "Bar"});
});

it("should raise for safe with no last_modified passed", () => {
Expand Down