json: true doesn't set Content-Type

[jzaefferer]

Documentation for the json options says:

json - sets body but to JSON representation of value and adds Content-type: application/json header. Additionally, parses the response body as JSON.

This doesn't work for json: true:

var request = require('request')
var x = request({
  uri: 'http://whatever',
  json: true
})
// FIXME this should include `content-type: application/json`
console.log(x.headers)
// > { host: 'whatever', accept: 'application/json' }

The actual implementation also checks for self.body:

if (typeof val === 'boolean') {
  if (self.body !== undefined && self.getHeader('content-type') !== 'application/x-www-form-urlencoded') {
    self.body = safeStringify(self.body)
    if (!self.hasHeader('content-type')) {
      self.setHeader('content-type', 'application/json')
    }
  }
}

I'm dealing with an API that expects a Content-Type: application/json even for a DELETE request without a body, so the existing implementation doesn't work here. I'm working around that by explicitly setting the header on every json: true request, of which there are many littered through this application.

I suggest changing the implementation to make the !self.hasHeader('content-type') check independent of the self.body check, so if val is a boolean, try to set the header, ignoring the missing body.

If that's acceptable, I can look into creating a patch, at least the implementation change should be straight forward.

[simov]

Yep, sounds logical. Just move it before the body check. 3 tests are failing in this case, but they can be considered incorrect in this case.

Also can you add a test for this behavior, just to make sure we won't brake it in future.

[jzaefferer]

Thanks @simov. I've created a pull request, #1772, that makes the change and updates tests. I think the existing tests cover the usecase well enough, so I didn't add a new one.