Skip to content
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
18 changes: 14 additions & 4 deletions src/execute.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,6 @@ export function buildRequest({
req.headers.accept = responseContentType
}

if (requestContentType) {
req.headers['content-type'] = requestContentType
}

// Add values to request
arrayOrEmpty(operation.parameters) // operation parameters
.concat(arrayOrEmpty(spec.paths[pathName].parameters)) // path parameters
Expand Down Expand Up @@ -127,6 +123,20 @@ export function buildRequest({
req = applySecurities({request: req, securities, operation, spec})
// Will add the query object into the URL, if it exists
mergeInQueryOrForm(req)

if (req.body || req.form) {
if (requestContentType) {
req.headers['content-type'] = requestContentType
} else if (Array.isArray(operation.consumes)) {
req.headers['content-type'] = operation.consumes[0]
} else if (Array.isArray(spec.consumes)) {
req.headers['content-type'] = spec.consumes[0]
} else if (operation.parameters.filter((p)=> p.type === "file").length) {
req.headers['content-type'] = "multipart/form-data"
} else if (operation.parameters.filter((p)=> p.in === "formData").length) {
req.headers['content-type'] = "application/x-www-form-urlencoded"
}
}
return req
}

Expand Down
137 changes: 133 additions & 4 deletions test/execute.js
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ describe('execute', () => {
})
})

it('should handle requestContentType', function () {
it('should not add content-type with no form-data or body param', function () {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ponelat this test is about not to set content type

Copy link
Contributor

Choose a reason for hiding this comment

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

ok... but this test includes a requestContentType. If that is present, it should always set the header.

Copy link
Contributor

Choose a reason for hiding this comment

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

With the issue in swagger-api/swagger-ui#2909, there is likely a requestContentType being passing in from swagger-ui. And that shouldn't happen

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here https://github.com/swagger-api/swagger-js/pull/1016/files#diff-2932c1430f43e1d22b8cf8b98fa5d2a8R127 first we check if there is body or form and only after that - if there is content-type passed from UI

Copy link
Contributor

Choose a reason for hiding this comment

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

I see that. But we shouldn't 😛 if there is a requestContentType we must use it. Its an override provided by the user ( or swagger-ui in our case ).

Copy link
Contributor

Choose a reason for hiding this comment

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

you're right @bodnia it does only make sense if there is a body. The user can override the content-type header in other ways, if need be IIRC.

// Given
const spec = {
host: 'swagger.io',
Expand All @@ -454,14 +454,143 @@ describe('execute', () => {
// Then
expect(req).toEqual({
url: 'http://swagger.io/one',
headers: {
'content-type': 'application/josh',
},
headers: {},
credentials: 'same-origin',
method: 'GET'
})
})

it('should add content-type multipart/form-data when param type is file and no other sources of consumes', function () {
// Given
const spec = {
host: 'swagger.io',
paths: {
'/one': {
post: {
operationId: 'postMe',
parameters: [{name: 'file', type: 'file', 'in': "formData"}]
}
}
}
}

// When
const req = buildRequest({
spec,
operationId: 'postMe',
parameters: { file: 'test'}})

// Then
expect(req).toEqual({
method: "POST",
"body": "file=test",
url: 'http://swagger.io/one',
headers: {
"content-type": "multipart/form-data"
},
credentials: 'same-origin'
})
})

it('should add content-type application/x-www-form-urlencoded when in: formData ', function () {
// Given
const spec = {
host: 'swagger.io',
paths: {
'/one': {
post: {
operationId: 'postMe',
parameters: [{name: 'file', in: 'formData'}]
}
}
}
}

// When
const req = buildRequest({
spec,
operationId: 'postMe',
parameters: { file: 'test'}})

// Then
expect(req).toEqual({
body: "file=test",
method: "POST",
url: 'http://swagger.io/one',
headers: {
"content-type": "application/x-www-form-urlencoded"
},
credentials: 'same-origin'
})
})

it('should add content-type from spec when no consumes in operation and no requestContentType passed', function () {
// Given
const spec = {
host: 'swagger.io',
consumes: ["test"],
paths: {
'/one': {
post: {
operationId: 'postMe',
parameters: [{name: 'file', in: 'formData'}]
}
}
}
}

// When
const req = buildRequest({
spec,
operationId: 'postMe',
parameters: { file: 'test'}})

// Then
expect(req).toEqual({
body: "file=test",
method: "POST",
url: 'http://swagger.io/one',
headers: {
"content-type": "test"
},
credentials: 'same-origin'
})
})

it('should add content-type from operation when no requestContentType passed', function () {
// Given
const spec = {
host: 'swagger.io',
consumes: ["no"],
paths: {
'/one': {
post: {
operationId: 'postMe',
consumes: ["test"],
parameters: [{name: 'file', in: 'formData'}]
}
}
}
}

// When
const req = buildRequest({
spec,
operationId: 'postMe',
parameters: { file: 'test'}})

// Then
expect(req).toEqual({
body: "file=test",
method: "POST",
url: 'http://swagger.io/one',
headers: {
"content-type": "test"
},
credentials: 'same-origin'
})
})

it('should build a request for all given fields', function () {
// Given
const spec = {
Expand Down