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

Add interop tests to verify that Kestrel's HTTP/2 implementation works with HttpClient's implementation #4763

Closed
Tratcher opened this issue Dec 29, 2017 · 14 comments
Assignees
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions clean-up This issue is internal clean-up and has no effect on public APIs or expected behaviors Done This issue has been fixed feature-kestrel HTTP2

Comments

@Tratcher
Copy link
Member

There are not currently any functional test for the Http2 features. We accidentally disabled the entire feature when merging the https and core packages due to a missing TFM and didn't notice. aspnet/KestrelHttpServer#2224

The big blocker here is the lack of consistent client support. However, even if there's one client on one of the OS's then that would be a good start.

@linianhui
Copy link

Does client mean web browser or web server?

@Tratcher
Copy link
Member Author

Browsers are clients, yes. System.Net.Http.HttpClient is the client we prefer for functional tests.

@linianhui
Copy link

@Tratcher But now System.Net.Http.HttpClient does not support http2. ( ̄▽ ̄)"

@benaadams
Copy link
Member

HttpClient supports http2 on .NET Core for UWP, Linux dotnet/corefx#4756 and Windows dotnet/corefx#10948 (but not Full Framework as yet)

@Tratcher
Copy link
Member Author

Note the Windows version restriction as well.
Windows 10 Anniversary release a.k.a Windows 10 Version 1607

@Tratcher Tratcher self-assigned this Jun 13, 2018
Tratcher referenced this issue in aspnet/KestrelHttpServer Jun 13, 2018
Limit Http/2 to TLS 1.2 #2251
Bootstrap functional tests #2238
Tratcher referenced this issue in aspnet/KestrelHttpServer Jun 13, 2018
Limit Http/2 to TLS 1.2 #2251
Bootstrap functional tests #2238
Tratcher referenced this issue in aspnet/KestrelHttpServer Jun 18, 2018
Limit Http/2 to TLS 1.2 #2251
Bootstrap functional tests #2238
@Tratcher Tratcher removed their assignment Jun 19, 2018
@Tratcher
Copy link
Member Author

I did some initial work in #2665 but only for Win10 WinHttpHandler. Linux and Mac have too many dependency issues, we may have to wait for SocketsHttpHandler.

@muratg
Copy link
Contributor

muratg commented Nov 28, 2018

The remaining test we'd like to add is using new HttpClient.

@aspnet-hello aspnet-hello transferred this issue from aspnet/KestrelHttpServer Dec 13, 2018
@aspnet-hello aspnet-hello added this to the 3.0.0 milestone Dec 13, 2018
@analogrelay
Copy link
Contributor

We think there's enough support to do this now.

@analogrelay analogrelay modified the milestones: 3.0.0, 3.0.0-preview6 Mar 27, 2019
@analogrelay analogrelay changed the title Add functional tests for http2 Add interop tests to verify that Kestrel's HTTP/2 implementation works with HttpClient's implementation Apr 30, 2019
@analogrelay
Copy link
Contributor

@Tratcher to follow up on the status of HTTP/2 support in HttpClient.

@analogrelay
Copy link
Contributor

@Tratcher turned the existing tests on again 🎉

We do need some additional interop tests that use Trailers.

@analogrelay analogrelay added the clean-up This issue is internal clean-up and has no effect on public APIs or expected behaviors label Jun 18, 2019
@analogrelay
Copy link
Contributor

This issue is now tracking adding tests that use Trailers to the HttpClient interop test suite.

@Tratcher
Copy link
Member Author

Tratcher commented Jul 2, 2019

HttpClient interop to verify:

  • ALPN handshake
  • H2C (via app context switch AppContext.SetSwitch("System.Net.Http.SocketsHttpHandler.Http2UnencryptedSupport", true);)
  • Bidirectional streaming https://github.com/dotnet/corefx/issues/38559
  • Response Trailers
  • Reset
    • Reset after CompleteAsync (kill the request body)
  • Multi-frame request & response headers
  • Settings
    • Concurrent request limits
    • Frame size limits
    • Header table size
    • Window size
    • Max header list size
  • Header validation (e.g. Custom host, url encoding, header encoding)

@analogrelay
Copy link
Contributor

Moving to preview9 for now. The bar for test-only changes is super low (basically just tell mode)

@analogrelay
Copy link
Contributor

We can keep churning on this in master for now. We have a lot of tests now, but @Tratcher's list has some things we still want to cover. We'll look at backporting test coverage if it would be helpful.

@Tratcher Tratcher added Done This issue has been fixed and removed Working labels Dec 17, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Jan 16, 2020
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions clean-up This issue is internal clean-up and has no effect on public APIs or expected behaviors Done This issue has been fixed feature-kestrel HTTP2
Projects
None yet
Development

No branches or pull requests

8 participants