balderdashy / balderdashy/sails
lib/router/res.js overwrites content-type header, when Express doesn't
- Dominant language
- JavaScript
- Stars
- 22.8k
- Forks
- 1.9k
- PR merge metrics
- No merged PRs in 30d
Description
**Sails version**: 1.0.3-2
**Node version**: 8.10
**NPM version**: 5.6.0
**DB adapter name**: sails-disk
**DB adapter version**: 2.0.1 (part of sails-hook-orm)
**Operating system**: Linux Mint 18.3
Steps:
1. write a custom response that calls set the `content-type` header and returns a JS object *not a string*, something like:
```js
function customOkResponse(optionalData) {
const res = this.res
res.set('content-type', 'application/vnd.example.v1+json')
if (optionalData === undefined) {
return res.sendStatus(200)
}
return res.status(200).send(optionalData)
}
````
1. write a unit test that uses this custom response
1. assert that the `content-type` header is what you set
Expected:
The header comes through as you set it.
Actual:
Our header gets overwritten because we returned a JS object, not a string, and https://github.com/balderdashy/sails/blob/635ec44316f797237019dfc5b1e14b8085eb960f/lib/router/res.js#L264 overwrote it.
The work around is to stringify the response in the custom response, like `JSON.stringify(optionalData)`.
The point is that Express doesn't behave like this. I can send a JS object while setting `content-type` and it *works*. Is it worth modifying `res.js:264` to only set `content-type` only when it's not already set?
I'm happy to provide a concrete example of this running or send a PR with the fix. Just wanted to discuss it first.
Contributor guide
Research direction
Start in lib/router/res.js at the linked line around 264 and inspect how the content-type header is set when a response returns a JavaScript object. Add or update a unit test using the custom response steps in the issue, then verify that an explicitly set content-type is preserved for object responses, matching Express behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- express, javascript
- Domain
- api, backend
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 55/100