Skip to content

Add http/2 support to HTTPBin #382

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 3 commits into from
Jun 25, 2021

Conversation

fabianfett
Copy link
Member

@fabianfett fabianfett commented Jun 24, 2021

This PR adds http/2 support to the HTTPBin test utility.

Note regarding semver:

This PR adds swift-nio-http2 as a dependency.

Modifications

  • Removed obscure initialization options for HTTPBin
    • channelPromise: EventLoopPromise<Channel>? = nil was only used once (in testUploadStreamingBackpressure).
    • connectionDelay: TimeAmount = .seconds(0) was never used
    • maxChannelAge: TimeAmount? = nil was never used
  • The initialization options ssl: Bool = false, compress: Bool = false, refusesConnections: Bool = false have moved into a Mode enum. The mode might be .http1_1(ssl: Bool, compress: Bool), .http2(compress: Bool), or .refuse
  • Added support for http/2 (not used in any test yet, however I verified the support with curl locally.)
  • channel pipeline modifications are nearly all synchronous.
  • the HTTPBin's request handler can be changed. This allows testing of more esoteric cases without having to create a new server every time. The default HTTPBin handler continues to use the HTTPBinHandler.

Result

  • Easier test utils that can be used for http/2

Follow ups

  • I also wanted to add a ServerQuiescingHelper to ensure that all connections are closed when the test ends. However we often seem to leak connections when we cancel tasks. This would make our CI very flaky. I hope to land this at a later point once all connection and connection pool improvements have landed (See [RFC] ConnectionPool rewrite and http/2 support #376).

@fabianfett fabianfett added the 🆕 semver/minor Adds new public API. label Jun 24, 2021
@fabianfett fabianfett added this to the 1.5.0 milestone Jun 24, 2021
@fabianfett fabianfett requested a review from weissi June 24, 2021 15:15
Copy link
Collaborator

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

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

Looks good aside from a typo.

private func syncAddHTTPProxyHandlers(
to channel: Channel,
connectionID: Int,
expectedAuthroization: String?
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo: expectedAuthroization -> expectedAuthorization

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed!

@fabianfett fabianfett force-pushed the ff-prepare-test-utils-for-h2 branch from 2207098 to c4c7212 Compare June 24, 2021 18:54
@Davidde94
Copy link
Collaborator

@swift-server-bot test this please

@fabianfett fabianfett force-pushed the ff-prepare-test-utils-for-h2 branch from 4653af5 to 3bf080c Compare June 25, 2021 09:45
@fabianfett fabianfett merged commit af837ed into swift-server:main Jun 25, 2021
@fabianfett fabianfett deleted the ff-prepare-test-utils-for-h2 branch June 25, 2021 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🆕 semver/minor Adds new public API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants