Hi #1

Closed
wants to merge 3 commits into
from

Projects

None yet

3 participants

@vkamensky

Please review my commit which fixing the following situation: script is crashing in case of pequest->params is missing.

@tonyg
Owner

Why do you want this change? Wouldn't it be just as easy to pass in [] where required instead of undefined?

This 'undefined' is provided by rfc4627_jsonrpc_http:parse_jsonrpc/5 when no params are specified in POST request:
https://github.com/mrspark/erlang-rfc4627/blob/9fb45432f9bc274690694f0a6933e354b3545955/src/rfc4627_jsonrpc_http.erl#L151
We may fix it there then.

Owner

Hmm, interesting. Reading the 1.1 JSON-RPC spec, it's hard to know what to do. So first off, I think you're right to fix it in rfc4627_jsonrpc_http:parse_jsonrpc/5, but the question then becomes: what should be done? My guess is that absent parameters should be treated like [], exactly as you suggested in this commit. The spec says that params is optional, but then says that if it has a value other than an array or object, the server must reject the request with an error, which seems a bit contradictory.

Let's then support parameterless invocations. Fixed in mrspark/erlang-rfc4627@0080fdd

Owner

Thanks! Applied.

@tonyg
Owner

By the way, thank you for the changes! Most appreciated. I've merged the last two from this list; if you'd like to argue for the first one ("fixing empty params error"), please open another pull request with it. Thanks again.

@tonyg tonyg closed this Oct 17, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment