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

HTTP2 and HTTP1 on the same socket #44887

Closed
FStefanni opened this issue Oct 4, 2022 · 18 comments
Closed

HTTP2 and HTTP1 on the same socket #44887

FStefanni opened this issue Oct 4, 2022 · 18 comments
Labels
feature request Issues that request new features to be added to Node.js. http Issues or PRs related to the http subsystem. http2 Issues or PRs related to the http2 subsystem.

Comments

@FStefanni
Copy link

What is the problem this feature will solve?

Support both HTTP1 and HTTP2 on the same server socket.

I have seen the discussion on #26795, but the issue was closed without a clear answer (for what I was able to understand).
As some of the commenters pointed out:

  • It is not an uncommon feature/need, since IoT and embedded devices could need it. And more in general, not all network traffic and server applications are related to browsers, so this feature would be very useful.
  • It is useful on case of reverse proxies, since it is very common to let them handle the TLS termination, and just forward plain HTTP to the actual services.
  • It simplifies and generalizes node.js projects, since servers could just create HTTP2 sockets, being always able to support both versions

What is the feature you are proposing to solve the problem?

Add a "allowHTTP1": true, option to HTTP2 non-TLS server, similar to the one available for HTTP2 TLS server.

What alternatives have you considered?

I don't see many alternatives actually...

@FStefanni FStefanni added the feature request Issues that request new features to be added to Node.js. label Oct 4, 2022
@bnoordhuis
Copy link
Member

HTTP2 non-TLS server

That's not a thing on today's internet. There's little point supporting something so niche.

I don't really buy the reverse proxy argument either. You don't normally reverse-proxy http/2 verbatim because of connection coalescing. That's just setting yourself up for getting pwned by the first hacker that comes along.

I don't see many alternatives actually...

If it's an important requirement for your application, you can start a net.Server listen socket, look at the first few bytes of the first packet, then forward to a http.Server or a http2.Http2Server.

@FStefanni
Copy link
Author

Hi,

maybe it is niche, so I can agree that it is not a top priority, but not supporting is something weird imho.

First of all, HTTP2 is a standard since quite a lot of time, and it has been designed so that it is possible to support HTTP1 & 2 on the same socket, which is very important: basically, the idea is to allow HTTP 1.1 for compatibility with old clients, but to use HTTP 2 with all the newer ones. Not something of today internet? Sorry, but it means that internet is not using the newest and best protocols. And again, not all network traffic is related to browsers.

Second, it seems to me a chicken-egg problem: frameworks do not support some features since programmers do not use that features, and programmers do not use that features because frameworks do not provide them. Frameworks should provide features and let programmers use the ones that more fit within their projects.

Lastly, it seems somehow strange that the coexistence of HTTP1 and 2 is supported for TLS, but not for non-TLS: it is an asymmetry which really stands out as something missing. Yes, I can try to implement by myself, but it can be complicated/hard (I do not know all the details, whilst node team has already gone throw this for the TLS part). Implementing this feature in node core will give the opportunity to use this feature to all people.

Regards.

@bnoordhuis
Copy link
Member

it has been designed so that it is possible to support HTTP1 & 2 on the same socket

Not really, not in the way most people would understand that sentence. You can upgrade a http/1 connection to http/2. You can do that with node, you just have to wire it up.

@tniessen tniessen added http Issues or PRs related to the http subsystem. http2 Issues or PRs related to the http2 subsystem. labels Oct 5, 2022
@tniessen
Copy link
Member

tniessen commented Oct 5, 2022

Add a "allowHTTP1": true, option to HTTP2 non-TLS server, similar to the one available for HTTP2 TLS server.

@FStefanni The allowHTTP1 option relies on ALPN to negotiate the protocol, which is well-defined and works nicely. However, ALPN is a TLS feature. For non-TLS connections, there is not so much we could do aside from what @bnoordhuis described above (i.e., guess what kind of connection it is based on the first few bytes). As far as I can tell, this bit can be implemented in userland.

@FStefanni
Copy link
Author

Hi,

I still believe that this could be useful, but at least I suppose that if implementing this is not of interest for you, it could be nice to have an example of how to implement this in userland: for me it is not obvious how to do this (detection of http version, negotiating the upgrade to version 2, and "redirection" to the correct server).

Regards.

@bnoordhuis
Copy link
Member

