-
Notifications
You must be signed in to change notification settings - Fork 818
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 support for CONNECT method on HTTP/2 connection #9616
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to add an AuTest for this. In case it's helpful, Proxy Verifier supports the CONNECT method. Zhengxi added a test using it here:
https://github.com/apache/trafficserver/blob/master/tests/gold_tests/connect/connect.test.py#L112
I think it should work fine for HTTP/2, but of course let us know if it shows problems.
It doesn't seem like Proxy Verifier supports H2 CONNECT method. It doesn't allow me to only send And this might be because of my limited knowledge about ProxyVerifier, but it looks like requests and responses are tightly coupled by
|
Yes, you are correct. Thank you for the heads up @maskit. I filed a Proxy Verifier issue for this: I believe all other attributes of Proxy Verifier should work with this. @duke8253 added support for explicit DATA frames which may be helpful here (although your use of the
Yes, the Proxy Verifier server uses uuid to know what response to send. I think this will fit the HTTP/2 CONNECT use case well. In the body of the CONNECT, to keep things distinct for the reader of the test, I suggest using a different uuid than the parent one. Then add a separate transaction to tell the server how to handle that UUID request. This is similar to what is done here: But, as you noted already, things are a bit different with HTTP/2. Rather than the whole connection being a tunnel after the CONNECT, as it is with h1, only the DATA frames of the particular stream are tunneled. So the requests will have to happen on the client side via DATA frames, and the origin will parse them as regular h1 using the replay file's uuid transactions specified in the DATA frame/content nodes. Thanks for working on this. It's neat to get this feature working in ATS (and Proxy Verifier). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks helpful. Let's get your change in as is which the current autest suite shows didn't break anything. I'll plan to take your observations from the autest you started with Proxy Verifier and generate a future release of Proxy Verifier that supports HTTP/2 CONNECT requests. With that we can add an autest with Proxy Verifier as a separate PR.
) In #9616, @maskit wrote an H2 CONNECT Autest but couldn't include that in the final PR because of a Proxy Verifier issue. Now that the Proxy Verifier issue is resolved, the Autest is added in this PR(with a few tweaks). ATS crashes with the new test executing HTTP/2 tunneling traffic. This PR also includes a fix to resolve that.
These are cherry-picks from asf that has the fix for crash caused by H2 CONNECT. * Added http connect Autest with proxy verifier (apache#9315) * Added a HTTP CONNECT test using proxy verifier * Updated comment * added proxy-response verification (cherry picked from commit b5f2023) * Added Autest for H2 CONNECT and fix a crash triggered by the test (apache#9781) In apache#9616, @maskit wrote an H2 CONNECT Autest but couldn't include that in the final PR because of a Proxy Verifier issue. Now that the Proxy Verifier issue is resolved, the Autest is added in this PR(with a few tweaks). ATS crashes with the new test executing HTTP/2 tunneling traffic. This PR also includes a fix to resolve that. (cherry picked from commit df7ccfe) * Update to Proxy Verifier v2.8.1 (apache#9834) Proxy Verifier v2.8.1 has fixes for the way Proxy Verifier relates to HTTP/2 CONNECT method request pseudo header fields. This will be helpful for testing HTTP/2 CONNECT requests. (cherry picked from commit d5c47a7)
* asf/master: (33 commits) Add error log for invalid OCSP response (apache#9674) Add new settings to specify TLS versions to use (apache#9667) Remove flask from tests/Pipfile (apache#9688) Doc: Add example of --enable-lto build option with LLVM (apache#9654) Added Zhengxi to the asf contributors (apache#9685) Don't build native QUIC implementation (apache#9670) Stabilize autest tls_hooks17 (apache#9671) Cleanup: remove ts::buffer from HostDB. (apache#9677) Fix leak in MultiTextMod in ControlBase. (apache#9675) Cleanup: remove TsBuffer.h from MIME.cc (apache#9661) Cleanup: remove ts::Buffer from ControlBase. (apache#9664) setup pre-commit hook at cmake generation time (apache#9669) Updated parent retry attempt logic (apache#9620) Try to do less work in hot function HttpHookState::getNext (apache#9660) cmake build, fixed warning using older openssl APIs (apache#9648) rename attempts to retry_attempts (apache#9655) OCSP: Fix unitialized variable error. (apache#9662) Add support for CONNECT method on HTTP/2 connection (apache#9616) Remove deprecated debug output functions from 10 source files. (apache#9657) Remove deprecated debug output functions from 14 source files. (apache#9658) ...
This blog post motivated me to implement CONNECT method support.
https://daniel.haxx.se/blog/2023/04/14/curl-speaks-http-2-with-proxy/