Conversation
Matthias247
left a comment
Matthias247
left a comment
There was a problem hiding this comment.
Thanks for the contribution!
Generally looks very helpful! I left a few comments on some places.
| // Create HTTP/2 stream abstraction on top of the socket | ||
| var wrappedStreams = tcpClient.Client.CreateStreams(); | ||
| var upgradeReadStream = new UpgradeReadStream(wrappedStreams.ReadableStream); | ||
| var (readableStream, writeableStream) = await CreateStreams(scheme, host, port); |
There was a problem hiding this comment.
If I understand the specification correctly then HTTP/2 via upgrade is only supported for not encrypted connections (h2c). So while this might work it seems to be against the spec.
Maybe just throw an exception that upgrade+TLS is not supported instead.
If this would work, then the ALPN modifier should also be not set - since the upgrade intent is signaled via HTTP upgrade instead of via ALPN for this case.
There was a problem hiding this comment.
Ah yes, I will throw a NotSupportException in this case.
| }, | ||
| }; | ||
| await stream.AuthenticateAsClientAsync(options, default(CancellationToken)); | ||
| logger.LogInformation("SSL Authenticated"); |
There was a problem hiding this comment.
Does at this place a verification need to get inserted that checks whether the server accepted the proposed ALPN protocol?
Matthias247
left a comment
Matthias247
left a comment
There was a problem hiding this comment.
I added one more suggestions. Let's see if I can apply that myself.
Co-Authored-By: Matthias Einwag <matthias.einwag@live.com>
|
oh wait, i think your suggestion had the protocol backwards. let me fix that and test. |
|
Fixed & tested. Here's where it's referenced in the spec. |
|
The last one should have been right? We are doing an upgrade, and cleartext TCP (since TLS always uses ALPN). So |
2 participants
Hi, I hope you don't mind the unsolicited PR. I was exploring some HTTP2 options in C# and found your library. I wanted to get it working with SSL and managed to do so in your CliClient example.
Some Notes:
Thanks!