balderdashy / balderdashy/sails

lib/router/res.js overwrites content-type header, when Express doesn't

Open
#4,445 4 comments 0 reactions 0 assignees View on GitHub
has pr inconsistency resolved
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.