Skip to content

Add SocketConnectionFactory and http2cat #14582

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

Merged
merged 4 commits into from
Oct 9, 2019
Merged

Add SocketConnectionFactory and http2cat #14582

merged 4 commits into from
Oct 9, 2019

Conversation

halter73
Copy link
Member

  • Add a Kestrel sample client for testing HTTP/2 servers
  • Add SocketConnectionFactory

TODO: Add SslStream
TODO: Support half-closing the SocketConnection

public static readonly int MaxRequestHeaderFieldSize = 16 * 1024;
public static readonly string _4kHeaderValue = new string('a', 4096);

public static readonly IEnumerable<KeyValuePair<string, string>> _browserRequestHeaders = new[]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyway to share with this with our test infrastructure.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe. It's already diverged a fair bit from Http2TestBase, so I'm not sure it's worth the effort though.


<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>netcoreapp5.0</TargetFramework>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be $(DefaultNetCoreTargetFramework)

@Tratcher
Copy link
Member

Tratcher commented Oct 4, 2019

This certainly meets its manual testing objectives. Here are some ideas for making it more broadly applicable in the future. E.g. I'd like to directly consume this as a library in Kestrel and HttpSys tests and remove the duplicate infrastructure.

  • Move the bulk of the code from the sample to a class library so other components can easily depend on it. Only Program.cs would stay in the sample.
  • Remove the internals-visible-to requirement, even if it requires copying more code out of Kestrel. Alternatively, those internals could be moved to a shared sources project that Kestrel directly consumes. Some of these internals overlap with the code we want to share with HttpClient anyways.

None of this is a near term priority.

@halter73 halter73 merged commit ff8363a into master Oct 9, 2019
@halter73 halter73 deleted the halter73/http2cat branch October 9, 2019 01:14
@davidfowl
Copy link
Member

@Tratcher can you capture that in an issue so we don’t lose it

@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants