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

[tomsaleeba]

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:

     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)
     }

2. write a unit test that uses this custom response
3. 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.

[mikermcneil]

@tomsaleeba This makes sense, thanks for the heads up! If you have a moment to send a PR, I'll review and merge

[tomsaleeba]

@mikermcneil here's your PR: https://github.com/balderdashy/sails/pull/4745

[johnabrams7]

@tomsaleeba Your PR has been merged! Thanks again. 👍

[mikermcneil]

(leaving it open until it's published)