The RFC I linked to spells out what the payloads look like.

For http/1, attach an 'upgrade' event listener to the http.Server object.

You can inject connections into a http.Server or a http2.Http2Server by emitting a 'connection' event on them, then emitting already read data as a 'data' event.

('data' is straightforward but a bit old-fashioned and inefficient. Check the streams documentation for more options.)

@bnoordhuis
Copy link
Member

@jasnell I'm curious, is there a reason automatic (or opt-in) upgrades to http/2 weren't implemented in the http/1 server? It'd be nice to have a unified interface, like in golang.

@FStefanni
Copy link
Author

Hi,

thank you for the pointers.

@jasnell I'm curious, is there a reason automatic (or opt-in) upgrades to http/2 weren't implemented in the http/1 server? It'd be nice to have a unified interface, like in golang.

Yes, basically this is how things are imho expected to work: transparently users can connect with http1 or http2, or choose to start with http1 and upgrade to http2 if supported by the server. And server side, there should be no need to have many server classes, one or each protocol version, or do "tricks" to support http1 and 2 on the same socket: a unified class able to do all of this would be much cleaner and improve programs maintainability and flexibility.

Actually my feature request aims exactly at this...

Regards

@bnoordhuis
Copy link
Member

Turns out go's net/http server doesn't support Upgrade: h2c either, only ALPN, like node's { allowHTTP: true }.

I think all evidence points to non-ALPN http/2 simply not being a thing and as such, I'm going to close out this issue.

@bnoordhuis bnoordhuis closed this as not planned Won't fix, can't repro, duplicate, stale Oct 6, 2022
@tniessen
Copy link
Member

tniessen commented Oct 6, 2022

FWIW, Upgrade: h2c appears to be well-defined (e.g., RFC 7540 Section 3.2), so it really boils down to: does anyone use insecure HTTP/2 in a justified manner?

@casret
Copy link

casret commented Feb 8, 2023

Insecure http/2 is very useful if you are ssl terminating at a load balancer.

@jasnell
Copy link
Member

jasnell commented Feb 8, 2023

Sorry, just spotted the mention on this .... It wasn't implemented initially just because at the time it wasn't the key focus and it wasn't yet clear how it would be used, so rather than try to pre-emptively guess, it was deferred. It should be absolutely possible to implement, just needs someone to do the work.

@FStefanni
Copy link
Author

Hi,

so maybe this issue should be reopen?

Regards

@bnoordhuis
Copy link
Member

Different ask (unencrypted vs. h1/h2 over the same socket) so no, better to open a new issue.

@stT-e5gna2z5MBS
Copy link

@bnoordhuis

You can inject connections into a http.Server or a http2.Http2Server by emitting a 'connection' event on them, then emitting already read data as a 'data' event.

import { createServer as netCreateServer } from "net"
import { createServer as http1CreateServer } from "http"
import { createServer as http2CreateServer } from "http2"

const http1Server = http1CreateServer(handleOnRequest)
const http2Server = http2CreateServer(handleOnRequest)
function handleOnRequest(req,res) {
    console.log(req.headers)
}

const netServer = netCreateServer({
},socket=>{
    socket.on('data',chunk=>{
        const pos_CR=chunk.indexOf("\r")
        const firstLine = chunk.toString("utf8",0,pos_CR)
        console.log(`firstLine: ${firstLine}`)
        if (firstLine==="PRI * HTTP/2.0") {
            http2Server.emit('connection',socket)
            // `http2Server.emit('data',chunk)` doesn't work because 'data' belongs to `http2.Http2ServerRequest` ?
        } else {
            http1Server.emit('connection',socket)
        }
    })
})

netServer.listen(80,"127.0.0.1")

// http2Server.emit('data',chunk) doesn't work because 'data' belongs to http2.Http2ServerRequest ?

@stT-e5gna2z5MBS
Copy link

readable.unshift(chunk)

import { createServer as netCreateServer } from "net"
import { createServer as http1CreateServer } from "http"
import { createServer as http2CreateServer } from "http2"

const http1Server = http1CreateServer(handleOnRequest)
const http2Server = http2CreateServer(handleOnRequest)
function handleOnRequest(req,res) {
    console.log(req.headers)
}

