Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Middleware Server Transport (Express/Connect) #12

Merged
merged 1 commit into from

2 participants

@dfellis
Collaborator

Wrote a Connect/Express-style middleware for the JSON-RPC server, after some prompting.

The most difficult part was remembering the Express API. :)

@countrodrigo countrodrigo commented on the diff
test/index.js
((16 lines not shown))
+ loopback: function(arg, callback) { callback(null, arg); }
+ });
+ app.use('/rpc', server.transport.middleware);
+
+ //app.listen(55555); // Express 3.0 removed the ability to cleanly shutdown an express server
+ // The following is copied from the definition of app.listen()
+ var server = http.createServer(app);
+ server.listen(55555);
+
+ var client = new Client(new ClientHttp('localhost', 55555, { path: '/rpc' }));
+ client.register('loopback');
+
+ http.get({
+ port: 55555,
+ path: '/foo'
+ }, function(res) {
@countrodrigo Collaborator

Why bother performing a plain http test? It makes the test case a bit confusing. Are you testing that your middleware doesn't screw up plain http requests?

@dfellis Collaborator
dfellis added a note

Yes, that's exactly why. The whole point of a middleware-style transport over the regular HTTP transport is that the transport can share the same HTTP server as the regular HTTP traffic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@countrodrigo countrodrigo commented on the diff
lib/transports/server/middleware.js
((8 lines not shown))
+function MiddlewareTransport(config) {
+ // Initialize the EventEmitter for this object
+ EventEmitter.call(this);
+
+ // Make sure the config object exists, the handler function exists,
+ // and the Access-Control-Allow-Origin header is properly set. Also
+ // allow the user to provide a reference to the underlying HTTP server
+ // so the ``shutdown`` method can work as expected, if desired.
+ config = config || {};
+ this.handler = function fakeHandler(json, next) { next({}); };
+ this.acao = config.acao ? config.acao : "*";
+ this.server = config.server || null;
+ this.middleware = this.requestHandler.bind(this);
+
+ return this;
+}
@countrodrigo Collaborator

; ?

@dfellis Collaborator
dfellis added a note

I'm not assigning this function to a variable, here (see line 8 above), so no semicolon needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@countrodrigo countrodrigo commented on the diff
lib/transports/server/middleware.js
((3 lines not shown))
+
+// Connect/Express middleware style JSON-RPC server transport
+// Let's you have a hybrid Connect/Express server that also performs JSON-RPC
+// on a particular path. Still done as an instance so you can conceivably have
+// multiple JSON-RPC servers on a single Connect/Express server.
+function MiddlewareTransport(config) {
+ // Initialize the EventEmitter for this object
+ EventEmitter.call(this);
+
+ // Make sure the config object exists, the handler function exists,
+ // and the Access-Control-Allow-Origin header is properly set. Also
+ // allow the user to provide a reference to the underlying HTTP server
+ // so the ``shutdown`` method can work as expected, if desired.
+ config = config || {};
+ this.handler = function fakeHandler(json, next) { next({}); };
+ this.acao = config.acao ? config.acao : "*";
@countrodrigo Collaborator

Do you really want to set the access control to * by default? That seems like a security risk that you are encouraging. If you just don't send the header then the browsers will force the requests to be coming from the same subdomain, which is a better default

@dfellis Collaborator
dfellis added a note

ACAO is security-by-stupidity, to be honest with you. You have to trust the client to obey the ACAO directives to not access things they aren't supposed to, which you can't do because older browsers and anyone with curl/wget can bypass them anyways and hand-craft malicious requests.

Your server security needs to be enforced on the server-side, not the client-side, so this ACAO configuration is just to placate Firefox (the ones that started this nonsense). You're no less secure with ACAO: "*" than with restricting the ACAO value, but just making things more difficult on yourself and possibly your own users if you forgot to include a subdomain in the ACAO listing and missed it during testing, which makes it the same kind of "security" as DRM.

@countrodrigo Collaborator
@dfellis Collaborator
dfellis added a note

I still don't see the benefit to ACAO -- if you make a service public on the 'net, you need to protect it against malicious connection attempts. ACAO gives young developers (and they disproportionately gravitate towards the web) a false sense of security that setting the ACAO value correctly will "protect" them.

On the flip-side, stupid bugs for clients arise from different browsers having a different "strictness" on interpreting ACAO (for instance IE didn't enforce ACAO at all until years after Firefox started, so "Windows Shops (tm)" didn't fix their ACAO-busted sites for a while. Simple user pain.

On top of that, young developers that don't know about the ACAO headers may think cross-site AJAX is simply impossible and do crazy things like the proxy server thing you mentioned, just because they didn't know how to configure their original server correctly.

ACAO = all pain, no gain, and I hate that kind of nonsensical bullshit tech with a firey passion. :P

@countrodrigo Collaborator

I take that to be a merge as-is.

@dfellis Collaborator
dfellis added a note

Didn't mean to be combative, but ACAO is a (mis)feature I don't really care for, and I'm being nice by letting people override it. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@countrodrigo
Collaborator

Comments addressed, views expressed, nothing to duel over. Merging.

@countrodrigo countrodrigo merged commit 953ecd0 into master

1 check passed

Details default The Travis build passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
32 README.md
@@ -29,6 +29,7 @@ var Client = jsonrpc.client; // The client constructor function
var ServerHttp = jsonrpc.transports.server.http; // The server HTTP transport constructor function
var ServerTcp = jsonrpc.transports.server.tcp; // The server TCP transport constructor function
+var ServerMiddleware = jsonrpc.transports.server.middleware; // The server Middleware transport constructor function (for Express/Connect)
var ClientHttp = jsonrpc.transports.client.http;
var ClientTcp = jsonrpc.transports.client.tcp;
@@ -41,6 +42,15 @@ var jsonRpcTcpServer = new Server(new ServerTcp(8001), {
loopback: function(obj, callback) { callback(undefined, obj); }
});
+var express = require('express');
+var app = express();
+app.use(express.bodyParser());
+var jsonRpcMiddlewareServer = new Server(new ServerMiddleware(), {
+ loopback: function(obj, callback) { callback(undefined, obj); }
+});
+app.use('/rpc', jsonRpcMiddlewareServer.transport.middleware);
+app.listen(8002);
+
// Either explicitly register the remote methods
var jsonRcpHttpClient = new Client(new ClientHttp('localhost', 8000));
jsonRpcHttpClient.register('loopback');
@@ -54,6 +64,12 @@ new Client(new ClientTcp('localhost', 8001), {}, function(jsonRpcTcpClient) {
console.log(val); // Prints 'foo'
});
});
+
+var jsonRpcExpressClient = new Client(new ClientHttp('localhost', 8002, { path: '/rpc' }));
+jsonRpcExpressClient.register('loopback');
+jsonRpcExpressClient.loopback('foo', function(err, val) {
+ console.log(val); // Prints 'foo'
+});
```
### Constructor Function Parameters
@@ -164,6 +180,22 @@ The Server TCP Transport events are:
``shutdown`` - This event is fired when the server is shutdown.
+#### jsonrpc.transports.server.middleware
+
+``new jsonrpc.transports.server.middleware(config)``
+
+``config`` - The configuration settings. For the Connect/Express middleware transport, these are:
+
+``acao`` - The ``Access-Control-Allow-Origin`` header value, which defaults to ``*``.
+
+``server`` - A reference to the underlying server the middleware relies on. Used only for ``shutdown`` compatibility, if desired.
+
+The Server Middleware Transport events are:
+
+``message`` - This event is fired whenever a complete message is received, and the registered callbacks receive the JSON-RPC object as their only argument.
+
+``shutdown`` - This event is fired when the transport is shutdown.
+
## Defining JSON-RPC Server Methods
By default, JSON-RPC server methods are asynchronous, taking a callback function as the last argument. The callback function assumes the first argument it receives is an error and the second argument is a result, in the Node.js style.
View
3  lib/index.js
@@ -8,7 +8,8 @@ module.exports = {
},
server: {
http: require('./transports/server/http'),
- tcp: require('./transports/server/tcp')
+ tcp: require('./transports/server/tcp'),
+ middleware: require('./transports/server/middleware')
}
}
};
View
62 lib/transports/server/middleware.js
@@ -0,0 +1,62 @@
+var util = require('util');
+var EventEmitter = require('events').EventEmitter;
+
+// Connect/Express middleware style JSON-RPC server transport
+// Let's you have a hybrid Connect/Express server that also performs JSON-RPC
+// on a particular path. Still done as an instance so you can conceivably have
+// multiple JSON-RPC servers on a single Connect/Express server.
+function MiddlewareTransport(config) {
+ // Initialize the EventEmitter for this object
+ EventEmitter.call(this);
+
+ // Make sure the config object exists, the handler function exists,
+ // and the Access-Control-Allow-Origin header is properly set. Also
+ // allow the user to provide a reference to the underlying HTTP server
+ // so the ``shutdown`` method can work as expected, if desired.
+ config = config || {};
+ this.handler = function fakeHandler(json, next) { next({}); };
+ this.acao = config.acao ? config.acao : "*";
@countrodrigo Collaborator

Do you really want to set the access control to * by default? That seems like a security risk that you are encouraging. If you just don't send the header then the browsers will force the requests to be coming from the same subdomain, which is a better default

@dfellis Collaborator
dfellis added a note

ACAO is security-by-stupidity, to be honest with you. You have to trust the client to obey the ACAO directives to not access things they aren't supposed to, which you can't do because older browsers and anyone with curl/wget can bypass them anyways and hand-craft malicious requests.

Your server security needs to be enforced on the server-side, not the client-side, so this ACAO configuration is just to placate Firefox (the ones that started this nonsense). You're no less secure with ACAO: "*" than with restricting the ACAO value, but just making things more difficult on yourself and possibly your own users if you forgot to include a subdomain in the ACAO listing and missed it during testing, which makes it the same kind of "security" as DRM.

@countrodrigo Collaborator
@dfellis Collaborator
dfellis added a note

I still don't see the benefit to ACAO -- if you make a service public on the 'net, you need to protect it against malicious connection attempts. ACAO gives young developers (and they disproportionately gravitate towards the web) a false sense of security that setting the ACAO value correctly will "protect" them.

On the flip-side, stupid bugs for clients arise from different browsers having a different "strictness" on interpreting ACAO (for instance IE didn't enforce ACAO at all until years after Firefox started, so "Windows Shops (tm)" didn't fix their ACAO-busted sites for a while. Simple user pain.

On top of that, young developers that don't know about the ACAO headers may think cross-site AJAX is simply impossible and do crazy things like the proxy server thing you mentioned, just because they didn't know how to configure their original server correctly.

ACAO = all pain, no gain, and I hate that kind of nonsensical bullshit tech with a firey passion. :P

@countrodrigo Collaborator

I take that to be a merge as-is.

@dfellis Collaborator
dfellis added a note

Didn't mean to be combative, but ACAO is a (mis)feature I don't really care for, and I'm being nice by letting people override it. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ this.server = config.server || null;
+ this.middleware = this.requestHandler.bind(this);
+
+ return this;
+}
@countrodrigo Collaborator

; ?

@dfellis Collaborator
dfellis added a note

I'm not assigning this function to a variable, here (see line 8 above), so no semicolon needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+// Attach the EventEmitter prototype to the prototype chain
+util.inherits(MiddlewareTransport, EventEmitter);
+
+// The ``requestHandler`` method gets the request and response objects, and passes
+// the request body and the bound responseHandler to the JSON-RPC hander function
+MiddlewareTransport.prototype.requestHandler = function requestHandler(req, res) {
+ // All requests are assumed to be "Express-like" and have the bodyParser run
+ // before it. Express doesn't have a good way (that I'm aware of) to specify
+ // "run this middleware if it hasn't already been run".
+ this.emit('message', req.body);
+ this.handler(req.body, this.responseHandler.bind(this, res));
+};
+
+// The ``responseHandler`` method takes the output object and sends it to the client
+MiddlewareTransport.prototype.responseHandler = function responseHandler(res, retObj) {
+ var outString = JSON.stringify(retObj);
+ res.writeHead(retObj.error? 500:200, {
+ "Access-Control-Allow-Origin": this.acao,
+ "Content-Length": Buffer.byteLength(outString, 'utf8'),
+ "Content-Type": "application/json;charset=utf-8"
+ });
+ res.end(outString);
+};
+
+// If the user defined the server in the config, the ``shutdown`` method will
+// tell the server to shut down. Likely, when a JSON-RPC server is used as a
+// middleware, this will not be done, but for API consistency's sake, it could.
+MiddlewareTransport.prototype.shutdown = function shutdown(done) {
+ if(this.server) {
+ this.emit('shutdown');
+ this.server.close(done);
+ } else {
+ if(done instanceof Function) done();
+ }
+};
+
+// Export the Server Middleware Transport
+module.exports = MiddlewareTransport;
View
3  package.json
@@ -22,7 +22,8 @@
},
"devDependencies": {
"nodeunit": "*",
- "docco-husky": "*"
+ "docco-husky": "*",
+ "express": "*"
},
"scripts": {
"realpublish": "./prepublish.sh",
View
42 test/index.js
@@ -5,6 +5,9 @@ var ClientHttp = jsonrpc.transports.client.http;
var ClientTcp = jsonrpc.transports.client.tcp;
var ServerHttp = jsonrpc.transports.server.http;
var ServerTcp = jsonrpc.transports.server.tcp;
+var ServerMiddleware = jsonrpc.transports.server.middleware;
+var express = require('express');
+var http = require('http');
exports.loopbackHttp = function(test) {
test.expect(1);
@@ -35,4 +38,43 @@ exports.failureTcp = function(test) {
});
});
});
+};
+
+exports.loopbackExpress = function(test) {
+ test.expect(2);
+
+ var app = express();
+ app.use(express.bodyParser());
+ app.get('/foo', function(req, res) {
+ res.end('bar');
+ });
+
+ var server = new Server(new ServerMiddleware(), {
+ loopback: function(arg, callback) { callback(null, arg); }
+ });
+ app.use('/rpc', server.transport.middleware);
+
+ //app.listen(55555); // Express 3.0 removed the ability to cleanly shutdown an express server
+ // The following is copied from the definition of app.listen()
+ var server = http.createServer(app);
+ server.listen(55555);
+
+ var client = new Client(new ClientHttp('localhost', 55555, { path: '/rpc' }));
+ client.register('loopback');
+
+ http.get({
+ port: 55555,
+ path: '/foo'
+ }, function(res) {
@countrodrigo Collaborator

Why bother performing a plain http test? It makes the test case a bit confusing. Are you testing that your middleware doesn't screw up plain http requests?

@dfellis Collaborator
dfellis added a note

Yes, that's exactly why. The whole point of a middleware-style transport over the regular HTTP transport is that the transport can share the same HTTP server as the regular HTTP traffic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ res.setEncoding('utf8');
+ var data = '';
+ res.on('data', function(chunk) { data += chunk; });
+ res.on('end', function() {
+ test.equal(data, 'bar', 'regular http requests work');
+ client.loopback('bar', function(err, result) {
+ test.equal(result, 'bar', 'JSON-RPC as a middleware works');
+ server.close(test.done.bind(test));
+ });
+ });
+ });
};
Something went wrong with that request. Please try again.