Sending boolean as body for JSON request not working properly

[apoco]

When trying to send a JSON boolean value in a request:

request({
    method: 'PUT',
    uri: someUrl',
    json: true,
    body: true
})

...I get this error:

TypeError: First argument needs to be a number, array or string.
    at new Buffer (buffer.js:188:15)
    at Request.init (/home/jacob/Code/request/request.js:596:21)
    at new Request (/home/jacob/Code/request/request.js:282:8)
    at request (/home/jacob/Code/request/index.js:50:10)
    at Test.<anonymous> (/home/jacob/Code/request/tests/test-json-request.js:24:3)
    at Test.bound [as _cb] (/home/jacob/Code/request/node_modules/tape/lib/test.js:59:32)
    at Test.run (/home/jacob/Code/request/node_modules/tape/lib/test.js:72:10)
    at Test.bound [as run] (/home/jacob/Code/request/node_modules/tape/lib/test.js:59:32)
    at Object.next [as _onImmediate] (/home/jacob/Code/request/node_modules/tape/lib/results.js:66:15)
    at processImmediate [as _immediateCallback] (timers.js:330:15)

Is this intentional? Also, is there an easy workaround for this issue?

[simov]

You can try this one out

request({
    method: 'PUT',
    uri: 'someUrl',
    json: {status:true}
})

[apoco]

That would work if I wanted to send a body containing { "status": true }, but not sending a body containing true.

This pull request fixes the issue, and it doesn't break any of the other tests: #1282 . Any thoughts on this change? I'm not sure the intention of the original check.

[lalitkapoor]

I think this is acceptable because of the following: http://en.wikipedia.org/wiki/JSON#Data_types.2C_syntax_and_example

Early versions of JSON (such as specified by RFC 4627) required that a valid JSON "document" must consist of only an object or an array type—though they could contain other types within them. This restriction was relaxed starting with RFC 7158, so that a JSON document may consist entirely of any possible JSON typed value.

[simov]

wow, didn't know about that

[apoco]

I made a slight but important modification to my PR; namely, if the body is null in the request options, a JSON null will not be sent. It seems highly likely that there's a lot of people that expect a null body to mean an empty body.

I kind of don't like this modification, but without the change, some breakage in our application occurred. Part of why we'd need this is that the json option kind of confuses the sending and receiving of JSON.

[lalitkapoor]

I think if we're going to support sending any JSON type, then we can't be biased in deciding which ones we allow. @apoco would it make sense to deal with this in your application instead?

[lalitkapoor]

According to the spec here: http://tools.ietf.org/html/rfc7158#section-3 We'd need to support:

A JSON value MUST be an object, array, number, or string, or one of
the following three literal names:

  false null true

The literal names MUST be lowercase. No other literal names are
allowed.

  value = false / null / true / object / array / number / string

  false = %x66.61.6c.73.65   ; false

  null  = %x6e.75.6c.6c      ; null

  true  = %x74.72.75.65      ; true

[apoco]

Yes, our application could deal with it instead, and I did figure out how to work around it. It maybe should be considered a breaking API change, though. In our case, with these options:

{ json: true, body: null }

...the intent was to send no body, but a null was sent instead. Doing this:

{ json: false, body: null }

... broke response handling; the JSON coming back wasn't parsed. However, this:

{ json: true }

...with body being undefined seemed to work. I don't know how intuitive this is, so maybe it should be considered a breaking change?

[apoco]

I can revert my null handling modification if that's what's wanted for sure.

[apoco]

Reverted.

[lalitkapoor]

I think setting json: true and not sending a body is fine. I don't think that's breaking unless it causes an issue. @apoco does that cause an issue for you (against master, without the code in your pull request)?

[apoco]

Yes, not sending a body and having json: true works fine. Setting body: null is what causes the issue since what was previously an empty body now has content. But that issue is not a bug with request, but an application issue which we can work around. So I reverted my change so that body: null sends a JSON null for consistency's sake.

[lalitkapoor]

@apoco great, I think the test looks. Can you please add checks for all of these:

value = false / null / true / object / array / number / string

That'll guarantee we in compliance w/the spec and put a lid on it.

[rlidwka]

null and undefined should be treated the same imho.

It is true that json allows primitive types, but some modules choose to accept only objects. See strict option in body-parser as an example.

I think current behavior is fair enough. If you want to send non-object json, you can always stringify it yourself.

[apoco]

Done @lalitkapoor; I added tests for all of those types.

[lalitkapoor]

Thanks @apoco. Any objections to me merging #1282 @simov @nylen?

[simov]

I wrote a little comment in #1282 other than than it looks good to me.

As a side note let's keep future PR conversations in the PR itself, right now it feels kind of detached (from the actual commits)

[lalitkapoor]

@simov agreed.

[simov]

Merged see #1282 for details

[sumeetsolse]

{
"fullName": "Ganesh Gonde",
"gender": false,
"dob": "1985-11-15",
"mobileNumber": 98959000321,
"tblMstHobbyDetails": []
}

[Naveenpatel45]

why cant i filter with boolean in json .