search

spdy-http2 / node-spdy

SPDY server on Node.js
2.81k stars 196 forks source link

Serving CONNECT isn't properly supported #376

Open pimterry opened 3 years ago

pimterry commented 3 years ago

On incoming HTTP/2 CONNECT requests SPDY fires a 'connect' event on the server, passing it the request and the raw network socket, exactly like HTTP/1.1.

The implication here is that the server should set the socket up to work as a raw data tunnel to the target host after this point, just like HTTP/1.1. This is what the (now removed) proxy test did, but this is wrong.

That's not how HTTP/2 CONNECT works: https://http2.github.io/http2-spec/#CONNECT. With HTTP/2 that one specific stream becomes a tunnel, not the entire connection. You can see more explanation in the HTTP/2 websockets spec, e.g.:

This document extends the HTTP CONNECT method, ... The result is a tunnel on a single HTTP/2 stream that can carry data for WebSockets (or any other protocol). The other streams on the connection may carry more extended CONNECT tunnels, traditional HTTP/2 data, or a mixture of both.

To handle CONNECT for HTTP/2 correctly, the server needs to get the specific stream in question, and proxy all traffic through that (wrapping all traffic up in DATA frames for that stream).

I've been playing around and I think this works:

server.on('connect', function (req, socket) {
  const targetUrl = req.headers['host'];
  const stream = req.connection._handle.getStream();

  stream.respond(200, {}, () => {
    // ...Proxy some traffic to targetUrl, piping requests & responses from & to `stream`
  });
});

Doing that requires digging around in req.connection._handle.getStream, and I'm fairly sure those aren't supposed to be public APIs. I think the above does this successfully, but it's definitely messing around in the internals and I'm not sure.

This could be properly supported quite easily I think: you'd just need to swap the socket param for the correct stream for this request. It'd be better to set the host as req.url too, which is currently undefined.