const netServer = netCreateServer({
},socket=>{
    socket.once('data',chunk=>{
        const pos_CR=chunk.indexOf("\r")
        const firstLine = chunk.toString("utf8",0,pos_CR)
        console.log(`firstLine: ${firstLine}`)
        socket.unshift(chunk)
        if (firstLine==="PRI * HTTP/2.0") {
            http2Server.emit('connection',socket)
        } else {
            http1Server.emit('connection',socket)
        }
    })
})

netServer.listen(80,"127.0.0.1")

http1Server now works, but http2Server doesn't
meanwhile, this works:

import { createServer as netCreateServer } from "net"
import { createServer as http1CreateServer } from "http"
import { createServer as http2CreateServer } from "http2"

const http1Server = http1CreateServer(handleOnRequest)
const http2Server = http2CreateServer(handleOnRequest)
function handleOnRequest(req,res) {
    console.log(req.headers)
}

const netServer = netCreateServer({
},socket=>{
    http2Server.emit('connection',socket)
})

netServer.listen(80,"127.0.0.1")

@stT-e5gna2z5MBS
Copy link

using socket.read(3) instead of socket.once('data',chunk=>{ works
#34532

socket.once('data',chunk=>{ fails:

const net = require('net');
const http = require('http');
const http2 = require('http2');

const rawConnListener = (socket) => {

    socket.once('data',chunk=>{
        // Put the data back, so the real server can handle it:
        socket.unshift(chunk);

        console.log(chunk.length)
        //min length from 'data' is 3 so this is fine
        if (chunk.toString('ascii',0,3) === 'PRI') { // Very dumb preface check
            console.log('emitting h2');
            h2Server.emit('connection', socket);
        } else {
            console.log('emitting h1');
            h1Server.emit('connection', socket);
        }
    })


}

const rawServer = net.createServer(rawConnListener);

const h1Server = http.createServer(() => {
    console.log('Got h1 request');
});

const h2Server = http2.createServer(async () => {
    console.log('Got h2 request');
});

rawServer.listen(8080);

Make a direct HTTP/2 request to it:

>curl --http2-prior-knowledge http://localhost:8080
curl: (16) Error in the HTTP2 framing layer

@stT-e5gna2z5MBS
Copy link

socket.pause() //works for h2, fails for h1

const net = require('net');
const http = require('http');
const http2 = require('http2');

const rawConnListener = async (socket) => {

    const chunk = await new Promise(resolve=>socket.once("data",resolve))
    socket.pause() //works for h2, fails for h1

    socket.unshift(chunk);

    if (chunk.toString('ascii',0,3) === 'PRI') {
        console.log('emitting h2');
        h2Server.emit('connection', socket);
    } else {
        console.log('emitting h1');
        h1Server.emit('connection', socket);
    }

}

const rawServer = net.createServer(rawConnListener);

const h1Server = http.createServer(() => {
    console.log('Got h1 request');
});

const h2Server = http2.createServer(async () => {
    console.log('Got h2 request');
});

rawServer.listen(8001);

socket._readableState.flowing = null //works for h2, unnecessary for h1
// socket._readableState[Object.getOwnPropertySymbols(socket._readableState).find(v=>v.description==="kPaused")] = null //is there a better way than Object.getOwnPropertySymbols() + Array.find() ?

const net = require('net');
const http = require('http');
const http2 = require('http2');

const rawConnListener = async (socket) => {

    const chunk = await new Promise(resolve=>socket.once("data",resolve))
    socket._readableState.flowing = null //works for h2, unnecessary for h1
    // socket._readableState[Object.getOwnPropertySymbols(socket._readableState).find(v=>v.description==="kPaused")] = null //is there a better way than Object.getOwnPropertySymbols() + Array.find() ?

    socket.unshift(chunk);

    if (chunk.toString('ascii',0,3) === 'PRI') {
        console.log('emitting h2');
        h2Server.emit('connection', socket);
    } else {
        console.log('emitting h1');
        h1Server.emit('connection', socket);
    }

}

const rawServer = net.createServer(rawConnListener);

const h1Server = http.createServer(() => {
    console.log('Got h1 request');
});

const h2Server = http2.createServer(async () => {
    console.log('Got h2 request');
});

rawServer.listen(8001);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. http Issues or PRs related to the http subsystem. http2 Issues or PRs related to the http2 subsystem.
Projects
None yet
Development

No branches or pull requests

6 participants