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

http3: fix listening on both QUIC and TCP #3465

Merged
merged 1 commit into from
Aug 20, 2022

Conversation

mojatter
Copy link
Contributor

@mojatter mojatter commented Jun 30, 2022

Hi, I found an issue with the -tcp option in example not working.

% cd example
% go run ./main.go -tcp
% cd example/client
% go run ./main.go -q https://localhost:6121/demo/tile
GET https://localhost:6121/demo/tile
client Starting new connection to localhost ([::]:56076 -> 127.0.0.1:6121), source connection ID (empty), destination connection ID 3b2948435331ca22, version v1
Got response for https://localhost:6121/demo/tile: &http.Response{Status:"404 Not Found", StatusCode:404, Proto:"HTTP/3.0", ProtoMajor:3, ProtoMinor:0, Header:http.Header{"Content-Type":[]string{"text/plain; charset=utf-8"}, "X-Content-Type-Options":[]string{"nosniff"}}, Body:(*http3.hijackableBody)(0x14000298040), ContentLength:-1, TransferEncoding:[]string(nil), Close:false, Uncompressed:false, Trailer:http.Header(nil), Request:(*http.Request)(nil), TLS:(*tls.ConnectionState)(0x140002aa000)}
Response Body: 19 bytes
client Closing connection with error: Application error 0x100
client Connection 3b2948435331ca22 closed.

As shown above, a request for QUIC returns 404 Not Found.
The cause of the problem is that the http3.Server is not configured with a Handler.

Applying this pull request will return 200 OK as follows.

% go run ./main.go -q https://localhost:6121/demo/tile
GET https://localhost:6121/demo/tile
client Starting new connection to localhost ([::]:64885 -> 127.0.0.1:6121), source connection ID (empty), destination connection ID e35d6be0ca052bac51b466c0717d63a9df45f5b9, version v1
Got response for https://localhost:6121/demo/tile: &http.Response{Status:"200 OK", StatusCode:200, Proto:"HTTP/3.0", ProtoMajor:3, ProtoMinor:0, Header:http.Header{}, Body:(*http3.hijackableBody)(0x14000032080), ContentLength:-1, TransferEncoding:[]string(nil), Close:false, Uncompressed:false, Trailer:http.Header(nil), Request:(*http.Request)(nil), TLS:(*tls.ConnectionState)(0x14000102000)}
Response Body: 83 bytes
client Closing connection with error: Application error 0x100
client Connection e35d6be0ca052bac51b466c0717d63a9df45f5b9 closed.

@google-cla
Copy link

google-cla bot commented Jun 30, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@marten-seemann marten-seemann self-requested a review July 27, 2022 14:25
Copy link
Member

@marten-seemann marten-seemann left a comment

Choose a reason for hiding this comment

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

Sorry for the long delay! This looks good to me.

@marten-seemann marten-seemann changed the title Fix listen TLS and QUIC both http3: fix listening on both QUIC and TCP Aug 20, 2022
@marten-seemann
Copy link
Member

@mojatter Can you please sign the CLA (see #3465 (comment)). We'd need that so we can merge the PR.

@mojatter
Copy link
Contributor Author

@marten-seemann Thank you. I fixed it.

@marten-seemann
Copy link
Member

Thank you!

@marten-seemann marten-seemann merged commit dd521c0 into quic-go:master Aug 20, 2022
@mojatter mojatter deleted the fix-listen-tls-quic-both branch September 6, 2022 03:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants