From 7c96810d3b81758310f6c2ef964df1e446ac0a4a Mon Sep 17 00:00:00 2001
From: mhx <mathieu@hanotaux.fr>
Date: Fri, 4 Sep 2026 11:00:41 +0200
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20do=20not=20drop=20bodyless=20DEL?=
 =?UTF-8?q?ETE=20over=20HTTP/3?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fix #385.

Over HTTP/3 a bodyless request cannot be told apart from one carrying an
unreadable body. Go's HTTP/2 server can make that distinction -- it only sets
ContentLength to -1 when the client left the stream open for DATA frames, so a
request that ends at the headers keeps ContentLength 0 and isBodyUnreadable
stays false (x/net/http2/server.go, `bodyOpen := !f.StreamEnded()`). quic-go has
no equivalent: http3/server_conn.go assigns req.Body unconditionally and
http3/headers.go defaults ContentLength to -1 whenever the Content-Length header
is absent, so every bodyless HTTP/3 request looks unreadable regardless of
method.

With DELETE in isMethodWithBody, that turns an ordinary
fetch(url, {method: "DELETE"}) into a 403 on HTTP/3 while the same call succeeds
on HTTP/2. dani reported this on #352 before it was merged.

A DELETE body is legal but has no defined semantics (RFC 9110 9.3.5), and the
gRPC streams #323/#332 guard against are always POST, so dropping DELETE from
the list costs no protection there. Note this diverges from lua-cs-bouncer's
METHODS_WITH_BODY, which still lists DELETE; the same false positive likely
applies there.

Adds Test_appsecQuery_unreadableBodyMethods, which pins the behaviour for all
seven methods and fails on the previous code for DELETE. isMethodWithBody had no
direct coverage before.
---
 bouncer.go      |  2 +-
 bouncer_test.go | 63 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/bouncer.go b/bouncer.go
index d3837b4..e55a425 100644
--- a/bouncer.go
+++ b/bouncer.go
@@ -746,7 +746,7 @@ func isBodyUnreadable(httpReq *http.Request) bool {
 // isMethodWithBody used only when isBodyUnreadable returns true but the request method can't have body.
 func isMethodWithBody(method string) bool {
 	switch method {
-	case http.MethodPost, http.MethodPut, http.MethodPatch, http.MethodDelete:
+	case http.MethodPost, http.MethodPut, http.MethodPatch:
 		return true
 	default:
 		return false
diff --git a/bouncer_test.go b/bouncer_test.go
index 00b2f10..a64873d 100644
--- a/bouncer_test.go
+++ b/bouncer_test.go
@@ -600,3 +600,66 @@ func Test_appsecQuery_reusesConnection(t *testing.T) {
 		})
 	}
 }
+
+func newUnreadableRequest(method string, done <-chan struct{}) *http.Request {
+	req, _ := http.NewRequest(method, "http://localhost/api/admin/reservations/8fff14a2", blockingBody{done: done})
+	req.ProtoMajor = 3
+	req.ContentLength = -1
+	return req
+}
+
+func Test_appsecQuery_unreadableBodyMethods(t *testing.T) {
+	appsecServer := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, _ *http.Request) {
+		rw.WriteHeader(http.StatusOK)
+	}))
+	defer appsecServer.Close()
+
+	appsecURL, _ := url.Parse(appsecServer.URL)
+
+	tests := []struct {
+		method      string
+		wantDropped bool
+	}{
+		{method: http.MethodGet, wantDropped: false},
+		{method: http.MethodHead, wantDropped: false},
+		{method: http.MethodOptions, wantDropped: false},
+		{method: http.MethodDelete, wantDropped: false},
+		{method: http.MethodPost, wantDropped: true},
+		{method: http.MethodPut, wantDropped: true},
+		{method: http.MethodPatch, wantDropped: true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.method, func(t *testing.T) {
+			bouncer := &Bouncer{
+				appsecScheme:              appsecURL.Scheme,
+				appsecHost:                appsecURL.Host,
+				appsecPath:                "/",
+				appsecBodyLimit:           10485760,
+				appsecUnreadableBodyBlock: true,
+				httpAppsecClient:          appsecServer.Client(),
+				log:                       logger.New("INFO", ""),
+			}
+
+			done := make(chan struct{})
+			defer close(done)
+
+			finished := make(chan error, 1)
+			go func() {
+				finished <- appsecQuery(bouncer, "1.2.3.4", newUnreadableRequest(tt.method, done))
+			}()
+
+			select {
+			case err := <-finished:
+				if tt.wantDropped && err == nil {
+					t.Errorf("appsecQuery() on an unreadable-body %s: expected the request to be dropped, got nil", tt.method)
+				}
+				if !tt.wantDropped && err != nil {
+					t.Errorf("appsecQuery() on a bodyless HTTP/3 %s returned error: %v", tt.method, err)
+				}
+			case <-time.After(2 * time.Second):
+				t.Fatalf("appsecQuery() blocked on an unreadable %s body", tt.method)
+			}
+		})
+	}
+}