-
Notifications
You must be signed in to change notification settings - Fork 186
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
Enable create TLS server without initial context #183
base: master
Are you sure you want to change the base?
Conversation
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. |
This simply allows lazy loading or async loading of SNI contexts, My usecase is a mitm proxy. Also, I think you're misleading 2 things:
|
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? |
@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. 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. |
just a friendly bump :) |
Fix #182