Add --http3 flag to buf curl #3127

sudorandom · 2024-07-04T18:02:05Z

This PR adds a new flag to buf curl, --http3 which forces buf curl to use HTTP/3 as the transport. There's no open issue for this that I can find to reference.

A couple notes:

This uses the quic-go library to do the heavy lifting. Once http3 is incorporated into net/http, some of this code can be removed.
HTTP/3 requires TLS, so it's invalid to pass --insecure and --http3
Simply enabling HTTP/3 for the transport doesn't work with gRPC but appears to work with gRPC-Web and Connect. The error I get is: {"code": "internal", "message": "protocol error: no Grpc-Status trailer: unexpected EOF"} since quic-go does not yet support HTTP Trailers.
I'm not 100% how to properly test unix sockets with HTTP3 so I just didn't implement that and instead return an error if the user gives --http3 and --unix-socket
I tried to maintain the same amount of logging (which I've enjoyed very much for diagnosing issues with gRPC in the past).
Alt-Svc is a header intended to inform clients (read: browsers) that alternative ways of accessing the website exists. So browsers may use HTTP/2 a few times before trying HTTP/3 if it's hinted in Alt-Srv. This PR does nothing with this, I just figured it's interesting to note that HTTP/3 "discovery" is possible without just trying and hoping it works.

Here's an example of using this against the demo.connectrpc endpoint (which, yes! does work with HTTP/3 already thanks to Google's load balancer)

$ buf curl --http3 --schema=buf.build/connectrpc/eliza \
    -d '{"sentence": "Hello world!"}' \
    https://demo.connectrpc.com/connectrpc.eliza.v1.ElizaService/Say -v
buf: * Invoking RPC connectrpc.eliza.v1.ElizaService.Say
buf: * Dialing (udp) demo.connectrpc.com:443...
buf: * ALPN: offering h3
buf: * TLS connection using TLS 1.3 / TLS_AES_128_GCM_SHA256
buf: * ALPN: server accepted h3
buf: * Server certificate:
buf: *   subject: CN=demo.connectrpc.com
buf: *   start date: 2024-05-08 10:34:52 +0000 UTC
buf: *   end date: 2024-08-06 11:28:46 +0000 UTC
buf: *   subjectAltNames: [demo.connectrpc.com]
buf: *   issuer: CN=GTS CA 1D4,O=Google Trust Services LLC,C=US
buf: * Server certificate chain verified
buf: * Server certificate is valid for demo.connectrpc.com
buf: * Connected to 34.102.161.239:443
buf: > (#1) POST /connectrpc.eliza.v1.ElizaService/Say
buf: > (#1) Accept-Encoding: gzip
buf: > (#1) Connect-Protocol-Version: 1
buf: > (#1) Connect-Timeout-Ms: 119684
buf: > (#1) Content-Type: application/proto
buf: > (#1) User-Agent: connect-go/1.16.2 (go1.22.4) buf/1.34.1-dev
buf: > (#1)
buf: } (#1) [14 bytes data]
buf: * (#1) Finished upload
buf: < (#1) HTTP/3.0 200 OK
buf: < (#1) Accept-Encoding: gzip
buf: < (#1) Alt-Svc: h3=":443"; ma=2592000,h3-29=":443"; ma=2592000
buf: < (#1) Content-Length: 34
buf: < (#1) Content-Type: application/proto
buf: < (#1) Date: Thu, 04 Jul 2024 18:19:08 GMT
buf: < (#1) Server: Google Frontend
buf: < (#1) Traceparent: 00-4c36ad3f37f33494aa6e414709bba87b-82503068c947eb97-00
buf: < (#1) Vary: Origin
buf: < (#1) Via: 1.1 google, 1.1 google
buf: < (#1) X-Cloud-Trace-Context: 4c36ad3f37f33494aa6e414709bba87b/9390058449679149975
buf: < (#1)
buf: { (#1) [34 bytes data]
buf: * (#1) Call complete
{
  "sentence": "Hello there...how are you today?"
}

CLAassistant · 2024-07-04T18:02:10Z

All committers have signed the CLA.

sudorandom · 2024-07-04T18:10:17Z

go.mod

@@ -1,6 +1,8 @@
 module github.com/bufbuild/buf

-go 1.20
+go 1.21


It seems like adding the github.com/quic-go/quic-go dependency bumped this up. I'm unsure why. If it's a problem I can look into this.

It's a problem for a few more weeks - we need to keep compat back for three versions for our enterprise customers who install from source. Can either try to fix now, or just hold off trying to merge this until 1.23 is released - up to you.

I think we can just wait. There's absolutely no rush for me.

bufdev

Looks pretty reasonable to me, defering to @jhump for a further review.

As it is now, we're blocked on Go 1.23 being released, as we need to support the previous three versions of Go, see comment on PR. This PR either needs to be 1.20-compatible, or we can just wait a few weeks - up to you @sudorandom.

private/buf/cmd/buf/command/curl/curl.go

bufdev · 2024-08-01T16:29:40Z

Note: blocked on Golang v1.23.0 and #3123 being merged.

jhump · 2024-08-01T16:35:05Z

Note: blocked on Golang v1.23.0 and #3123 being merged.

Would also be good to see quic-go/quic-go#4581 merged first. But it's not really a blocker (if quic-go project proves too slow to accept that patch, we can proceed here without i).

bufdev · 2024-08-14T16:44:05Z

@sudorandom can you merge in main so we can check this out?

sudorandom · 2024-08-15T18:44:18Z

@bufdev I've rebased, so let me know what you think!

jhump

Looks great! Just a couple of minor remarks.

jhump · 2024-07-08T15:35:42Z

private/buf/bufcurl/tls.go

@@ -38,6 +38,8 @@ type TLSSettings struct {
 	// "h2", and exclude "http/1.1". If the server does not pick a
 	// protocol, "h2" is assumed as the default.
 	HTTP2PriorKnowledge bool
+
+	HTTP3 bool


nit: doc comment

Added a comment!

jhump · 2024-08-20T16:42:44Z

private/buf/cmd/buf/command/curl/curl.go

+		if f.HTTP3 {
+			return makeHTTP3Client(f, bufcurl.GetAuthority(host, requestHeaders), container.VerbosePrinter())
+		}


Maybe instead have rename makeHTTPClient -> makeHTTPRoundTripper, and have it delegate to makeHTTP3RoundTripper internally. Then you can factor the two calls to bufcurl.NewVerboseHTTPClient into a single call right here.

I believe I've implemented your suggestion. I agree, it looks a bit better!

jhump

LGTM!

sudorandom commented Jul 4, 2024

View reviewed changes

bufdev changed the title ~~Adds --http3 flag to buf curl~~ Add --http3 flag to buf curl Jul 6, 2024

bufdev reviewed Jul 8, 2024

View reviewed changes

private/buf/cmd/buf/command/curl/curl.go Outdated Show resolved Hide resolved

sudorandom force-pushed the http3 branch from 60a1d60 to d5dcf4e Compare August 1, 2024 17:35

sudorandom force-pushed the http3 branch from d5dcf4e to f9a8395 Compare August 14, 2024 16:47

bufdev requested a review from jhump August 15, 2024 18:45

sudorandom force-pushed the http3 branch 3 times, most recently from ab78e8d to c88d49f Compare August 16, 2024 15:31

jhump reviewed Aug 20, 2024

View reviewed changes

sudorandom added 2 commits August 20, 2024 19:04

Adds --http3 flag to buf curl

4c42509

address PR notes

80c9209

sudorandom force-pushed the http3 branch from c88d49f to 80c9209 Compare August 20, 2024 17:04

trivial: add a bit more to the http3 comment

850ded9

sudorandom requested a review from jhump August 20, 2024 17:51

jhump approved these changes Aug 21, 2024

View reviewed changes

bufdev merged commit 3deb7e8 into bufbuild:main Aug 21, 2024
3 checks passed

sudorandom deleted the http3 branch August 21, 2024 18:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add --http3 flag to buf curl #3127

Add --http3 flag to buf curl #3127

sudorandom commented Jul 4, 2024 •

edited

Loading

CLAassistant commented Jul 4, 2024 •

edited

Loading

sudorandom Jul 4, 2024

bufdev Jul 6, 2024

sudorandom Jul 8, 2024

bufdev left a comment

bufdev commented Aug 1, 2024

jhump commented Aug 1, 2024

bufdev commented Aug 14, 2024

sudorandom commented Aug 15, 2024

jhump left a comment

jhump Jul 8, 2024

sudorandom Aug 20, 2024

jhump Aug 20, 2024

sudorandom Aug 20, 2024

jhump left a comment

Add --http3 flag to buf curl #3127

Add --http3 flag to buf curl #3127

Conversation

sudorandom commented Jul 4, 2024 • edited Loading

CLAassistant commented Jul 4, 2024 • edited Loading

sudorandom Jul 4, 2024

Choose a reason for hiding this comment

bufdev Jul 6, 2024

Choose a reason for hiding this comment

sudorandom Jul 8, 2024

Choose a reason for hiding this comment

bufdev left a comment

Choose a reason for hiding this comment

bufdev commented Aug 1, 2024

jhump commented Aug 1, 2024

bufdev commented Aug 14, 2024

sudorandom commented Aug 15, 2024

jhump left a comment

Choose a reason for hiding this comment

jhump Jul 8, 2024

Choose a reason for hiding this comment

sudorandom Aug 20, 2024

Choose a reason for hiding this comment

jhump Aug 20, 2024

Choose a reason for hiding this comment

sudorandom Aug 20, 2024

Choose a reason for hiding this comment

jhump left a comment

Choose a reason for hiding this comment

sudorandom commented Jul 4, 2024 •

edited

Loading

CLAassistant commented Jul 4, 2024 •

edited

Loading