Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Serving CONNECT isn't properly supported #376

Open
pimterry opened this issue Jul 8, 2020 · 0 comments
Open

Serving CONNECT isn't properly supported #376

pimterry opened this issue Jul 8, 2020 · 0 comments

Comments

@pimterry
Copy link

pimterry commented Jul 8, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant