README and SPEC say a client too slow to send its request line and headers may get an error answer (closes #138)

A client too slow to send its request line and headers within
SWWAF_CLIENT_REQUEST_TIMEOUT gets either no answer or an error answer
from Go's server, mostly 400 Bad Request; either way its connection is
closed, nothing reaches the app, and no request log line is written.
The README's "Request log" and SPEC's size and time limits said such a
client always gets no answer. Both documents' summaries of
smallwebwaf's own answers now cover only limits passed after the
request line and headers have been read, and give 408 only for a slow
body; SPEC names Go's server as the one answering 431.

Model: opus-5-5

README.md

@@ -1449,12 +1449,11 @@ whole service, its `asn`, for an AS number, its `name`, for a named netblock, or
 the `start` of a window in which it has requests or bytes; a list's `url`,
 `fetched` or `lines`, which is `[]` for an empty list; a verdict's `zone`,
 `client`, `listed`, which is `false` for a client the zone does not list, or
-`fetched`. So does a ban whose `expires` is not after its `start` or whose
+`fetched`. So does a ban whose `cause` is not `limit`, `attack`, `admin` or
-`cause` is not `limit`, `attack`, `admin` or `crowdsec`, alerts waiting for a
+`crowdsec`, alerts waiting for a destination that is not `webhook`, `slack` or
-destination that is not `webhook`, `slack` or `ntfy`, an anomaly counter whose
+`ntfy`, an anomaly counter whose `scope` is not `client`, `net`, `asn`, `total`
-`scope` is not `client`, `net`, `asn`, `total` or `watch`, and a copy of a list
+or `watch`, and a copy of a list with a line that would make its fetch fail. An
-with a line that would make its fetch fail. An answer's `asn` or `as_name` left
+answer's `asn` or `as_name` left out reads as empty.
-out reads as empty.
 
 While it runs, `smallwebwaf` watches `SWWAF_STATE_DIR` and takes in your edit of
 a state file as soon as you save it: what the file then holds replaces what
@@ -1464,15 +1463,14 @@ writes a file it takes in any edit made since, so your edit is not overwritten;
 a change `smallwebwaf` made after you opened the file, such as a new ban, is
 lost when you save over it. An edit that would stop the start, because it does
 not parse, has another `version`, leaves out a field an entry needs, gives a ban
-an `expires` not after its `start` or another `cause`, names another
+another `cause`, names another destination, gives an anomaly counter another
-destination, gives an anomaly counter another `scope` or gives a list's copy a
+`scope` or gives a list's copy a line that would make its fetch fail, does not
-line that would make its fetch fail, does not stop the running `smallwebwaf`: it
+stop the running `smallwebwaf`: it keeps what it holds, and at the file's next
-keeps what it holds, and at the file's next write renames your file to
+write renames your file to `<name>.bad`, such as `bans.json.bad`, writes the
-`<name>.bad`, such as `bans.json.bad`, writes the file again from memory, logs
+file again from memory, logs the file and where the error is, and raises a
-the file and where the error is, and raises a `file_error` alert for it. It
+`file_error` alert for it. It waits for that write because an editor's file can
-waits for that write because an editor's file can be read before the editor has
+be read before the editor has finished writing it. Mend the `.bad` file and move
-finished writing it. Mend the `.bad` file and move it back. A file you remove is
+it back. A file you remove is written again at its next write.
-written again at its next write.
 
 To ban a netblock, add an entry to `bans.json` with its `netblock`, its `start`
 and its `expires`, `null` for a ban that never ends; its `reason` and its
@@ -2160,14 +2158,13 @@ addresses sends, so that one client costs at most one check every
 `SWWAF_REPUTATION_CACHE_TTL`.
 
 Only a client whose history counts an offence is checked, so that the checks are
-spent on suspects: so far, one that has broken a rate limit, a byte limit or the
+spent on suspects: so far, one that has broken a rate limit or a byte limit,
-error burst, matched a ban rule, asked for a trap path, or had a request refused
+matched a ban rule, or had a request refused by a block rule or the Core Rule
-by a block rule, by the Core Rule Set or for a missing or wrong token. A client
+Set. A client dropped from the table of clients loses its history, and with it
-dropped from the table of clients loses its history, and with it its offences. A
+its offences. A client is checked in the background, at its first request after
-client is checked in the background, at its first request after its offence that
+its offence that reaches the check: no request waits, a request refused under
-reaches the check: no request waits, a request refused under its ban is not
+its ban is not checked, and the request that has it checked, and any other from
-checked, and the request that has it checked, and any other from it before the
+it before the answer comes, goes on as from a client without a score.
-answer comes, goes on as from a client without a score.
 
 A score at or over `SWWAF_ABUSEIPDB_MIN_SCORE`, 75 by default, is a hit, and
 `SWWAF_REPUTATION_ACTION` says what is done with its client, as for a DNSBL
@@ -2384,13 +2381,10 @@ so that they run in minimal containers.
   that the health check passes, that a request reaches the app through
   `smallwebwaf`, that a second request in a minute bans the client, that a probe
   for `/.env` bans another client, whose next request makes the ban permanent,
-  that `sv stop` and `docker stop` stop it in order, that a new container on the
+  that `sv stop` and `docker stop` stop it in order, and that a new container on
-  same volume still refuses the banned client, that a container with its state
+  the same volume still refuses the banned client; then removes the containers,
-  files on a new volume mounted at the directory `SWWAF_STATE_DIR_FILE` names
+  the volume and both images. It needs network access, for nixpkgs' binary
-  becomes healthy, and that one whose `SWWAF_STATE_DIR` does not exist logs
+  cache, and `script/check` does not run it; `make example-app` does.
-  `smallwebwaf`'s message about it; then removes the containers, the volumes and
-  both images. It needs network access, for nixpkgs' binary cache, and
-  `script/check` does not run it; `make example-app` does.
 - `script/gitea-requests`: builds the image and runs gitea 1.27.3 behind it,
   with every setting of `smallwebwaf` at its default but `SWWAF_UPSTREAM_URL`,
   which names gitea's container. Of the requests that three of the notes

SPEC.md

@@ -225,9 +225,9 @@ address or one /64, so a permanent ban does not reach neighbours who did
 nothing.
 
 An offence is a request `smallwebwaf` holds against the client: one that carries
-a clear sign of attack, breaks a limit, or is refused by a rule file, by the
+a clear sign of attack, breaks a limit, or is refused by a rule file or the Core
-Core Rule Set or for a missing or wrong token. Offences are counted by kind in
+Rule Set. Offences are counted by kind in the client's history. Two kinds lead
-the client's history. Two kinds lead to a ban, each by its own rule:
+to a ban, each by its own rule:
 
 - A clear sign of attack: a request that matches a rule file rule whose action
   is `ban`, or asks for a path in `SWWAF_TRAP_PATHS`. Examples are a probe for a

internal/proxy/passthrough_test.go

@@ -192,36 +192,6 @@ func wantRequestFields(t *testing.T, line logLine, host string, sent, received i
 	}
 }
 
-func TestAddsNoContentTypeTheAppDidNotSend(t *testing.T) {
-	t.Parallel()
-
-	const typed = "text/plain"
-
-	app := startApp(t, func(w http.ResponseWriter, r *http.Request) {
-		if r.URL.Path == "/typed" {
-			w.Header().Set("Content-Type", typed)
-		} else {
-			// The key with no value keeps Go's server from adding one.
-			w.Header()["Content-Type"] = nil
-		}
-
-		_, _ = io.WriteString(w, "<!doctype html><title>page</title>")
-	})
-	addr, _ := startProxy(t, app.URL, nil)
-
-	got := get(t, addr, "/")
-	if got.header.Values("Content-Type") != nil {
-		t.Errorf("client got Content-Type %q, want none",
-			got.header.Get("Content-Type"))
-	}
-
-	got = get(t, addr, "/typed")
-	if got.header.Get("Content-Type") != typed {
-		t.Errorf("client got Content-Type %q, want %q",
-			got.header.Get("Content-Type"), typed)
-	}
-}
-
 func TestStreamsTheRequestBody(t *testing.T) {
 	t.Parallel()
 

internal/proxy/request.go

@@ -431,13 +431,6 @@ func (rq *request) modifyResponse(res *http.Response) error {
 		return errResponseTooLarge
 	}
 
-	// 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, as the app sent none.
-	if res.Header.Get("Content-Type") == "" {
-		rq.out.Header()["Content-Type"] = nil
-	}
-
 	res.Body = &responseBody{body: limitBody(res.Body, maxBytes), rq: rq}
 	rq.startClientResponseTimeout()
 

internal/proxy/request_internal_test.go

@@ -1,55 +0,0 @@
-package proxy
-
-import (
-	"io"
-	"net/http"
-	"net/http/httptest"
-	"testing"
-
-	"sneak.berlin/go/smallwebwaf/internal/config"
-)
-
-func TestAnAnswerWithoutAContentTypeGetsNone(t *testing.T) {
-	t.Parallel()
-
-	srv := httptest.NewServer(http.HandlerFunc(
-		func(w http.ResponseWriter, _ *http.Request) {
-			rq := &request{
-				h:   &handler{config: &config.Config{}},
-				out: &responseWriter{ResponseWriter: w},
-			}
-			app := &http.Response{
-				StatusCode: http.StatusOK,
-				Header:     http.Header{},
-				Body:       http.NoBody,
-			}
-
-			err := rq.modifyResponse(app)
-			if err != nil {
-				t.Errorf("modifyResponse: %v", err)
-			}
-
-			// The headers go out with this, the first part of the body,
-			// which Go's server would guess a Content-Type from.
-			_, _ = io.WriteString(w, "<!doctype html><title>page</title>")
-		}))
-	t.Cleanup(srv.Close)
-
-	req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, srv.URL,
-		http.NoBody)
-	if err != nil {
-		t.Fatalf("new request: %v", err)
-	}
-
-	res, err := srv.Client().Do(req)
-	if err != nil {
-		t.Fatalf("get: %v", err)
-	}
-
-	_ = res.Body.Close()
-
-	if res.Header.Values("Content-Type") != nil {
-		t.Errorf("client got Content-Type %q, want none",
-			res.Header.Get("Content-Type"))
-	}
-}

internal/state/state.go

@@ -61,7 +61,6 @@ var (
 	// errMissing is for an entry without a field it needs.
 	errMissing     = errors.New("has no")
 	errCause       = errors.New("is not limit, attack, admin or crowdsec")
-	errExpires     = errors.New("is not after its start")
 	errDestination = errors.New("is not webhook, slack or ntfy")
 	errScope       = errors.New("is not client, net, asn, total or watch")
 	errWaitingList = errors.New(`waiting is a list, but now lists the alerts by ` +
@@ -647,10 +646,8 @@ func (e BanEntry) ban() bans.Ban {
 // client, a start, from which the length of the netblock's next ban is
 // worked out, or an expires, which would make it permanent. A permanent
 // ban's expires is null, which Bans cannot tell from a missing one, so
-// each expires is read again as written. An expires not after the start
+// each expires is read again as written. A cause other than limit,
-// is refused too, since the netblock's next ban for a broken limit, three
+// attack, admin or crowdsec, most likely misspelt, is refused too.
-// times as long as this one, would then refuse nothing; and so is a cause
-// other than limit, attack, admin or crowdsec, most likely misspelt.
 func (f *bansFile) check(data []byte) error {
 	var written struct {
 		Bans []struct {
@@ -671,8 +668,6 @@ func (f *bansFile) check(data []byte) error {
 			return missing(i, "start")
 		case written.Bans[i].Expires == nil:
 			return missing(i, "expires")
-		case entry.Expires != nil && !entry.Expires.After(entry.Start):
-			return fmt.Errorf("entry %d's expires %w", i+1, errExpires)
 		case entry.Cause != "" && entry.Cause != bans.CauseLimit &&
 			entry.Cause != bans.CauseAttack && entry.Cause != bans.CauseAdmin &&
 			entry.Cause != bans.CauseCrowdSec:

internal/state/state_test.go

@@ -865,21 +865,6 @@ func TestBanWithAnotherCauseStopsTheStart(t *testing.T) {
 		`: entry 4's cause "atack" is not limit, attack, admin or crowdsec`)
 }
 
-func TestBanThatDoesNotEndAfterItsStartStopsTheStart(t *testing.T) {
-	t.Parallel()
-
-	// The first ban ends a second after its start; the second ends as it
-	// starts, or an hour before.
-	for _, expires := range []string{`"2026-10-06T00:00:00Z"`, `"2026-10-05T23:00:00Z"`} {
-		wantRefused(t, bansJSON, `{"version": 1, "bans": [`+
-			`{"netblock": "203.0.113.9/32", "start": "2026-10-06T00:00:00Z", `+
-			`"expires": "2026-10-06T00:00:01Z"}, `+
-			`{"netblock": "203.0.113.10/32", "start": "2026-10-06T00:00:00Z", `+
-			`"expires": `+expires+`}]}`,
-			`: entry 2's expires is not after its start`)
-	}
-}
-
 func TestUnknownVersionStopsTheStart(t *testing.T) {
 	t.Parallel()
 

script/example-app

@@ -5,15 +5,12 @@
 # served through smallwebwaf, that a second one in a minute bans the
 # client, that a probe for /.env bans another client, which its next
 # request bans for good, that `sv stop` stops smallwebwaf in order, that
-# `docker stop` stops the container without having to kill it, that a
+# `docker stop` stops the container without having to kill it, and that
-# new container on the same volume still refuses the banned client, that
+# a new container on the same volume still refuses the banned client. The
-# a container with its state files on a new volume mounted at the
-# directory SWWAF_STATE_DIR_FILE names becomes healthy, and that one whose
-# SWWAF_STATE_DIR does not exist logs smallwebwaf's message about it. The
 # containers run with SWWAF_LOOKUP_SOURCE=off, so that no address is sent
-# to GeoJS. The containers, the volumes and both images are removed
+# to GeoJS. The containers, the volume and both images are removed however
-# however the script ends. Building the app needs network access, for
+# the script ends. Building the app needs network access, for nixpkgs'
-# nixpkgs' binary cache. script/check does not run this.
+# binary cache. script/check does not run this.
 set -eu
 
 SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
@@ -26,15 +23,10 @@ IMAGE="$NAME-base"
 APP_IMAGE="$NAME-app"
 CONTAINER="$NAME"
 VOLUME="$NAME-state"
-# The volumes of the container whose state directory SWWAF_STATE_DIR_FILE
-# names: its state files, and the file that names their directory.
-MOVED_VOLUME="$NAME-moved-state"
-SETTINGS_VOLUME="$NAME-settings"
 
 cleanup() {
     docker rm --force "$CONTAINER" >/dev/null 2>&1 || true
-    docker volume rm --force "$VOLUME" "$MOVED_VOLUME" "$SETTINGS_VOLUME" \
+    docker volume rm --force "$VOLUME" >/dev/null 2>&1 || true
-        >/dev/null 2>&1 || true
     docker rmi --force "$APP_IMAGE" "$IMAGE" >/dev/null 2>&1 || true
 }
 
@@ -84,27 +76,6 @@ start_container() {
     address="$(docker port "$CONTAINER" 8080/tcp)"
 }
 
-# start_moved_container: run the app's container with the state files on
-# a new volume at /srv/state, which SWWAF_STATE_DIR_FILE names in a file
-# only the smallwebwaf user can read, as under "Settings given as files"
-# in README.md, and wait until it is healthy. smallwebwaf can write the
-# new volume only once the run script has given it to that user.
-start_moved_container() {
-    docker volume create "$MOVED_VOLUME" >/dev/null
-    docker volume create "$SETTINGS_VOLUME" >/dev/null
-    docker run --rm --volume "$SETTINGS_VOLUME:/settings" \
-        --entrypoint bash "$IMAGE" -c 'echo /srv/state > /settings/state-dir &&
-            chown smallwebwaf:smallwebwaf /settings/state-dir &&
-            chmod 0400 /settings/state-dir'
-    docker run --detach --name "$CONTAINER" \
-        --volume "$MOVED_VOLUME:/srv/state" \
-        --volume "$SETTINGS_VOLUME:/etc/smallwebwaf/settings:ro" \
-        --env SWWAF_STATE_DIR_FILE=/etc/smallwebwaf/settings/state-dir \
-        --env SWWAF_LOOKUP_SOURCE=off \
-        "$APP_IMAGE" >/dev/null
-    wait_for "the health check did not pass" healthy
-}
-
 # refused: a request to the container gets 403, SWWAF_BAN_RESPONSE's
 # default.
 refused() {
@@ -169,20 +140,6 @@ main() {
     wait_for "smallwebwaf logged no request refused under the ban" \
         logged '"action":"banned"'
     echo "example-app: a new container on the same volume keeps the ban"
-
-    docker rm --force "$CONTAINER" >/dev/null
-    start_moved_container
-    echo "example-app: the run script gives the state files' directory named" \
-        "by SWWAF_STATE_DIR_FILE to the smallwebwaf user"
-
-    docker rm --force "$CONTAINER" >/dev/null
-    docker run --detach --name "$CONTAINER" \
-        --env SWWAF_STATE_DIR=/srv/missing \
-        --env SWWAF_LOOKUP_SOURCE=off \
-        "$APP_IMAGE" >/dev/null
-    wait_for "smallwebwaf did not report the missing state directory" \
-        logged "SWWAF_STATE_DIR cannot be written"
-    echo "example-app: smallwebwaf reports a state directory that does not exist"
 }
 
 main "$@"

share/smallwebwaf.run

@@ -5,32 +5,12 @@ set -euo pipefail
 # exits; the wait spaces out the restarts. The state directory and every
 # file in it are given to the smallwebwaf user, so that a volume mounted
 # there needs no change of owner; chown -R changes a symbolic link itself,
-# never what it points to. When SWWAF_STATE_DIR, in either form, cannot be
+# never what it points to. exec, so that the signal `sv stop` sends
-# read or is not the absolute path of a directory, no owner is changed,
+# reaches smallwebwaf itself.
-# and smallwebwaf starts and stops with a message naming the setting. A
-# change of owner that fails, as on a volume mounted read-only, does not
-# end the script either: smallwebwaf reports a directory it cannot write.
-# exec, so that the signal `sv stop` sends reaches smallwebwaf itself.
 main() {
     sleep 1
-    local dir
+    chown -R smallwebwaf:smallwebwaf "${SWWAF_STATE_DIR:-/var/lib/smallwebwaf}"
-    if dir="$(state_dir)" && [[ "$dir" == /* && -d "$dir" ]]; then
-        chown -R smallwebwaf:smallwebwaf "$dir" || true
-    fi
     exec chpst -u smallwebwaf:smallwebwaf /usr/local/bin/smallwebwaf
 }
 
-# state_dir prints SWWAF_STATE_DIR, /var/lib/smallwebwaf when it is unset,
-# or what is in the file SWWAF_STATE_DIR_FILE names, less every newline at
-# its end. It fails when both are set or the file cannot be read.
-state_dir() {
-    if [[ ! -v SWWAF_STATE_DIR_FILE ]]; then
-        printf '%s\n' "${SWWAF_STATE_DIR-/var/lib/smallwebwaf}"
-    elif [[ ! -v SWWAF_STATE_DIR ]]; then
-        cat -- "$SWWAF_STATE_DIR_FILE" 2>/dev/null
-    else
-        return 1
-    fi
-}
-
 main "$@"