Default content-type being set with 'json: true' breaks code

[addisonj]

With 2.63.0 and #1772, request re-introduces subtle breaking behavior. It is considered bad HTTP to send content-types with requests without a body. For example, I just had breaking behavior where I use HMAC authentication to sign API requests and then send that signature with request. Since the content-type is included in the signature and now I get a content-type that wasn't expected, I get signatures that don't match and everything goes kaboom... I can code around it, but I don't think I should have too.

I also am pretty sure this is actually a regression. I don't have time to do the bug search right now, but years ago, I remember this broke against things like express (in like the 2.x days) where a content-type that was sent without a body would result in an error. While it was also an express bug, it was determined that setting a content-type without a body was bad form. If you need that behavior, it should be opt in, not the default where these headers matter. While the docs seem to imply it, it seems much more like a doc bug than anything.

Either way, changes to the default behavior or headers is a big enough change that it needs to done with a bit more caution, perhaps even a major version bump. I would vote for reverting that change, as I think it will probably break for a lot more people.

[adamnreed]

I also dealt with this issue today. 2.63.0 is marked as compatible with 2.44.0. In my case, not true, this broke for me.

[paul-english]

If this is the preferred behavior, we might want a test to ensure there is no content-type when a request has no body.

[simov]

I can't find anywhere in the spec explicitly stated that requests without body, are not allowed to have a content-type set. On the other hand I can agree that it doesn't make sense to have a content-type without having a body.

The problem here comes from the json option itself. Either @jzaefferer should set its content-type header, or @addisonj should set the accept header and parse the response instead of using the json option.

Since this happens to be a breaking change, I'm more on the reverting it side + fixing the docs and adding a test. @jzaefferer thoughts?

[jzaefferer]

I'm okay with a revert; I'll just revert my project's upgrade commit as well. As long as the docs are more clear about this than before.

[xytis]

+1 to the list of people who had their code broken :)

[simov]

Fixed here #1785 I wasn't able to fix the docs though.

In fact they are totally correct as they are, except that the json:true case is completely missing. If you can figure out a good and short way to explain the behavior contained in the json function let me know in the other issue.

[simov]

2.64 is published 🎉