rproxy, the proxy in front of each app, asks servewaf and fails open (closes #171)

`smallwebwaf rproxy` reads the first 64 KiB of each body, asks servewaf over
HTTP/1.1, and passes what servewaf allows to the app within its own limits. Any
other answer goes to the visitor as servewaf gave it. SWRP_DEFAULT_DISPOSITION
decides only when servewaf cannot be connected to, does not answer at all, or
does not answer within SWRP_TIMEOUT_MS. Only the methods SPEC.md lists pass on;
any other gets 405.

Duplication: internal/rproxy copies passing-on code from internal/proxy until
#172 removes the originals.
Rule suppressed: gosec G704 and G705; paralleltest on one test.
Judgement call: an answer over 1 MiB, or with a status below 100, gets 502.
Judgement call: every target not starting with one / gets 400.

Model: opus-5-5

README.md

@@ -353,8 +353,10 @@ pinned.
   each request, and passes nothing to an app (see "`servewaf`" in
   [`SPEC.md`](SPEC.md)).
 - `smallwebwaf rproxy` is the reverse proxy in front of one app that asks
-  `smallwebwaf servewaf` about each request, and keeps the site answering while
+  `smallwebwaf servewaf` about each request. When `servewaf` cannot be connected
-  `servewaf` is down. Its settings start with `SWRP_`, and it needs
+  to, does not answer at all, or does not answer within `SWRP_TIMEOUT_MS`,
+  `SWRP_DEFAULT_DISPOSITION` decides. Its default, `allow`, keeps the site
+  answering. The settings of `rproxy` start with `SWRP_`, and it needs
   `SWRP_WAF_BACKEND`, `servewaf`'s URL (see "`rproxy`" in [`SPEC.md`](SPEC.md)).
 - `docker stop` stops both processes at once. `smallwebwaf` gives the requests
   in progress five seconds to finish, so the app must keep answering for five

SPEC.md

@@ -1066,8 +1066,8 @@ request. It is the visitor's request as the visitor sent it:
 - a `Host` that is not empty and holds no IPv6 zone;
 - at most one `User-Agent`.
 
-A body whose length the visitor did not announce comes chunked, whatever the
+A body whose length the visitor did not announce comes chunked. Any other body
-method. Any other body comes with a `Content-Length` of the part it carries.
+comes with a `Content-Length` of the part it carries.
 
 `rproxy` sets these headers, replacing any the visitor sent, and adds no others:
 
@@ -1135,7 +1135,8 @@ Every answer but the last below carries `X-Smallwebwaf-Disposition`.
   place of a refusal while `SWWAF_BAN_RESPONSE` is `close`. `rproxy` closes the
   visitor's connection without an answer.
 - No such header, status `400`, for a question `servewaf` refuses. `rproxy`
-  treats this answer as it treats no answer at all.
+  gives the visitor its status, headers and body, as it does for any answer
+  without a valid `X-Smallwebwaf-Disposition`.
 
 In `observe` mode a request that `enforce` mode would refuse is answered
 `allow`, and its log line has `would_action`. A path under `/_smallwebwaf/` is
@@ -1150,9 +1151,14 @@ short.
 
 Once the visitor's answer has ended, `rproxy` sends `POST /_smallwebwaf/report`
 for each request `servewaf` answered with `allow`, `deny` or `close`. It sends
-none after a `400` or no answer. The report carries no `X-Smallwebwaf-Client`.
+none after:
-Its body is one JSON object with all of these fields, named as the request log's
+
-fields of the same meaning:
+- an answer without a valid `X-Smallwebwaf-Disposition`, such as a `400`;
+- an answer it cannot pass on (see "Without an answer from `servewaf`");
+- no answer.
+
+The report carries no `X-Smallwebwaf-Client`. Its body is one JSON object with
+all of these fields, named as the request log's fields of the same meaning:
 
 - `request_id`: the question's `X-Request-ID`;
 - `status`: what the visitor got, `0` if nothing, otherwise from `100` to `599`;
@@ -1242,8 +1248,10 @@ by a request and ban notes name the site by its host.
 
 `smallwebwaf rproxy` is the reverse proxy in front of one app. It asks
 `servewaf` about each request, and passes on what `servewaf` allows. It keeps
-nothing between requests and never touches disk. When `servewaf` is down, slow
+nothing between requests and never touches disk. When `servewaf` cannot be
-or overloaded, the site keeps answering.
+connected to, does not answer at all, or does not answer within
+`SWRP_TIMEOUT_MS`, `SWRP_DEFAULT_DISPOSITION` decides. Its default, `allow`,
+keeps the site answering.
 
 `rproxy` reads only its `SWRP_` settings, which `docs/settings.md` lists. One of
 them is required: `SWRP_WAF_BACKEND`, `servewaf`'s URL. It has no metrics, no
@@ -1262,8 +1270,9 @@ and health check.
    request" says. It removes any IPv6 zone.
 3. It makes a fresh request id, whatever id the request came with. `servewaf`
    and the app get it as `X-Request-ID`, and `rproxy`'s log line gives it.
-4. It refuses a request it cannot put to `servewaf` as the visitor sent it (see
+4. It refuses a request with a method it does not pass on, or one it cannot put
-   "Requests `rproxy` refuses itself").
+   to `servewaf` as the visitor sent it (see "Requests `rproxy` refuses
+   itself").
 5. It reads up to the first 64 KiB of the body into memory.
 6. It sends `servewaf` the question (see "The question"). It speaks HTTP/1.1 to
    `servewaf`, whatever `servewaf` offers. Go's HTTP/2 client would refuse to
@@ -1271,9 +1280,11 @@ and health check.
    question never carries the visitor's `Trailer`.
 7. It does what the answer says (see "The answer"):
     - `allow`: it passes the request to the app.
-    - `deny`: the visitor gets the answer's status, headers and body, less
-      `X-Smallwebwaf-Disposition`.
     - `close`: it closes the visitor's connection without an answer.
+    - Any other answer: the visitor gets its status, headers and body, less
+      `X-Smallwebwaf-Disposition`. That holds for a `deny`, for an error such as
+      `servewaf`'s `400` or a `5xx`, and for an answer without a valid
+      `X-Smallwebwaf-Disposition`.
 8. Once the answer has ended, it writes its log line. `docs/request-log.md`
    gives the fields. An app's answer can end just after the line: Go's server
    sends the closing chunk of a chunked answer, and may send all of an empty
@@ -1286,11 +1297,27 @@ and health check.
 
 ### Requests `rproxy` refuses itself
 
-`SWRP_DEFAULT_DISPOSITION` applies only when `servewaf` itself is down, slow or
+`SWRP_DEFAULT_DISPOSITION` applies only when `servewaf` cannot be connected to,
-answers badly. A visitor can make `servewaf` slow. A request that keeps
+does not answer at all, or does not answer within `SWRP_TIMEOUT_MS`. A visitor
-`servewaf` busy past `SWRP_TIMEOUT_MS`, like any request while `servewaf` is
+can make `servewaf` slow. A request that keeps `servewaf` busy past
-overloaded, gets `SWRP_DEFAULT_DISPOSITION`. `deny` closes this, at the cost of
+`SWRP_TIMEOUT_MS`, like any request while `servewaf` is overloaded, gets
-answering `503` whenever `servewaf` is slow.
+`SWRP_DEFAULT_DISPOSITION`. `deny` closes this, at the cost of answering `503`
+whenever `servewaf` is slow. Making such requests hard to send is
+https://git.eeqj.de/sneak/smallwebwaf/issues/182.
+
+`rproxy` passes on only these methods:
+
+- `GET`
+- `HEAD`
+- `POST`
+- `OPTIONS`
+- `PUT`
+- `DELETE`
+- `PATCH`
+- `QUERY`
+
+It answers any other method `405`, with no question and no report. The answer's
+`Allow` header names these methods.
 
 `rproxy` answers `400`, with no question and no report, to a request with:
 
@@ -1313,15 +1340,15 @@ log line has status `0`.
 
 ### Without an answer from `servewaf`
 
-`SWRP_DEFAULT_DISPOSITION` decides what is done with the request when
+`SWRP_DEFAULT_DISPOSITION` decides what is done with the request only when
 `servewaf`:
 
+- cannot be connected to;
+- does not answer at all;
 - does not answer within `SWRP_TIMEOUT_MS`, which covers connecting, sending the
-  question and reading the whole answer;
+  question and reading the whole answer.
-- cannot be reached;
+
-- answers without a valid `X-Smallwebwaf-Disposition`, as with the `400` for a
+An answer whose body breaks off counts as no answer at all.
-  question it refuses;
-- sends an answer whose body breaks off or is over 1 MiB.
 
 `allow`, the default, passes the request to the app. `deny` answers `503`.
 Either way `rproxy` writes a process warning, its log line has
@@ -1330,6 +1357,11 @@ Either way `rproxy` writes a process warning, its log line has
 A path under `/_smallwebwaf/` that `allow` would pass on is answered `404`
 instead, so that the app never sees that prefix.
 
+`rproxy` cannot pass on an answer whose body is over 1 MiB or whose status is
+below `100`. The visitor gets `502` from `rproxy`, never the default. `rproxy`
+writes a process warning, its log line has no `disposition`, and it sends no
+report.
+
 A visitor that goes away while `servewaf` is being asked is no failure of
 `servewaf`. `rproxy` closes the connection, with no warning, no default and no
 report. Its log line has status `0` and no `disposition`.

docs/request-log.md

@@ -204,8 +204,11 @@ it to `servewaf`'s two. It has these fields and no others:
 - `duration_response`: milliseconds from then until the end of the answer.
 - `disposition`: what was done with the request: `allow`, `deny` or `close`. It
   is `servewaf`'s answer, or the value of `SWRP_DEFAULT_DISPOSITION`. It is left
-  out when `servewaf` was not asked, as for the health check, and when the
+  out when:
-  visitor went away while it was being asked.
+    - `servewaf` was not asked, as for the health check;
+    - the visitor went away while `servewaf` was being asked;
+    - `servewaf`'s answer had no valid `X-Smallwebwaf-Disposition`;
+    - `rproxy` could not pass on `servewaf`'s answer, and answered `502`.
 - `default_disposition`: `true` when `SWRP_DEFAULT_DISPOSITION` gave
   `disposition`, and otherwise left out.
 

docs/settings.md

@@ -341,9 +341,10 @@ A few limits are fixed rather than settings:
 - at most 20,000 anomaly counters are kept;
 - each alert destination's queue holds at most 1,000 alerts;
 - `servewaf` holds at most 10,000 answered requests awaiting their reports;
-- `rproxy` takes a request line and headers of at most 32 KiB, closes a
+- `rproxy` takes a request line and headers of at most 32 KiB, and closes a
-  kept-open connection idle for 120 seconds, and reads at most 1 MiB of the body
+  kept-open connection idle for 120 seconds;
-  of `servewaf`'s answer.
+- `rproxy` reads at most 1 MiB of the body of `servewaf`'s answer, and answers
+  `502` to a longer one.
 
 ## `servewaf`
 
@@ -376,9 +377,10 @@ A time here is a whole number of milliseconds above zero, such as `2000`.
   speaks HTTP/1.1 to it.
 - `SWRP_TIMEOUT_MS` (default `2000`): how long `rproxy` waits for `servewaf`'s
   answer, to a question and to a report. A time; never `off`.
-- `SWRP_DEFAULT_DISPOSITION` (default `allow`): what is done with a request
+- `SWRP_DEFAULT_DISPOSITION` (default `allow`): what is done with a request when
-  `servewaf` gives no answer about. `allow` passes it to the app, and `deny`
+  `servewaf` cannot be connected to, does not answer at all, or does not answer
-  answers `503`.
+  within `SWRP_TIMEOUT_MS`. `allow` passes it to the app, and `deny` answers
+  `503`. It does not apply to an answer `servewaf` gives, such as an error.
 - `SWRP_MAX_REQUEST_SIZE` (default `100M`): the largest request body. A size or
   `off`.
 - `SWRP_MAX_RESPONSE_SIZE` (default `1G`): the largest response body. A size or

internal/config/rproxy.go

@@ -27,8 +27,9 @@ type RProxy struct {
 	// Timeout is how long rproxy waits for servewaf's answer to a question,
 	// and to a report (SWRP_TIMEOUT_MS). It cannot be off.
 	Timeout time.Duration
-	// DefaultDisposition is what is done with a request servewaf gives no
+	// DefaultDisposition is what is done with a request when servewaf cannot be
-	// answer about, allow or deny (SWRP_DEFAULT_DISPOSITION).
+	// connected to, does not answer at all, or does not answer within Timeout:
+	// allow or deny (SWRP_DEFAULT_DISPOSITION).
 	DefaultDisposition string
 	// MaxRequestSize is the largest request body (SWRP_MAX_REQUEST_SIZE), and
 	// MaxResponseSize the largest response body (SWRP_MAX_RESPONSE_SIZE).
@@ -113,8 +114,9 @@ func (e *environment) wafBackend(name string) *url.URL {
 	return backend
 }
 
-// disposition reads the setting that is what is done with a request servewaf
+// disposition reads the setting that is what is done with a request when servewaf
-// gives no answer about: allow or deny.
+// cannot be connected to, does not answer at all, or does not answer in time:
+// allow or deny.
 func (e *environment) disposition(name, defaultValue string) string {
 	value := e.value(name, defaultValue)
 	if value != "allow" && value != "deny" {

internal/requestlog/requestlog.go

@@ -216,8 +216,9 @@ type RProxyLine struct {
 	// if that is sooner. DurationResponse runs from then until the answer ends.
 	DurationRequest  float64 `json:"duration_request"`
 	DurationResponse float64 `json:"duration_response"`
-	// Disposition is servewaf's answer or, while DefaultDisposition is true, the
+	// Disposition is the valid disposition servewaf answered or, while
-	// value of SWRP_DEFAULT_DISPOSITION. It is "" when servewaf was not asked.
+	// DefaultDisposition is true, the value of SWRP_DEFAULT_DISPOSITION. It is ""
+	// when neither gave one.
 	Disposition        string `json:"disposition,omitempty"`
 	DefaultDisposition bool   `json:"default_disposition,omitempty"`
 }

internal/rproxy/question_test.go

@@ -255,7 +255,7 @@ func TestAQuestionCarriesItsBodyWhateverTheMethod(t *testing.T) {
 		{"with its length", announcedLength(4) + "body", "body", nil},
 	} {
 		for _, method := range []string{
-			http.MethodConnect, http.MethodGet, http.MethodDelete, http.MethodPost,
+			http.MethodGet, http.MethodDelete, http.MethodPost,
 		} {
 			t.Run(method+" "+tc.name, func(t *testing.T) {
 				t.Parallel()
@@ -281,6 +281,73 @@ func TestAQuestionCarriesItsBodyWhateverTheMethod(t *testing.T) {
 	}
 }
 
+func TestAMethodNotPassedOnIsAnswered405(t *testing.T) {
+	t.Parallel()
+
+	for _, method := range []string{http.MethodTrace, "PROPFIND", "LOCK", "BREW"} {
+		t.Run(method, func(t *testing.T) {
+			t.Parallel()
+
+			servewaf := startServeWAF(t, allow)
+			app := startApp(t, func(http.ResponseWriter, *http.Request) {
+				t.Error("the app was asked")
+			})
+			addr, out := startRProxy(t, servewaf.URL, app.URL, nil)
+
+			got := visit(t, newRequest(t, method, addr, "/", http.NoBody))
+			if got.status != http.StatusMethodNotAllowed ||
+				got.header.Get("Allow") != "GET, HEAD, POST, OPTIONS, PUT, DELETE, PATCH, QUERY" {
+				t.Errorf("answered %+v, want 405 naming the methods passed on", got)
+			}
+
+			line := out.requestLine(t)
+			if line["status"] != float64(http.StatusMethodNotAllowed) ||
+				line["disposition"] != nil || line["default_disposition"] != nil {
+				t.Errorf("line %v, want status 405 and no disposition", line)
+			}
+
+			if len(servewaf.questions) != 0 {
+				t.Error("servewaf was asked")
+			}
+
+			servewaf.wantNoReport(t)
+		})
+	}
+}
+
+func TestEachMethodPassedOnReachesServeWAFAndTheApp(t *testing.T) {
+	t.Parallel()
+
+	for _, method := range []string{
+		http.MethodGet, http.MethodHead, http.MethodPost, http.MethodOptions,
+		http.MethodPut, http.MethodDelete, http.MethodPatch, "QUERY",
+	} {
+		t.Run(method, func(t *testing.T) {
+			t.Parallel()
+
+			reached := make(chan string, 1)
+			servewaf := startServeWAF(t, allow)
+			app := startApp(t, func(_ http.ResponseWriter, r *http.Request) {
+				reached <- r.Method
+			})
+			addr, _ := startRProxy(t, servewaf.URL, app.URL, nil)
+
+			got := visit(t, newRequest(t, method, addr, "/", http.NoBody))
+			if got.status != http.StatusOK {
+				t.Errorf("answered %+v, want the app's 200", got)
+			}
+
+			if asked := servewaf.question(t); asked.method != method {
+				t.Errorf("servewaf was asked about %s", asked.method)
+			}
+
+			if atApp := <-reached; atApp != method {
+				t.Errorf("the app was asked %s", atApp)
+			}
+		})
+	}
+}
+
 func TestARequestThatCannotGoToServeWAFAsSentIsRefused(t *testing.T) {
 	t.Parallel()
 
@@ -290,7 +357,7 @@ func TestARequestThatCannotGoToServeWAFAsSentIsRefused(t *testing.T) {
 		"a target of a scheme":    "GET x:admin HTTP/1.1\r\n" + host,
 		"a target of a URL":       "GET http://site.example/ HTTP/1.1\r\n" + host,
 		"a host Go will not send": "GET http://a<b/ HTTP/1.1\r\n" + host,
-		"a target of a host":      "CONNECT site.example:443 HTTP/1.1\r\n" + host,
+		"a target of a host":      "GET site.example:443 HTTP/1.1\r\n" + host,
 		"a target of *":           "GET * HTTP/1.1\r\n" + host,
 		"a target of //":          "GET //x/y HTTP/1.1\r\n" + host,
 		"a target of // escaped":  "GET //x%2Fadmin| HTTP/1.1\r\n" + host,

internal/rproxy/request.go

@@ -49,8 +49,8 @@ type request struct {
 	peerTrusted bool
 	start       time.Time
 
-	// disposition is what is done with the request, "" until servewaf has been
+	// disposition is what is done with the request: servewaf's valid disposition,
-	// asked.
+	// or SWRP_DEFAULT_DISPOSITION. It is "" when neither gave one.
 	disposition string
 	// answered is true when servewaf gave disposition, which makes a report due,
 	// and byDefault when SWRP_DEFAULT_DISPOSITION did.

internal/rproxy/rproxy.go

@@ -1,9 +1,12 @@
 // Package rproxy is the reverse proxy in front of one app that asks servewaf
 // about each request. It passes a request servewaf allows to the app and the
-// app's answer back, within the size and time limits. When servewaf gives no
+// app's answer back, within the size and time limits. Any other answer from
-// answer, SWRP_DEFAULT_DISPOSITION decides. A request it cannot put to servewaf
+// servewaf goes to the visitor as servewaf gave it. SWRP_DEFAULT_DISPOSITION
-// as the visitor sent it, it refuses itself. It keeps nothing between requests
+// decides only when servewaf cannot be connected to, does not answer at all, or
-// and never touches disk. "rproxy" in SPEC.md says what it does.
+// does not answer within SWRP_TIMEOUT_MS. It refuses itself a request with a
+// method it does not pass on, or one it cannot put to servewaf as the visitor
+// sent it. It keeps nothing between requests and never touches disk. "rproxy" in
+// SPEC.md says what it does.
 package rproxy
 
 import (
@@ -13,6 +16,7 @@ import (
 	"log/slog"
 	"net/http"
 	"net/url"
+	"slices"
 	"strings"
 	"sync"
 	"time"
@@ -45,6 +49,10 @@ const (
 // never reaches the app.
 const adminPrefix = "/_smallwebwaf/"
 
+// allowedMethods lists the methods rproxy passes on, as an Allow header gives
+// them. It answers any other method 405 itself.
+const allowedMethods = "GET, HEAD, POST, OPTIONS, PUT, DELETE, PATCH, QUERY"
+
 // Params are what New needs.
 type Params struct {
 	Config *config.RProxy
@@ -150,10 +158,10 @@ func newServeWAFTransport() *http.Transport {
 }
 
 // ServeHTTP handles one request: it answers the health check itself, refuses a
-// request it cannot put to servewaf as the visitor sent it, and for any other
+// request with a method it does not pass on or one it cannot put to servewaf as
-// request reads the first part of the body, asks servewaf, and does what the
+// the visitor sent it, and for any other request reads the first part of the
-// answer says. Once the answer has ended, it writes the request's log line and
+// body, asks servewaf, and does what the answer says. Once the answer has ended,
-// sends its report.
+// it writes the request's log line and sends its report.
 func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	rq := h.newRequest(w, r)
 	defer rq.finish(r.Context())
@@ -164,6 +172,13 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	if !slices.Contains(strings.Split(allowedMethods, ", "), r.Method) {
+		rq.out.Header().Set("Allow", allowedMethods)
+		rq.answer(http.StatusMethodNotAllowed)
+
+		return
+	}
+
 	if !canBeAsked(r) {
 		rq.answer(http.StatusBadRequest)
 
@@ -174,23 +189,24 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	answer := rq.askServeWAF(r.Context())
+	answer, err := rq.ask(r.Context())
+	if err != nil {
+		rq.handleNoUsableAnswer(r.Context(), err)
+
+		return
+	}
+
+	rq.noteAnswer(answer)
 
 	switch rq.disposition {
 	case proxy.DispositionAllow:
 		rq.passToApp(r.Context())
-	case proxy.DispositionDeny:
+	case proxy.DispositionClose:
-		if answer == nil {
-			rq.answer(http.StatusServiceUnavailable)
-
-			return
-		}
-
-		rq.answerWith(answer)
-	default: // proxy.DispositionClose, or none when the visitor has gone
 		// This panic has Go's server close the connection without an answer,
 		// and log nothing; the deferred finish still runs.
 		panic(http.ErrAbortHandler)
+	default: // a deny, an error, or an answer without a valid disposition
+		rq.answerWith(answer)
 	}
 }
 

internal/rproxy/rproxy_test.go

@@ -63,6 +63,9 @@ const (
 	contentLengthHeader  = "Content-Length"
 	// fromServeWAFHeader is a header the stand-in for servewaf adds to a deny.
 	fromServeWAFHeader = "X-From-Servewaf"
+	// addedHeaderValue is the value of a header a test adds to an answer, to see it
+	// reach the visitor.
+	addedHeaderValue = "yes"
 )
 
 // What the stand-in for servewaf answers.
@@ -292,7 +295,7 @@ func allow(w http.ResponseWriter, _ *http.Request) {
 // deny answers a question with deny, status 403 and a body of its own.
 func deny(w http.ResponseWriter, _ *http.Request) {
 	w.Header().Set(dispositionHeader, dispositionDeny)
-	w.Header().Set(fromServeWAFHeader, "yes")
+	w.Header().Set(fromServeWAFHeader, addedHeaderValue)
 	w.WriteHeader(http.StatusForbidden)
 	_, _ = io.WriteString(w, deniedBody)
 }
@@ -505,7 +508,7 @@ func TestPassesAnAllowedRequestToTheAppAndItsAnswerBack(t *testing.T) {
 	app := startApp(t, func(w http.ResponseWriter, r *http.Request) {
 		seen <- r.Clone(t.Context())
 
-		w.Header().Set("X-App", "yes")
+		w.Header().Set("X-App", addedHeaderValue)
 		w.WriteHeader(http.StatusTeapot)
 		_, _ = io.WriteString(w, greeting)
 	})
@@ -522,7 +525,7 @@ func TestPassesAnAllowedRequestToTheAppAndItsAnswerBack(t *testing.T) {
 
 	got := visit(t, req)
 	if got.status != http.StatusTeapot || got.body != greeting ||
-		got.header.Get("X-App") != "yes" {
+		got.header.Get("X-App") != addedHeaderValue {
 		t.Errorf("answered %+v, want the app's answer", got)
 	}
 
@@ -709,7 +712,7 @@ func TestDenyGivesTheVisitorServeWAFsAnswer(t *testing.T) {
 
 	got := get(t, addr, "/")
 	if got.status != http.StatusForbidden || got.body != deniedBody ||
-		got.header.Get(fromServeWAFHeader) != "yes" {
+		got.header.Get(fromServeWAFHeader) != addedHeaderValue {
 		t.Errorf("answered %+v, want servewaf's answer", got)
 	}
 
@@ -843,22 +846,27 @@ func TestCloseClosesTheConnectionWithoutAnAnswer(t *testing.T) {
 	}
 }
 
-// noAnswers are the ways servewaf can fail to give an answer that holds.
+// answeringWith returns what starts a stand-in for servewaf that answers each
-func noAnswers() map[string]func(t *testing.T) string {
+// question as answer does, and returns its URL.
-	answeringWith := func(answer http.HandlerFunc) func(t *testing.T) string {
+func answeringWith(answer http.HandlerFunc) func(t *testing.T) string {
-		return func(t *testing.T) string {
+	return func(t *testing.T) string {
-			t.Helper()
+		t.Helper()
 
-			return startServeWAF(t, answer).URL
+		return startServeWAF(t, answer).URL
-		}
 	}
+}
 
+// noAnswers are the ways servewaf can leave the decision to the default.
+func noAnswers() map[string]func(t *testing.T) string {
 	return map[string]func(t *testing.T) string{
-		"down": func(*testing.T) string { return nowhere },
+		"no connection": func(*testing.T) string { return nowhere },
-		"too slow": answeringWith(func(_ http.ResponseWriter, r *http.Request) {
+		"no answer at all": answeringWith(func(http.ResponseWriter, *http.Request) {
+			panic(http.ErrAbortHandler)
+		}),
+		"a slow start": answeringWith(func(_ http.ResponseWriter, r *http.Request) {
 			<-r.Context().Done()
 		}),
-		"too slow after its headers": answeringWith(
+		"headers followed by a stalled body": answeringWith(
 			func(w http.ResponseWriter, r *http.Request) {
 				w.Header().Set(dispositionHeader, dispositionDeny)
 				w.WriteHeader(http.StatusForbidden)
@@ -866,17 +874,6 @@ func noAnswers() map[string]func(t *testing.T) string {
 
 				<-r.Context().Done()
 			}),
-		"no disposition": answeringWith(func(w http.ResponseWriter, _ *http.Request) {
-			w.WriteHeader(http.StatusOK)
-		}),
-		"a question refused": answeringWith(func(w http.ResponseWriter, _ *http.Request) {
-			http.Error(w, "refused", http.StatusBadRequest)
-		}),
-		"an unknown disposition": answeringWith(
-			func(w http.ResponseWriter, _ *http.Request) {
-				w.Header().Set(dispositionHeader, "maybe")
-				w.WriteHeader(http.StatusOK)
-			}),
 		"an answer cut short": answeringWith(
 			func(w http.ResponseWriter, _ *http.Request) {
 				w.Header().Set(dispositionHeader, dispositionDeny)
@@ -888,14 +885,220 @@ func noAnswers() map[string]func(t *testing.T) string {
 
 				panic(http.ErrAbortHandler)
 			}),
-		"an answer too long": answeringWith(func(w http.ResponseWriter, _ *http.Request) {
-			w.Header().Set(dispositionHeader, dispositionDeny)
-			w.WriteHeader(http.StatusForbidden)
-			_, _ = w.Write(make([]byte, 1<<20+1))
-		}),
 	}
 }
 
+func TestAnAnswerOtherThanAllowGoesToTheVisitorAsServeWAFGaveIt(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range []struct {
+		name, disposition, body string
+		status                  int
+	}{
+		{"a refusal", dispositionDeny, deniedBody, http.StatusForbidden},
+		{"a refusal, 429", dispositionDeny, deniedBody, http.StatusTooManyRequests},
+		{"servewaf's 400", "", "refused\n", http.StatusBadRequest},
+		{"a 500", "", "broken\n", http.StatusInternalServerError},
+		{"a 503 denied", dispositionDeny, "busy\n", http.StatusServiceUnavailable},
+		{"no header", "", "who knows\n", http.StatusOK},
+		{"an unknown disposition", "maybe", "who knows\n", http.StatusOK},
+	} {
+		for _, setting := range []string{dispositionAllow, dispositionDeny} {
+			t.Run(tc.name+" with the default "+setting, func(t *testing.T) {
+				t.Parallel()
+
+				servewaf := startServeWAF(t, func(w http.ResponseWriter, _ *http.Request) {
+					if tc.disposition != "" {
+						w.Header().Set(dispositionHeader, tc.disposition)
+					}
+
+					w.Header().Set(fromServeWAFHeader, addedHeaderValue)
+					w.WriteHeader(tc.status)
+					_, _ = io.WriteString(w, tc.body)
+				})
+				app := startApp(t, func(http.ResponseWriter, *http.Request) {
+					t.Error("the app was asked")
+				})
+				addr, out := startRProxy(t, servewaf.URL, app.URL, map[string]string{
+					defaultDisposition: setting,
+				})
+
+				got := get(t, addr, "/")
+				if got.status != tc.status || got.body != tc.body ||
+					got.header.Get(fromServeWAFHeader) != addedHeaderValue {
+					t.Errorf("answered %+v, want servewaf's answer", got)
+				}
+
+				if len(got.header.Values(dispositionHeader)) != 0 {
+					t.Errorf("the visitor got %s", dispositionHeader)
+				}
+
+				line := out.requestLine(t)
+				if line["status"] != float64(tc.status) || line["default_disposition"] != nil {
+					t.Errorf("line %v, want status %d and no default", line, tc.status)
+				}
+
+				if out.has("servewaf gave no answer") {
+					t.Error("rproxy warned that servewaf gave no answer")
+				}
+			})
+		}
+	}
+}
+
+func TestAnAnswerWithoutAValidDispositionGetsNoReport(t *testing.T) {
+	t.Parallel()
+
+	servewaf := startServeWAF(t, func(w http.ResponseWriter, _ *http.Request) {
+		http.Error(w, "refused", http.StatusBadRequest)
+	})
+	app := startApp(t, greet)
+	addr, out := startRProxy(t, servewaf.URL, app.URL, nil)
+
+	if got := get(t, addr, "/"); got.status != http.StatusBadRequest {
+		t.Errorf("answered %+v, want servewaf's 400", got)
+	}
+
+	if line := out.requestLine(t); line["disposition"] != nil {
+		t.Errorf("line %v, want no disposition", line)
+	}
+
+	servewaf.wantNoReport(t)
+}
+
+func TestAnAnswerWithoutAContentTypeGoesToTheVisitorWithNone(t *testing.T) {
+	t.Parallel()
+
+	servewaf := startServeWAF(t, func(w http.ResponseWriter, _ *http.Request) {
+		w.Header()["Content-Type"] = nil
+		w.WriteHeader(http.StatusInternalServerError)
+		_, _ = io.WriteString(w, "<html>broken")
+	})
+	addr, _ := startRProxy(t, servewaf.URL, nowhere, nil)
+
+	got := get(t, addr, "/")
+	if contentType, sent := got.header["Content-Type"]; sent {
+		t.Errorf("the visitor got the Content-Type %v", contentType)
+	}
+}
+
+func TestAnAnswerOf1MiBGoesToTheVisitor(t *testing.T) {
+	t.Parallel()
+
+	const size = 1 << 20
+
+	servewaf := startServeWAF(t, func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusInternalServerError)
+		_, _ = w.Write(make([]byte, size))
+	})
+	addr, _ := startRProxy(t, servewaf.URL, nowhere, nil)
+
+	got := get(t, addr, "/")
+	if got.status != http.StatusInternalServerError || len(got.body) != size {
+		t.Errorf("answered %d with %d bytes, want servewaf's 500 with %d",
+			got.status, len(got.body), size)
+	}
+}
+
+// startServeWAFWriting starts a stand-in for servewaf that reads each question and
+// answers it with text, as it is, and returns its URL. It can send what Go's
+// server would not.
+func startServeWAFWriting(t *testing.T, text string) string {
+	t.Helper()
+
+	listener, err := (&net.ListenConfig{}).Listen(t.Context(), "tcp", localhost+":0")
+	if err != nil {
+		t.Fatalf("listen: %v", err)
+	}
+
+	t.Cleanup(func() {
+		_ = listener.Close()
+	})
+
+	go func() {
+		for {
+			conn, err := listener.Accept()
+			if err != nil {
+				return
+			}
+
+			_ = conn.SetDeadline(time.Now().Add(waitLimit))
+			_, _ = http.ReadRequest(bufio.NewReader(conn))
+			_, _ = io.WriteString(conn, text)
+			_ = conn.Close()
+		}
+	}()
+
+	return "http://" + listener.Addr().String()
+}
+
+// overOneMiB answers a question with a body over the most rproxy reads, and with
+// disposition unless it is "".
+func overOneMiB(disposition string) http.HandlerFunc {
+	return func(w http.ResponseWriter, _ *http.Request) {
+		if disposition != "" {
+			w.Header().Set(dispositionHeader, disposition)
+		}
+
+		w.WriteHeader(http.StatusForbidden)
+		_, _ = w.Write(make([]byte, 1<<20+1))
+	}
+}
+
+func TestAnAnswerThatCannotBePassedOnIsAnswered502(t *testing.T) {
+	t.Parallel()
+
+	for name, backend := range map[string]func(t *testing.T) string{
+		"a deny over 1 MiB":    answeringWith(overOneMiB(dispositionDeny)),
+		"an allow over 1 MiB":  answeringWith(overOneMiB(dispositionAllow)),
+		"no header over 1 MiB": answeringWith(overOneMiB("")),
+		"a status below 100": func(t *testing.T) string {
+			t.Helper()
+
+			return startServeWAFWriting(t, "HTTP/1.1 099 Odd\r\nContent-Length: 0\r\n\r\n")
+		},
+	} {
+		for _, setting := range []string{dispositionAllow, dispositionDeny} {
+			t.Run(name+" with the default "+setting, func(t *testing.T) {
+				t.Parallel()
+
+				app := startApp(t, func(http.ResponseWriter, *http.Request) {
+					t.Error("the app was asked")
+				})
+				addr, out := startRProxy(t, backend(t), app.URL, map[string]string{
+					defaultDisposition: setting,
+				})
+
+				if got := get(t, addr, "/"); got.status != http.StatusBadGateway {
+					t.Errorf("answered %+v, want 502", got)
+				}
+
+				line := out.requestLine(t)
+				if line["status"] != float64(http.StatusBadGateway) ||
+					line["disposition"] != nil || line["default_disposition"] != nil {
+					t.Errorf("line %v, want status 502 and no disposition", line)
+				}
+
+				out.waitFor(t, "servewaf's answer is dropped")
+			})
+		}
+	}
+}
+
+func TestAnAnswerThatCannotBePassedOnGetsNoReport(t *testing.T) {
+	t.Parallel()
+
+	servewaf := startServeWAF(t, overOneMiB(dispositionDeny))
+	addr, out := startRProxy(t, servewaf.URL, nowhere, nil)
+
+	if got := get(t, addr, "/"); got.status != http.StatusBadGateway {
+		t.Errorf("answered %+v, want 502", got)
+	}
+
+	out.requestLine(t)
+	servewaf.wantNoReport(t)
+}
+
 func TestWithoutAnAnswerTheDefaultDispositionDecides(t *testing.T) {
 	t.Parallel()
 
@@ -984,11 +1187,13 @@ func TestAVisitorThatGoesAwayWhileServeWAFIsAskedGetsNoDefault(t *testing.T) {
 func TestSendsNoReportWithoutAnAnswer(t *testing.T) {
 	t.Parallel()
 
-	servewaf := startServeWAF(t, func(w http.ResponseWriter, _ *http.Request) {
+	servewaf := startServeWAF(t, func(_ http.ResponseWriter, r *http.Request) {
-		http.Error(w, "refused", http.StatusBadRequest)
+		<-r.Context().Done()
 	})
 	app := startApp(t, greet)
-	addr, out := startRProxy(t, servewaf.URL, app.URL, nil)
+	addr, out := startRProxy(t, servewaf.URL, app.URL, map[string]string{
+		timeoutSetting: "200",
+	})
 
 	wantGreeting(t, get(t, addr, "/"))
 	out.requestLine(t)

internal/rproxy/servewaf.go

@@ -23,8 +23,7 @@ const answerBodyMaxBytes = 1 << 20
 const reportAnswerMaxBytes = 4 << 10
 
 var (
-	errNoDisposition = errors.New("the answer has no valid " + proxy.DispositionHeader)
+	errCannotPassOn  = errors.New("the answer cannot be passed to the visitor")
-	errAnswerTooLong = errors.New("the answer's body is over 1 MiB")
 	errReportRefused = errors.New("the report was answered")
 )
 
@@ -35,40 +34,57 @@ type servewafAnswer struct {
 	body   []byte
 }
 
-// askServeWAF asks servewaf about the request, and notes what is to be done
+// noteAnswer notes what servewaf answered. An answer without a valid disposition
-// with it. It returns servewaf's answer, or nil when servewaf gave none, and
+// notes nothing, and no report is due for it: servewaf holds no decision to report
-// SWRP_DEFAULT_DISPOSITION decides. When the visitor went away while servewaf
+// on.
-// was being asked, servewaf has not failed, and nothing is noted. ctx is the
+func (rq *request) noteAnswer(answer *servewafAnswer) {
-// request's own context.
+	disposition := answer.header.Get(proxy.DispositionHeader)
-func (rq *request) askServeWAF(ctx context.Context) *servewafAnswer {
-	answer, err := rq.ask(ctx)
-	if err != nil {
-		if ctx.Err() != nil {
-			return nil
-		}
 
-		// A request that keeps servewaf busy past SWRP_TIMEOUT_MS, like any request
+	switch disposition {
-		// while servewaf is overloaded, gets SWRP_DEFAULT_DISPOSITION here. deny closes
+	case proxy.DispositionAllow, proxy.DispositionDeny, proxy.DispositionClose:
-		// this, at the cost of answering 503 whenever servewaf is slow.
+		rq.disposition = disposition
+		rq.answered = true
+		rq.asn = answer.header.Get(asnHeader)
+		rq.country = answer.header.Get(countryHeader)
+	}
+}
+
+// handleNoUsableAnswer deals with a request whose question ended in err, which
+// leaves no answer to act on. ctx is the request's own context.
+func (rq *request) handleNoUsableAnswer(ctx context.Context, err error) {
+	switch {
+	case ctx.Err() != nil:
+		// The visitor went away while servewaf was being asked. servewaf has not
+		// failed, and nothing is noted. This panic has Go's server close the
+		// connection without an answer, and log nothing; the deferred finish still
+		// runs.
+		panic(http.ErrAbortHandler)
+	case errors.Is(err, errCannotPassOn):
+		rq.h.processLog.Warn("servewaf's answer is dropped", "error", err.Error())
+		rq.answer(http.StatusBadGateway)
+	default:
+		// servewaf cannot be connected to, does not answer at all, or does not answer
+		// within SWRP_TIMEOUT_MS. A request that keeps servewaf busy past
+		// SWRP_TIMEOUT_MS, like any request while servewaf is overloaded, gets
+		// SWRP_DEFAULT_DISPOSITION here. deny closes this, at the cost of answering 503
+		// whenever servewaf is slow. Making such requests hard to send is
+		// https://git.eeqj.de/sneak/smallwebwaf/issues/182.
 		rq.h.processLog.Warn("servewaf gave no answer", "error", err.Error())
 
 		rq.disposition = rq.h.config.DefaultDisposition
 		rq.byDefault = true
 
-		return nil
+		if rq.disposition == proxy.DispositionAllow {
+			rq.passToApp(ctx)
+		} else {
+			rq.answer(http.StatusServiceUnavailable)
+		}
 	}
-
-	rq.disposition = answer.header.Get(proxy.DispositionHeader)
-	rq.answered = true
-	rq.asn = answer.header.Get(asnHeader)
-	rq.country = answer.header.Get(countryHeader)
-
-	return answer
 }
 
 // ask sends servewaf the question, and returns its answer, which must have come
-// whole within SWRP_TIMEOUT_MS, with a valid disposition. It ends when ctx does,
+// whole within SWRP_TIMEOUT_MS. It ends when ctx does, as when the visitor goes
-// as when the visitor goes away.
+// away.
 func (rq *request) ask(ctx context.Context) (*servewafAnswer, error) {
 	ctx, cancel := context.WithTimeout(ctx, rq.h.config.Timeout)
 	defer cancel()
@@ -92,16 +108,15 @@ func (rq *request) ask(ctx context.Context) (*servewafAnswer, error) {
 		return nil, fmt.Errorf("read the answer: %w", err)
 	}
 
-	if len(body) > answerBodyMaxBytes {
+	switch {
-		return nil, errAnswerTooLong
+	case len(body) > answerBodyMaxBytes:
+		return nil, fmt.Errorf("%w: its body is over 1 MiB", errCannotPassOn)
+	case res.StatusCode < http.StatusContinue:
+		// Go's client takes such a status. Go's server panics when asked to send it.
+		return nil, fmt.Errorf("%w: its status is %d", errCannotPassOn, res.StatusCode)
 	}
 
-	switch res.Header.Get(proxy.DispositionHeader) {
+	return &servewafAnswer{res.StatusCode, res.Header, body}, nil
-	case proxy.DispositionAllow, proxy.DispositionDeny, proxy.DispositionClose:
-		return &servewafAnswer{res.StatusCode, res.Header, body}, nil
-	default:
-		return nil, fmt.Errorf("%w, and status %d", errNoDisposition, res.StatusCode)
-	}
 }
 
 // canBeAsked reports whether Go's client can send servewaf r as the visitor sent
@@ -177,6 +192,13 @@ func (rq *request) answerWith(answer *servewafAnswer) {
 	header := rq.out.Header()
 	maps.Copy(header, answer.header)
 	header.Del(proxy.DispositionHeader)
+
+	// Go's server adds a Content-Type it guesses from the body to a response whose
+	// headers have no Content-Type key. The key with no value sends none.
+	if header.Get("Content-Type") == "" {
+		header["Content-Type"] = nil
+	}
+
 	rq.send(answer.status, answer.body)
 }
 

internal/smallwebwaf/rproxy_test.go

@@ -304,7 +304,7 @@ func sendRaw(t *testing.T, site, request string) int {
 	return res.StatusCode
 }
 
-func TestAConnectBodyStaysABodyAtServeWAF(t *testing.T) {
+func TestAChunkedBodyStaysABodyAtServeWAF(t *testing.T) {
 	t.Parallel()
 
 	addr, serveWAFOut, stop := startServeWAF(t, serveWAFSettings(t))
@@ -319,12 +319,11 @@ func TestAConnectBodyStaysABodyAtServeWAF(t *testing.T) {
 		proxy.ClientHeader + ": " + otherClient + "\r\n" +
 		"X-Request-ID: " + smuggledID + "\r\n\r\n"
 
-	status := sendRaw(t, site, "CONNECT /tunnel HTTP/1.1\r\nHost: site.example\r\n"+
+	status := sendRaw(t, site, "POST /upload HTTP/1.1\r\nHost: site.example\r\n"+
 		"X-Forwarded-For: "+placed+"\r\nTransfer-Encoding: chunked\r\n\r\n"+
 		strconv.FormatInt(int64(len(smuggled)), 16)+"\r\n"+smuggled+"\r\n0\r\n\r\n")
-	if status != http.StatusForbidden {
+	if status != http.StatusOK {
-		t.Errorf("status %d, want the 403 of the Core Rule Set, which refuses CONNECT",
+		t.Errorf("status %d, want the app's 200", status)
-			status)
 	}
 
 	// Read as a further request, it would act straight after the answer.