http2 cleartext support #1170
Closed
http2 cleartext support #1170
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for this PR! The use of h2c on its own looks good, but I would prefer if it sits behind a feature flag and is not enabled by default. The caveat about memory consumption from tus/tus-js-client#700 (comment) is concerning enough to let user opt into this behavior. I can imagine that in the future we have multiple flags to enable different HTTP versions, e.g. |
|
Thanks @Acconut good point - pushed a new commit. Let me know if it addresses your concern. |
|
Thanks for the updates! I will try them out on my own in about two weeks |
Acconut
pushed a commit
that referenced
this pull request
Sep 10, 2024
Squashed commit of the following: commit 13957b3acc4cdfc0b3ec93d6eddf58bbaf00f4ad Merge: 624a36b d613243 Author: Marius Kleidl <[email protected]> Date: Tue Sep 10 09:04:14 2024 +0200 Merge branch 'main' into pr/1170 commit 624a36b1c8d55a8b823b4ea6558315208d2de2af Author: Marius Kleidl <[email protected]> Date: Tue Sep 10 09:00:14 2024 +0200 Improve documentation commit 91b84a9 Author: Kristoph Matthews <[email protected]> Date: Wed Aug 21 12:00:31 2024 -0400 explicit h2c flag commit ad5774d Author: Kristoph Matthews <[email protected]> Date: Tue Aug 20 15:52:02 2024 -0400 http2 cleartext support
|
Merged in dfaf9f7, thank you very much for this! |
|
This is released in https://github.com/tus/tusd/releases/tag/v2.5.0. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is in reference to this thread in the tus-js-client repository issues. @Acconut invited us to create a lightweight PR to solve this issue, on the tusd side.
PROBLEM
In GCP Cloud Run, HTTP/2 traffic is accepted but TLS is terminated at the load balancer and traffic is forwarded to the tusd container as cleartext (non-encrypted) http. This is problematic because tusd expects TLS for HTTP/2 traffic.
SOLUTION
Golang's net/http2 library allows for a drop in support of HTTP/2 in cleartext (h2c), with simultaneous capability for HTTP/1.1 cleartext. Meaning, if traffic is identified to be HTTP/1.1, the h2c handler will bypass HTTP/2 and serve based on HTTP/1.1.
This PR uses the
newHandler()method to wrap the existingmuxhandler, such that no additional configuration is needed to handle either HTTP version. HTTP/2 encrypted traffic can also be handled as usual by making calls directly to tusd via https. If behind a load balancer such as Cloud Run, all that would be required in terms of configuration is the-behind-proxyflag.TESTING
We built an image based on this PR and tested it for file uploads on Cloud Run with tus-js-client and it performed successfully.