search

molnarg / node-http2

An HTTP/2 client and server implementation for node.js
MIT License
1.79k stars 187 forks source link

Enable create TLS server without initial context #183

Open felicienfrancois opened 8 years ago

felicienfrancois commented 8 years ago

Fix https://github.com/molnarg/node-http2/issues/182

nwgh commented 8 years ago

So I get that node's https module allows this, but how many things does not having this functionality break? In reality, it makes zero sense to have a server with no SNI contexts for HTTP/2 - since SNI support is a requirement in the RFC. As such, I'm not inclined to allow this (even though it deviates from the https API) without a really good reason.

felicienfrancois commented 8 years ago

This simply allows lazy loading or async loading of SNI contexts, For example when there are a lot of contexts or when they are not known in advance.

My usecase is a mitm proxy. But I could give you more usecase if you need it.

Also, I think you're misleading 2 things:

nwgh commented 8 years ago

Thanks, @felicienfrancois for the added context. I got to thinking about this more last night (even before you commented), and what I think we should do is this - remove the requirement for adding the SNI context from http2.Server.create, but in http2.Server.start, do the check to make sure we have at least one SNI context (it doesn't make sense, in my mind, to start a server without some context).

Does that seem reasonable?

felicienfrancois commented 8 years ago

@nwgh this is better and it would partly solve my usecase (I still need to start the server after I add the first context. So I will have to store or test if the server have already been started or not).

But I still does not understand why leaving this constraint. Would it break if we start a server without initial SNI context ? This is like starting a static server with no files to serve or serving an empty directory. In a functional point of view it does not makes sense but is it a reason to prevent it ? There could be yet knowns or unknowns situations where it could be useful or required, for a functional or more probably a technical reason.

The usecases that come to my mind are dynamic contexts, coming from a database for example (one context per client). It would means that the server cannot be started until there is at least one entry in database (i.e. one client) which may be tricky to handle.

Then, I think serving the standard SNI error on serve, is as meaningful as throwing an error on start.

felicienfrancois commented 8 years ago

just a friendly bump :)