diff --git a/s3api/middlewares/drain-request-body.go b/s3api/middlewares/drain-request-body.go index 25c6b02e..396041f9 100644 --- a/s3api/middlewares/drain-request-body.go +++ b/s3api/middlewares/drain-request-body.go @@ -81,11 +81,12 @@ func drainRequestBody(ctx fiber.Ctx) { // body, but not for a chunked one: past the terminating chunk it goes back // to the socket for another chunk header that will never come. Reading a // chunked body the handler already finished would block until the deadline - // and hold the response back with it, so only Content-Length framing is - // drained. Nothing is lost for the aws-chunked uploads this exists for -- - // STREAMING-* payloads carry a Content-Length. + // and hold the response back with it, so do not drain chunked framing. The + // stream may still have unread bytes, though, so the connection must not be + // reused for another request. cLength := ctx.Request().Header.ContentLength() - if cLength <= 0 { + if cLength < 0 { + ctx.Response().Header.SetConnectionClose() return } diff --git a/s3api/middlewares/drain-request-body_test.go b/s3api/middlewares/drain-request-body_test.go index 293217e1..861ce24c 100644 --- a/s3api/middlewares/drain-request-body_test.go +++ b/s3api/middlewares/drain-request-body_test.go @@ -206,6 +206,42 @@ func TestDrainRequestBody_doesNotStallAChunkedBodyTheHandlerFinished(t *testing. } } +func TestDrainRequestBody_closesConnectionForUnreadChunkedBody(t *testing.T) { + addr := startEarlyResponder(t, false) + + conn, err := net.Dial("tcp", addr) + if err != nil { + t.Fatalf("dial: %v", err) + } + defer conn.Close() + if err := conn.SetDeadline(time.Now().Add(30 * time.Second)); err != nil { + t.Fatalf("set deadline: %v", err) + } + + // Leave one decoded byte unread after the handler's initial read. The + // middleware cannot safely probe for EOF on a chunked request. + chunk := bytes.Repeat([]byte("a"), handlerReadBytes+1) + body := fmt.Appendf(nil, "PUT /object HTTP/1.1\r\nHost: %s\r\nTransfer-Encoding: chunked\r\n\r\n%x\r\n", addr, len(chunk)) + body = append(body, chunk...) + body = append(body, []byte("\r\n0\r\n\r\n")...) + if _, err := conn.Write(body); err != nil { + t.Fatalf("write request: %v", err) + } + + resp, err := http.ReadResponse(bufio.NewReader(conn), nil) + if err != nil { + t.Fatalf("read response: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusBadRequest { + t.Fatalf("expected status %v, got %v", http.StatusBadRequest, resp.StatusCode) + } + if !resp.Close { + t.Fatal("expected 'Connection: close' for an unread chunked body") + } +} + // The same, for a Content-Length body: fasthttp reports EOF idempotently there, // so it is drained, but a handler that already finished it must not be delayed. func TestDrainRequestBody_doesNotStallABodyTheHandlerFinished(t *testing.T) {