Fix null id in RPC response during startup #6362

pull forrestv wants to merge 1 commits into bitcoin:master from forrestv:fixrpcidstartup changing 1 files +7 −7

forrestv commented at 1:56 AM on July 2, 2015: contributor

When processing RPC commands during warmup phase, parse the request object before returning an error so that id value can be used in the response.

Prior to this commit, RPC commands sent during Bitcoin's warmup/startup phase were responded to with a JSON-RPC error with an id of null, which violated the JSON-RPC 2.0 spec:

id: This member is REQUIRED. It MUST be the same as the value of the id member in the Request Object. If there was an error in detecting the id in the Request object (e.g. Parse error/Invalid Request), it MUST be Null.

When processing RPC commands during warmup phase, parse the

request object before returning an error so that id value can
be used in the response.

Prior to this commit, RPC commands sent during Bitcoin's
warmup/startup phase were responded to with a JSON-RPC error
with an id of null, which violated the JSON-RPC 2.0 spec:

id: This member is REQUIRED. It MUST be the same as the value
of the id member in the Request Object. If there was an error
in detecting the id in the Request object (e.g. Parse
error/Invalid Request), it MUST be Null.

72b9452b1d

laanwj added the label RPC on Jul 2, 2015
laanwj commented at 11:31 AM on July 2, 2015: member

Good catch. The new place is better in any case because this also catches other invocations of CRPCTable::execute, such as from the GUI.

Tested ACK.
jonasschnelli commented at 11:54 AM on July 2, 2015: contributor

utACK. If I'm right, the REST interface does also check if the we are in warmup phase. Because moving the warmup check will also affect REST.
laanwj commented at 3:42 PM on July 2, 2015: member

@jonasschnelli the code is moved from HTTPReq_JSONRPC to CRPCTable::execute, both is downstream from REST/HTTP selection, so RESTS cannot be affected. REST should do its own check and raise its own error, which has nothing to do with JSONRPCError.

Here's the same check for REST: https://github.com/bitcoin/bitcoin/blob/master/src/rest.cpp#L556
jonasschnelli commented at 3:46 PM on July 2, 2015: contributor

Sorry. Right, there are no changes for REST calls.
laanwj merged this on Jul 2, 2015
laanwj closed this on Jul 2, 2015
laanwj referenced this in commit d6db1157bc on Jul 2, 2015
luke-jr referenced this in commit 9d83ef421a on Jan 10, 2016
luke-jr referenced this in commit d60afa17b0 on Jan 10, 2016
zkbot referenced this in commit 9af55822fb on Feb 15, 2017
zkbot referenced this in commit a7cf698873 on Mar 4, 2017
DrahtBot locked this on Aug 16, 2022

Contributors

Labels