uber-archive / multitransport-jsonrpc

JSON-RPC Client (Node.js & Browser) and Server (Node.js) aim at "natural looking" server and client code.
116 stars 22 forks source link

Making an incorrect request can cause the server to crash. #64

Closed nackjicholson closed 9 years ago

nackjicholson commented 9 years ago

Let's say this is my rpc server.

var jsonRpcHttpServer = new Server(new ServerHttp(8000), {
    add: function(a, b, callback) {
      callback(null, a+b);
    }
});

If I were to have a client post this:

{"jsonrpc":"2.0","method":"add","params":[1, 2]}

Hurray everything works, the world is perfect, no one every sends bad web requests we're done! :metal:

BUT if some super hacker sends a bad request that is missing a param?

{"jsonrpc":"2.0","method":"add","params":[]}

This will always throw a fatal error and crash the server.

What I would expect to be correct behavior is that a and b should become set to undefined because they weren't passed as params. That would be okay... worst case scenario you get a nonsensical response back like null because that's the result of undefined + undefined. But what actually happens is: b and callback become undefined and a becomes the CALLBACK!!!

So when the callback is called you get a Fatal error message telling you undefined is not a function, and the server crashes.

There is an absolutely ridiculous way to get around it, and at least keep the server running.

var jsonRpcHttpServer = new Server(new ServerHttp(8000), {
    add: function(a, b, callback) {
      if (typeof a === 'function') {
        callback = a;
        a = undefined;
      }

      if (typeof b === 'function') {
        callback = b;
        b = undefined;
      }

      callback(null, a+b);
    }
});

I may try forking this project to see if I can dig through the source and see how you're handling the argument passing in order to make a quick fix. If I do find it, I'll make a PR. If not, please consider making a fix. I find this to be my only strong reason so far to start searching for a different json-rpc module.

nackjicholson commented 9 years ago

817601a1f5735e70bb4ea79d342fba4035877b2d here is a commit which fixes it on my fork. Haven't run your test suite on it though. Seems to work for me, with some light manual testing.

dfellis commented 9 years ago

Thank you for this fix. :)