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 size and
time limits. It reports each request servewaf answered. SWRP_DEFAULT_DISPOSITION
decides when servewaf gives no answer. A request it cannot put to servewaf as
sent, it refuses itself.

Duplication: internal/rproxy copies passing-on code from internal/proxy until
#172 removes the originals.
Rule suppressed: gosec G704 and G705, on servewaf's URL and the visitor's answer.
Rule suppressed: paralleltest, on the test of a switch rproxy refuses.
Judgement call: the SWRP_ size and time limits take off; SWRP_TIMEOUT_MS does not.
Judgement call: every target not starting with one / gets 400.

Model: opus-5-5

README.md

@@ -353,10 +353,8 @@ 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. When `servewaf` cannot be 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. The settings of `rproxy` start with `SWRP_`, and it needs
+  `smallwebwaf servewaf` about each request, and keeps the site answering while
+  `servewaf` is down. Its settings 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. Any other body
-comes with a `Content-Length` of the part it carries.
+A body whose length the visitor did not announce comes chunked, whatever the
+method. Any other body comes with a `Content-Length` of the part it carries.
 
 `rproxy` sets these headers, replacing any the visitor sent, and adds no others:
 
@@ -1135,8 +1135,7 @@ 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`
-  gives the visitor its status, headers and body, as it does for any answer
-  without a valid `X-Smallwebwaf-Disposition`.
+  treats this answer as it treats no answer at all.
 
 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
@@ -1151,14 +1150,9 @@ 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:
-
-- 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:
+none after a `400` or 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`;
@@ -1248,10 +1242,8 @@ 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` cannot be
-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.
+nothing between requests and never touches disk. When `servewaf` is down, slow
+or overloaded, the site keeps 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
@@ -1270,9 +1262,8 @@ 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 with a method it does not pass on, or one it cannot put
-   to `servewaf` as the visitor sent it (see "Requests `rproxy` refuses
-   itself").
+4. It refuses a request it cannot put 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
@@ -1280,11 +1271,9 @@ 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.
-    - `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
+    - `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.
 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
@@ -1297,27 +1286,11 @@ and health check.
 
 ### Requests `rproxy` refuses itself
 
-`SWRP_DEFAULT_DISPOSITION` applies only when `servewaf` cannot be connected to,
-does not answer at all, or does not answer within `SWRP_TIMEOUT_MS`. A visitor
-can make `servewaf` slow. A request that keeps `servewaf` busy past
-`SWRP_TIMEOUT_MS`, like any request while `servewaf` is overloaded, gets
-`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.
+`SWRP_DEFAULT_DISPOSITION` applies only when `servewaf` itself is down, slow or
+answers badly. A visitor can make `servewaf` slow. A request that keeps
+`servewaf` busy past `SWRP_TIMEOUT_MS`, like any request while `servewaf` is
+overloaded, gets `SWRP_DEFAULT_DISPOSITION`. `deny` closes this, at the cost of
+answering `503` whenever `servewaf` is slow.
 
 `rproxy` answers `400`, with no question and no report, to a request with:
 
@@ -1340,15 +1313,15 @@ log line has status `0`.
 
 ### Without an answer from `servewaf`
 
