-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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 post request with body failed #2551
Comments
I don't understand what you're trying to say here. Can you please
|
1b. Caddy version (run caddy version or paste commit SHA) 1c. Go version (if building Caddy from source; run go version) 1d. curl version
2b. Why it's a bug (if it's not obvious) curl -H "Content-Type:application/json" -v "https://192.168.94.48:8080/g/demo-def-t2-server/9.0/testField1k" -d '{"name":"mmm"}' -k curl --http3 -H "Content-Type:application/json" -X POST -v "https://192.168.94.48:8080/g/demo-def-t2-server/9.0/testField1k" -d "{"name":"mmm"}" 2c. Log output
:8080 { 3c.Make an HTTP request: curl --http3 -H "Content-Type:application/json" -X POST -v "https://192.168.94.48:8080/g/demo-def-t2-server/9.0/testField1k" -d "{"name":"mmm"}" |
when I debug for problem reason, I found this: if t.BodyCloser != nil { if err := t.BodyCloser.Close(); err != nil { return err } } ` the function s.sendStream.Close() will call func (s *sendStream) Close() error , this function call |
Sorry, this is getting even less comprehensible than it was before. I have literally no idea what you're trying to say here. |
@ggjjlldd To post code blocks, wrap the code with ``` on the lines before and after the code block.
For inline code It's nearly impossible to read your post without proper formatting. |
the problem description1. Environment
2. Description
Use http1.1 such as:
but use http3 such as
the different is here, http3 no response body return
3. Tutorial (minimal steps to reproduce the bug)
|
Please go read https://guides.github.com/features/mastering-markdown/ this is killing me You're using > for quotes. Don't do that. Quotes don't preserve whitespace. Use ``` for code blocks. |
the reasonwhen I debug this problem , I found reason:
send_stream.go function:
|
@francislavoie please help me! |
@ggjjlldd I don't see anything wrong with that code. That's how we close a stream in quic-go. Your "minimal" example is not really minimal either. I doubt that you need to set keep-alives, buffer sizes, gzip encoding and certificates to make this work (self-signed cert should be fine here). Can you please clean it up to make it actually a minimal example, so we can run this locally? |
It not "minimal" example, you can use curl to test. My quiche client has some result with curl. May be it is not problems with quic-go, But use quic-go in caddy will make error. |
Your Caddyfile is far from minimal, and requires more preparation than necessary (for example, I'd have to generate a certificate first). Can you please post a minimal Caddyfile that reproduces the problem? |
Just tried to set it up despite the totally messy Caddyfile, and noticed that there's a reverse proxy directive in that file. @ggjjlldd You need to provide us a minimal configuration, otherwise we can't help you. |
Yeah, sorry, I'm with @marten-seemann on this one -- I don't really understand how to continue debugging this. For a minimal example, we need to reproduce the problem without reverse proxy. If you do make a change to the code that fixes it, we'd be happy to see it, but a reason explaining the change would be needed too. |
Thanks for this. Testing http3 with firefox and seeing the same behavior. Reverse proxying various services and while http3 get requests are working, http3 posts are not. Consequently, I'm not able to login to any of these services. Hard to imagine a scenario without a reverse proxy that would involve a post request.. |
How can we minimally reproduce it? |
May require reverse proxying to something that has an endpoint that accepts a post request. I did a test in firefox posting to a static html endpoint (default caddy index), which worked fine. I'll need to compile curl with http3 support to do more testing. |
@marten-seemann @mholt The post request failed must use reverse proxy directive. Because caddy use golang HTTP library will result such problems. So I can not give you a minimal caddyfile. It must include reverse proxy directive. |
Okay, so help me catch myself up on the current state of the issue: Is it a bug in the reverse proxy or the quic library? If it works over http2 and http1, why does http3 break it? |
So I debug for reason. Find quic-go body.Close() function will close send stream and cancel context. It will result the request be canceled. |
But golang http library will use body.Closer when read body done. It call quic-go body.Close() will result cancel context. |
@ggjjlldd I see, so you're saying that body.Close() is being called too early and/or unnecessarily by quic-go, because the go standard library will (indirectly) do it later when the response body is closed. Did I get that right? I'm not very familiar with this part of the quic-go code base, maybe @marten-seemann would have some intuition there. If you remove the extra call to Close() in quic-go and try again, does that solve the problem? (Even if it is the wrong solution, I'm interested if you can confirm that fixes it for you.) |
file: go/src/net/http/transfer.go
@mholt if I remove the extra call to Close() in quic-go, it be fine. and I solve my problems. |
@ggjjlldd Great, thanks for confirming. @marten-seemann Is it possible that quic-go is closing a stream that should be closed later by the Go standard library when the response body is closed? |
Which |
@marten-seemann I dunno if this is what @ggjjlldd was referring to, but I'm guessing it's this stack:
on v0.15.7 (or at least, that's what my go.mod shows currently). However, I too would like to know exactly which Close() call is removed that makes it work. Of course, the trick is to remove the right Close() call... Or, maybe it's just a matter of not cancelling a context (again, judging from above posts)?
|
Yeah right, if you remove the call to |
@marten-seemann you are right. So I need your help to fix . Maybe close quic-go stream later. |
@ggjjlldd I still don't understand the problem, and you're not helping me understand it. I'm sorry, but I can't help you with this kind of information you're giving me. |
the problems happened because use caddy reverseproxy directive. reverseproxy module call net/http library to transport request. But when call transport file use function writeBody will result close body.Close body will result close quic-go stream . It will result canceled context. and request will be canceled. @marten-seemann |
And any other you do not understand? @marten-seemann |
@ggjjlldd I think Marten is saying that we can't just remove the Close() call -- doing so might hide the error you're seeing, but not closing it probably results in a resource leak. Since we still don't fulllllly understand the problem (since it is hard for us to reproduce it), can you suggest another solution that doesn't leak resources? |
@mholt @marten-seemann
|
why quic-go close stream when body read done??? I think it is not reasonable. When connection timeout or fin stream arrive, It will be close stream. @marten-seemann @mholt |
you can use quiche client to reproduce it. the quiche link : https://github.com/cloudflare/quiche, it is a http3 library and include http3 client tool @mholt @marten-seemann
use caddy version:2.0.0
|
But without body is ok
|
Hi @mholt, I think I'll need your help on this one. I used @ggjjlldd's Caddyfile in #2551 (comment) (thank you, @ggjjlldd!). For the backend server I used a simple PHP script: <?php
print_r($_POST);
print_r($_SERVER);
?> Making a HTTP/3 request including a body indeed leads to a connection timeout, but I'm not exactly sure why. It looks like
@mholt Do you have any idea what is going wrong here? Closing the stream seems like the right thing to do, when the application is done writing the response body and calls |
I'll take a look -- at first pass, ctxCancel would only cancel a Background context, not any context that Caddy passes in. So that might be a red herring. @marten-seemann You're seeing a connection timeout -- isn't that something different than what is being reported initially (I'll be honest, I dunno -- I think the fundamental symptom is an empty POST body, but it's not clear to me if it's the upstream request body that's empty or if it's the reply to downstream that is empty.) @ggjjlldd You posted this at the very top:
This looks like a Caddy log but logfmt doesn't support nested objects. Can you please disable the logfmt encoding and use the default log formatting so we can get the full output, then please post the full log output (it should look like JSON)? |
I wrote a client to reproduce the bug in Go, so we don't have to rely on quiche here. I don't know how sophisticated their client implementation is - there's no reason to time out on in the situation, and in fact quic-go wouldn't do that. The client is here: https://gist.github.com/marten-seemann/1cb0d87a43935c377c9e59a49c3dbc71. You should be able to just Usage:
Performs a POST request via HTTP/2 / HTTP/1.1.
Performs the same POST request via HTTP/3. It also outputs a qlog. qlogs can be loaded in qvis to inspect the trace. Not sure if that really helps us debug this issue though, all I see is the client sending the request (on stream 0) and then sending a FIN, whereas the server sends a FIN at offset 0. @mholt Any idea how to further debug this? |
@marten-seemann Thanks for doing this, I'll try to take a look after the holiday! |
@mholt Any update on this? |
@marten-seemann That custom client you whipped up is really handy. I was able to make it more minimally reproducible, all you need is this Caddyfile (no external backend required):
I confirmed that the Go standard libarary's I have also confirmed that it works with HTTP/1 and HTTP/2 but does not work with HTTP/3. If it helps to know, this is where we set up the HTTP/3 server: https://sourcegraph.com/github.com/caddyserver/caddy@9415feca7cf0fe14b9d881c7318be2da20b2985f/-/blob/modules/caddyhttp/app.go#L325-347 - feel free to let me know if I did anything wrong there. |
@mholt That's neat. Hadn't thought of using Caddy-run backend. But of course, why not...
I don't see any context in that code snippet. Which context is causing the problem?
That looks correct, as far as I can tell. |
I don't know. The context wouldn't be in that code snippet, since it's an |
We set a request context: But we don't cancel any contexts in the entire HTTP/3 package. We do cancel (and close) the underlying QUIC streams at various locations though. I'm wondering if this is getting translated into a request cancellation by the standard library somehow? |
Maybe; child contexts get canceled when their parent contexts do. So if the stream context is canceled, it will cancel the request, and the standard lib http.Transport will stop using it, I guess. |
Yes, the stream context is cancel when the send side of the stream is closed (or canceled). So that means we're back to a too early |
Perhaps -- either too early, or if it's not too early, it's possible that a second context is needed (one for send, one for receive sides of the stream). Traditionally, as you know, duplexed TCP has just 1 context: if the sender closes, the receiver does too. But in UDP that is not always the case. I don't know enough about it, but maybe cancelling the context for a send side should not also close the context for the receive side. But maybe that is more complicated. Hopefully it is as simple as a close too early :) |
I don't think the request context has anything to do with this. Even if I remove these 4 lines So I guess the cancelation of the context was a result of the closing of the stream, not the cause. So the question remains: Why is the stream closed too early? Unfortunately, I don't really know how the reverse proxy is implemented in Caddy, so I'm a bit lost at this point. @mholt Can you help me out here? What's the expected data flow when a request comes in? |
@marten-seemann It has less to do with how Caddy works and more to do with how the Go standard lib and quic-go interoperate; all the same requests work over HTTP/1 and HTTP/2. I think your guess is as good as mine at this point. Doing a little "binary search" through net/http/transport.go, I have confirmed that the request gets to this point: https://sourcegraph.com/github.com/golang/[email protected]/-/blob/src/net/http/transport.go#L571 with the error "context canceled." The error comes through this line: https://sourcegraph.com/github.com/golang/[email protected]/-/blob/src/net/http/transport.go#L565 Further bisecting leads to: https://sourcegraph.com/github.com/golang/[email protected]/-/blob/src/net/http/transport.go#L2471 Which comes from here: https://sourcegraph.com/github.com/golang/[email protected]/-/blob/src/net/http/transport.go#L2540-2543 Which of course comes from: https://sourcegraph.com/github.com/golang/[email protected]/-/blob/src/net/http/transport.go#L2495 So, it is simply a matter of the request's context being cancelled, likely somewhere in quic-go (since it doesn't happen with H2 or H1). I even tried adding |
Now I really don't understand any more what's going on here. You're linking to
I removed the only place where we're creating a request context in quic-go (see #2551 (comment)), so I doubt that a context cancellation is the cause of the issue.
I agree that it's probably a problem somewhere in quic-go (probably not context cancelation though), but with my limited understanding of what Caddy is actually doing here, I have no way of debugging this any further. |
@marten-seemann The request to the backend is not http3, the request to the proxy is. I don't understand why how caddy's reverse proxy works is relevant here at all. It proxies http, regardless of protocol version or underlying transport layer. |
hi,How is the problem? |
this problem origin publish in caddy issue, but i think it is a quic go problems,
more info:caddyserver/caddy#3429
post request has body so can not return response. you can reference this link:
caddyserver/caddy#3429
i am a co-worker with @majc08149 , i find problem reason and code. But I do not know how to modify? @mholt
I paste problem code :
file->net/http/transfer.go:371
if t.BodyCloser != nil { if err := t.BodyCloser.Close(); err != nil { return err } }
the function BodyCloser.Close() call quic-go->http3->body.go Close() like :
`
func (r *body) Close() error {
// quic.Stream.Close() closes the write side, not the read side
if r.isRequest {
return r.str.Close()
}
r.requestDone()
r.str.CancelRead(quic.ErrorCode(errorRequestCanceled))
return nil
}
`
this call quic-go->stream.go function 👍
func (s *stream) Close() error { if err := s.sendStream.Close(); err != nil { return err } return nil }
the function s.sendStream.Close() will call func (s *sendStream) Close() error , this function call
s.ctxCancel() to cancel context.
so the request reverseproxy will be canceled.
please help me how to fix this bug?
The text was updated successfully, but these errors were encountered: