Making http "hookable"

[jasnell]

@nodejs/http ... Pulled over to a new issue from https://github.com/nodejs/http/issues/2#issuecomment-151283755

One of the possible approaches we can take with the http module is allowing third party modules to systematically replace core functions of the http implementation with their own bits. To do this we would need to:

• Clearly identify which bits are replaceable
• Clearly define the API / contract that the replacement components would need to implement
• Clearly define the API for registering those replacements
• Refactor the current implementation to make reuse and replacement of key parts easier / possible

To keep things as simple as possible, I'd recommend that we identify the replaceable pieces as coarsely as possible. This is just a strawman at this point.... feel free to knock it down.

• Allow the entire HTTP Parser implementation to be replaced. This would entail wrapping the HTTP Parser into a JS API and providing an API for developers to replace the parser in use (e.g. something like: const http = require('http'); http.useParser(myParser);)
• Allow parsing handlers for individual HTTP headers to be registered. This would be part of the new HTTP Parser wrapper API mentioned above. The existing http-parser implementation would need to be modified to use the registered callbacks to parse specific header field values. Custom HTTP parser implementations would be expected to follow the same contract. (e.g. something like: const http = require('http'); http.onHeader('x-foo', myFooParser);)
• On the server side, allow the Request handling logic to be replaced. Once the request has been parsed, it's handed off to logic that determines the handling. Alternative implementations would be expected to support the existing API contract but could extend that behavior and provide alternative APIs:

    const http = require('http');
    const myimpl = require('my-implementation');
    // myimpl is a complete reimplementation of the request handling logic, as opposed to 
    // just a callback...
    const server = http.createServer(myimpl); 
    server.listen(8080);

Not quite sure yet what all would need to happen on the client side... still working through that. Thoughts?

[jasnell]

The alternative on the first two bullet points above is to make the custom header handling part of the alternative parser implementation... e.g.

const http = require('http');
const customparser = require('my-parser-implementation');
customparser.onHeader('x-foo', Xfooparser);
http.useParser(customparser);

[aredridel]

It would be nice to have the parser be per-instance, not global, because being able to globally override behavior, while nice for hotfixes to issues, is frustrating when trying to make defensive, reliable modules that use an API that can be plugged.

[aks-]

The header parsing of the response from the server could be one thing which can be made hookable.

[mscdex]

Why not just expose the existing API that the current http parser is/was using? Namely, reinitialize(), execute(), and finish()?

[jasnell]

@mscdex ... yep, that's definitely a solid option (in fact, it should be the default option unless a better choice comes along later). Essentially, we'd have a new API for plugging in a replacement parser at the JS level that implemented those same three methods.

[trevnorris]

This would entail wrapping the HTTP Parser into a JS API and providing an API for developers to replace the parser in use

While I agree with this theoretically, the overhead could prove to be substantial on the existing implementation. Unless it can be done w/o near-zero overhead then I doubt much of the general audience will go for it.

The existing http-parser implementation would need to be modified to use the registered callbacks to parse specific header field values.

What if the header field parser is insecure? IOW, sort of the point of this is to (pretty much) remove responsibility from node in terms of securely parsing. Also it would remove the opportunity to do any performance enhancements in the future. I've been playing with a couple ideas that could offer substantial gains, but after a change like this those improvements are unlikely to ever land.

I saw the easiest route to bludgeon the issue in the face and remove everything between the TCP data event and the http server 'request' event. So the API would essentially look like:

http.setNewServer(callback);
http.setOnRequest(server, socket, req);

Those names are horrible, but we can look past that.

Where server is the TCP server, socket is the incoming connection and req is an object with various necessary fields (e.g. headers). Then the http module would emit the 'request' as normal to the user with the data that the intercepting module generated.

Just an idea, and still very rough, but figured that type of approach would ease the necessary aspect of separating the HTTPParser from the other list of changes.

[hueniverse]

Is there really a strong case of swapping the core http implementation? Why not just let people ignore it. If you are using a module that directly accesses http, then let it. It will work just fine and the API contract will not be a concern.

[jasnell]

@hueniverse ... essentially make it easier for modules to directly access the underlying stream/socket then? Correct?

[trevnorris]

Asked myself the same question and an applicable reason my be so any other modules that use the native http module will automatically inherit the changes. e.g.

require('my-custom-http');
var express = require('express');

As a way to enforce certain header parsing policies.

[bnoordhuis]

Isn't that too much spooky action at a distance? I'd extend express so it can take a custom http implementation.

[trevnorris]

I can appreciate not allowing spooky action, but wouldn't any type of http hooks result in the same?

[bnoordhuis]

Not when you have to explicitly configure express. The spooky action I'm thinking of is a module three levels deep quietly making process-wide changes.

[trevnorris]

Then the premise of

One of the possible approaches we can take with the http module is allowing third party modules to systematically replace core functions of the http implementation with their own bits.

is no longer valid.

[hueniverse]

My requirement is to be able to write my own http server without having to write all the TCP and TLS pieces.

[trevnorris]

@hueniverse would you mind giving a few brief points on how you think that would look? I assume you mean not needing to handle the 'connection' event, etc., but I'm failing to see an API possibility around that.

[hueniverse]

I'll need to refresh my knowledge on the current code. Hope to do that next few days.