Uh oh!
There was an error while loading. Please reload this page.
http2: specify default TLS options for http2 client connection. - #61
http2: specify default TLS options for http2 client connection.#61jmuk wants to merge 4 commits into
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
jasnell
commented
May 2, 2017
Had a different approach in mind for fixing this: diff--gita/lib/internal/http2/core.jsb/lib/internal/http2/core.jsindex6afc87e6f7..052513289f100755---a/lib/internal/http2/core.js+++b/lib/internal/http2/core.js
@@ -1077,6+1077,7 @@ functioninitializeOptions(options){options=options||{};if(typeofoptions!=='object')thrownewTypeError('options must be an object');+options=Object.create(options);options.allowHalfOpen=true;options.settings=options.settings||{};if(typeofoptions.settings!=='object')
@@ -1084,10+1085,12 @@ functioninitializeOptions(options){returnoptions;}-functioninitializeTLSOptions(options){+functioninitializeTLSOptions(options,host){options=initializeOptions(options);options.ALPNProtocols=['hc','h2'];options.NPNProtocols=['hc','h2'];+if(host!==undefined)+options.servername=host;returnoptions;}
@@ -1235,7+1238,7 @@ functionconnect(authority,options,listener){socket=net.connect(port,host);break;case 'https:':-socket=tls.connect(port,host,options);+socket=tls.connect(port,host,initializeTLSOptions(options,host));break;
default:
thrownewTypeError(`protocol "${protocol}" in unsupported.`);james@ubuntu:~/node/http2-jasnell$This allows us to have a central place for setting the TLS options for both server and client. |
This verifies to send the server name and ALPN protocols by default.
jmuk
commented
May 2, 2017
Thank you for the review! Added a test case, and fixed the client code for your latter suggestion. |
jmuk
commented
May 2, 2017
By the way -- where does the ALPN name |
jasnell
commented
May 2, 2017
I think it's an old pre-standard value. One of the browser impls was using it. Whether or not we need to keep it remains to be seen |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
jmuk
commented
May 4, 2017
Removed the count, thank you for the background info. |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
jasnell
commented
May 5, 2017
@mcollina ... ping |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
mcollina
commented
May 5, 2017
LGTM |
jasnell
commented
May 7, 2017
Landed! |
fixes: #59