-`SWRP_DEFAULT_DISPOSITION` decides what is done with the request only when
+`SWRP_DEFAULT_DISPOSITION` decides what is done with the request 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.
-
-An answer whose body breaks off counts as no answer at all.
+  question and reading the whole answer;
+- cannot be reached;
+- answers without a valid `X-Smallwebwaf-Disposition`, as with the `400` for a
+  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
@@ -1357,11 +1330,6 @@ 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,11 +204,8 @@ 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;
-    - 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`.
+  out when `servewaf` was not asked, as for the health check, and when the
+  visitor went away while it was being asked.
 - `default_disposition`: `true` when `SWRP_DEFAULT_DISPOSITION` gave
   `disposition`, and otherwise left out.
 

docs/settings.md

@@ -341,10 +341,9 @@ 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, and closes a
-  kept-open connection idle for 120 seconds;
-- `rproxy` reads at most 1 MiB of the body of `servewaf`'s answer, and answers
-  `502` to a longer one.
+- `rproxy` takes a request line and headers of at most 32 KiB, closes a
+  kept-open connection idle for 120 seconds, and reads at most 1 MiB of the body
+  of `servewaf`'s answer.
 
 ## `servewaf`
 
@@ -377,10 +376,9 @@ 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 when
-  `servewaf` cannot be connected to, does not answer at all, or does not answer
-  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_DEFAULT_DISPOSITION` (default `allow`): what is done with a request
+  `servewaf` gives no answer about. `allow` passes it to the app, and `deny`
+  answers `503`.
 - `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,9 +27,8 @@ 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 when servewaf cannot be
-	// connected to, does not answer at all, or does not answer within Timeout:
-	// allow or deny (SWRP_DEFAULT_DISPOSITION).
+	// DefaultDisposition is what is done with a request servewaf gives no
+	// answer about, 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).
@@ -114,9 +113,8 @@ func (e *environment) wafBackend(name string) *url.URL {
 	return backend
 }
 
-// disposition reads the setting that is what is done with a request when servewaf
-// cannot be connected to, does not answer at all, or does not answer in time:
-// allow or deny.
+// disposition reads the setting that is what is done with a request servewaf
+// gives no answer about: 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,9 +216,8 @@ 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 the valid disposition servewaf answered or, while
-	// DefaultDisposition is true, the value of SWRP_DEFAULT_DISPOSITION. It is ""
-	// when neither gave one.
+	// Disposition is servewaf's answer or, while DefaultDisposition is true, the
+	// value of SWRP_DEFAULT_DISPOSITION. It is "" when servewaf was not asked.
 	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.MethodGet, http.MethodDelete, http.MethodPost,
+			http.MethodConnect, http.MethodGet, http.MethodDelete, http.MethodPost,
 		} {
 			t.Run(method+" "+tc.name, func(t *testing.T) {
 				t.Parallel()
@@ -281,73 +281,6 @@ 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()
 
@@ -357,7 +290,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":      "GET site.example:443 HTTP/1.1\r\n" + host,
+		"a target of a host":      "CONNECT 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: servewaf's valid disposition,
-	// or SWRP_DEFAULT_DISPOSITION. It is "" when neither gave one.
+	// disposition is what is done with the request, "" until servewaf has been
+	// asked.
 	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,12 +1,9 @@
 // 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. Any other answer from
-// servewaf 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. 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.
+// app's answer back, within the size and time limits. When servewaf gives no
+// answer, SWRP_DEFAULT_DISPOSITION decides. A request it cannot put to servewaf
+// as the visitor sent it, it refuses itself. It keeps nothing between requests
+// and never touches disk. "rproxy" in SPEC.md says what it does.
 package rproxy
 
 import (
@@ -16,7 +13,6 @@ import (
 	"log/slog"
 	"net/http"
 	"net/url"
-	"slices"
 	"strings"
 	"sync"
 	"time"
@@ -49,10 +45,6 @@ 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
@@ -158,10 +150,10 @@ func newServeWAFTransport() *http.Transport {
 }
 
 // ServeHTTP handles one request: it answers the health check itself, refuses a
-// request with a method it does not pass on or one it cannot put to servewaf as
-// the visitor sent it, and for any other request reads the first part of the
-// body, asks servewaf, and does what the answer says. Once the answer has ended,
-// it writes the request's log line and sends its report.
+// request it cannot put to servewaf as the visitor sent it, and for any other
+// request reads the first part of the body, asks servewaf, and does what the
+// answer says. Once the answer has ended, 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())
@@ -172,13 +164,6 @@ 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)
 
@@ -189,24 +174,23 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	answer, err := rq.ask(r.Context())
-	if err != nil {
-		rq.handleNoUsableAnswer(r.Context(), err)
-
-		return
-	}
-
-	rq.noteAnswer(answer)
+	answer := rq.askServeWAF(r.Context())
 
 	switch rq.disposition {
 	case proxy.DispositionAllow:
 		rq.passToApp(r.Context())
-	case proxy.DispositionClose:
+	case proxy.DispositionDeny:
+		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,9 +63,6 @@ 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.
@@ -295,7 +292,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, addedHeaderValue)
+	w.Header().Set(fromServeWAFHeader, "yes")
 	w.WriteHeader(http.StatusForbidden)
 	_, _ = io.WriteString(w, deniedBody)
 }
@@ -508,7 +505,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", addedHeaderValue)
+		w.Header().Set("X-App", "yes")
 		w.WriteHeader(http.StatusTeapot)
 		_, _ = io.WriteString(w, greeting)
 	})
@@ -525,7 +522,7 @@ func TestPassesAnAllowedRequestToTheAppAndItsAnswerBack(t *testing.T) {
 
 	got := visit(t, req)
 	if got.status != http.StatusTeapot || got.body != greeting ||
-		got.header.Get("X-App") != addedHeaderValue {
+		got.header.Get("X-App") != "yes" {
 		t.Errorf("answered %+v, want the app's answer", got)
 	}
 
@@ -712,7 +709,7 @@ func TestDenyGivesTheVisitorServeWAFsAnswer(t *testing.T) {
 
 	got := get(t, addr, "/")
 	if got.status != http.StatusForbidden || got.body != deniedBody ||
-		got.header.Get(fromServeWAFHeader) != addedHeaderValue {
+		got.header.Get(fromServeWAFHeader) != "yes" {
 		t.Errorf("answered %+v, want servewaf's answer", got)
 	}
 
@@ -846,27 +843,22 @@ func TestCloseClosesTheConnectionWithoutAnAnswer(t *testing.T) {
 	}
 }
 
-// answeringWith returns what starts a stand-in for servewaf that answers each
-// question as answer does, and returns its URL.
-func answeringWith(answer http.HandlerFunc) func(t *testing.T) string {
-	return func(t *testing.T) string {
-		t.Helper()
-
-		return startServeWAF(t, answer).URL
-	}
-}
-
-// noAnswers are the ways servewaf can leave the decision to the default.
+// noAnswers are the ways servewaf can fail to give an answer that holds.
 func noAnswers() map[string]func(t *testing.T) string {
+	answeringWith := func(answer http.HandlerFunc) func(t *testing.T) string {
+		return func(t *testing.T) string {
+			t.Helper()
+
+			return startServeWAF(t, answer).URL
+		}
+	}
+
 	return map[string]func(t *testing.T) string{
-		"no connection": func(*testing.T) string { return nowhere },
-		"no answer at all": answeringWith(func(http.ResponseWriter, *http.Request) {
-			panic(http.ErrAbortHandler)
-		}),
-		"a slow start": answeringWith(func(_ http.ResponseWriter, r *http.Request) {
+		"down": func(*testing.T) string { return nowhere },
+		"too slow": answeringWith(func(_ http.ResponseWriter, r *http.Request) {
 			<-r.Context().Done()
 		}),
-		"headers followed by a stalled body": answeringWith(
+		"too slow after its headers": answeringWith(
 			func(w http.ResponseWriter, r *http.Request) {
 				w.Header().Set(dispositionHeader, dispositionDeny)
 				w.WriteHeader(http.StatusForbidden)
@@ -874,6 +866,17 @@ 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)
@@ -885,220 +888,14 @@ 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()
 
@@ -1187,13 +984,11 @@ func TestAVisitorThatGoesAwayWhileServeWAFIsAskedGetsNoDefault(t *testing.T) {
 func TestSendsNoReportWithoutAnAnswer(t *testing.T) {
 	t.Parallel()
 
-	servewaf := startServeWAF(t, func(_ http.ResponseWriter, r *http.Request) {
-		<-r.Context().Done()
+	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, map[string]string{
-		timeoutSetting: "200",
-	})
+	addr, out := startRProxy(t, servewaf.URL, app.URL, nil)
 
 	wantGreeting(t, get(t, addr, "/"))
 	out.requestLine(t)

internal/rproxy/servewaf.go

@@ -23,7 +23,8 @@ const answerBodyMaxBytes = 1 << 20
 const reportAnswerMaxBytes = 4 << 10
 
 var (
-	errCannotPassOn  = errors.New("the answer cannot be passed to the visitor")
+	errNoDisposition = errors.New("the answer has no valid " + proxy.DispositionHeader)
+	errAnswerTooLong = errors.New("the answer's body is over 1 MiB")
 	errReportRefused = errors.New("the report was answered")
 )
 
@@ -34,57 +35,40 @@ type servewafAnswer struct {
 	body   []byte
 }
 
-// noteAnswer notes what servewaf answered. An answer without a valid disposition
-// notes nothing, and no report is due for it: servewaf holds no decision to report
-// on.
-func (rq *request) noteAnswer(answer *servewafAnswer) {
-	disposition := answer.header.Get(proxy.DispositionHeader)
+// askServeWAF asks servewaf about the request, and notes what is to be done
+// with it. It returns servewaf's answer, or nil when servewaf gave none, and
+// SWRP_DEFAULT_DISPOSITION decides. When the visitor went away while servewaf
+// was being asked, servewaf has not failed, and nothing is noted. ctx is the
+// request's own context.
+func (rq *request) askServeWAF(ctx context.Context) *servewafAnswer {
+	answer, err := rq.ask(ctx)
+	if err != nil {
+		if ctx.Err() != nil {
+			return nil
+		}
 
-	switch disposition {
-	case proxy.DispositionAllow, proxy.DispositionDeny, proxy.DispositionClose:
-		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.
+		// 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.
 		rq.h.processLog.Warn("servewaf gave no answer", "error", err.Error())
 
 		rq.disposition = rq.h.config.DefaultDisposition
 		rq.byDefault = true
 
-		if rq.disposition == proxy.DispositionAllow {
-			rq.passToApp(ctx)
-		} else {
-			rq.answer(http.StatusServiceUnavailable)
-		}
+		return nil
 	}
+
+	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. It ends when ctx does, as when the visitor goes
-// away.
+// whole within SWRP_TIMEOUT_MS, with a valid disposition. It ends when ctx does,
+// as when the visitor goes away.
 func (rq *request) ask(ctx context.Context) (*servewafAnswer, error) {
 	ctx, cancel := context.WithTimeout(ctx, rq.h.config.Timeout)
 	defer cancel()
@@ -108,15 +92,16 @@ func (rq *request) ask(ctx context.Context) (*servewafAnswer, error) {
 		return nil, fmt.Errorf("read the answer: %w", err)
 	}
 
-	switch {
-	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)
+	if len(body) > answerBodyMaxBytes {
+		return nil, errAnswerTooLong
 	}
 
-	return &servewafAnswer{res.StatusCode, res.Header, body}, nil
+	switch res.Header.Get(proxy.DispositionHeader) {
+	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
@@ -192,13 +177,6 @@ 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 TestAChunkedBodyStaysABodyAtServeWAF(t *testing.T) {
+func TestAConnectBodyStaysABodyAtServeWAF(t *testing.T) {
 	t.Parallel()
 
 	addr, serveWAFOut, stop := startServeWAF(t, serveWAFSettings(t))
@@ -319,11 +319,12 @@ func TestAChunkedBodyStaysABodyAtServeWAF(t *testing.T) {
 		proxy.ClientHeader + ": " + otherClient + "\r\n" +
 		"X-Request-ID: " + smuggledID + "\r\n\r\n"
 
-	status := sendRaw(t, site, "POST /upload HTTP/1.1\r\nHost: site.example\r\n"+
+	status := sendRaw(t, site, "CONNECT /tunnel 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.StatusOK {
-		t.Errorf("status %d, want the app's 200", status)
+	if status != http.StatusForbidden {
+		t.Errorf("status %d, want the 403 of the Core Rule Set, which refuses CONNECT",
+			status)
 	}
 
 	// Read as a further request, it would act straight after the answer.