fix(probes): Bound responses with a ten-byte compatibility ceiling

CHANGELOG.md

@@ -13,7 +13,18 @@ All notable changes to this project are documented here. The format follows
   large `IdleProbes` values; raise `IdleTimeout` to collect more samples and
   reach higher percentile thresholds.
 
+### Changed
+- Small probe responses must now be complete, nonempty bodies of 1–10 bytes.
+  Previously accepted empty or larger custom-server responses are invalid.
+  The ten-byte ceiling is fixed, with no caller override; it preserves the
+  observed Apple and Cloudflare responses (#40).
+
 ### Fixed
+- Declared oversized probe bodies are rejected before reading, and streamed
+  bodies after at most 11 bytes. Invalid sizes no longer become latency
+  samples; bounded warnings explain rejection while valid samples and load
+  measurements survive (#40).
+  Probe cost remains estimated; transport buffering can exceed body-read limits.
 - Idle-timeout warnings retain the preceding probe error and are omitted
   when all requested idle samples have already completed (#39).
 - Caller cancellation retains completed idle samples in partial results;

README.md

@@ -29,7 +29,8 @@ Cost       370.5 MB moved in 21.0s
 
 1. **Discovery** – fetches the server's JSON config (`/.well-known/nq` or a
    vendor path) to learn the small-download, large-download and upload URLs.
-2. **Idle latency** – `IdleProbes` sequential GETs of the 1-byte resource,
+2. **Idle latency** – `IdleProbes` sequential GETs of the small resource
+   (1 byte in the draft; up to 10 accepted for Cloudflare compatibility),
    each on a **fresh connection**, so a sample includes DNS + TCP + TLS + HTTP.
    Per-stage medians are reported via `net/http/httptrace`.
 3. **Download under load**, then **upload under load** (sequentially, so the
@@ -222,6 +223,7 @@ compromise) and transparent TCP-level proxies that pass TLS through untouched
 | `MaxDuration` | 12 s per direction | the budget: phase ends; if not yet stable → `truncated`, `reason=duration_cap`. Cost ≤ rate × 12 s |
 | `MaxBytes` | **none** (opt-in) | set on metered links; phase ends → `reason=bytes_cap` |
 | `MaxFlows` | 16 | never more concurrent load connections |
+| Small response body | 1–10 bytes | reject empty/oversized responses; read at most 11 bytes to detect overflow; discard invalid samples and warn |
 | `ctx` cancellation | – | all flows stop within ~200 ms; partial result, `cancelled=true` |
 
 The combined phase budget is `ConfigTimeout + IdleTimeout + N × MaxDuration`,
@@ -245,6 +247,22 @@ connection closed when it hands it over. There are no retries or telemetry.
 The client's `--idle-timeout` measures the whole idle probing phase; the
 server's flag of the same name limits quiet connections between requests.
 
+Probe cost is estimated, including failed loaded attempts: 5000 bytes for a
+foreign probe and 1000 for a self probe. Idle and discovery are outside the
+per-direction byte totals. Body-read limits do not prevent transport/socket
+read-ahead, and headers, TLS, in-flight work, and cancellation can exceed the
+accounted budget. `MaxBytes` is not an exact wire-byte limit. Invalid-size
+warnings are limited to one per phase/probe kind; load continues with only
+valid latency samples.
+
+The ten-byte response ceiling is fixed; there is no caller override. A server
+that changes its small response above ten bytes becomes incompatible with
+latency probing. Load continues to collect capacity measurements within the
+configured budgets, even if every probe fails; loaded latency is then absent
+and RPM is zero. Set `MaxBytes` and `MaxDuration` to bound that cost.
+[Dated compatibility checks](testdata/config/README.md) record the observed
+Apple and Cloudflare response sizes and how to repeat the small GETs.
+
 ## Deviations from the draft
 
 | Item | Draft | Here | Why |
@@ -260,6 +278,7 @@ server's flag of the same name limits quiet connections between requests.
 | Capacity change | – | a > 25 % goodput drop restarts stability tracking | The draft averages across the change. |
 | Responsiveness window | last MAD intervals | every sample since throughput became stable (`loaded_window`) | Foreign probes are sparse (a TLS handshake each); a fixed 4-tick window could hold self samples and no foreign ones, which read as "no fresh connection ever succeeded". Stability is still judged on the draft's window. |
 | Probe byte accounting | – | foreign 5000 B, self 1000 B (draft's estimates) | Counted against `MaxBytes` and the 5 % capacity rule. |
+| Small response size | 1 byte | complete nonempty bodies up to 10 bytes accepted; fixed ceiling, no override | Preserve Cloudflare's advertised ten-byte probe; reject empty and larger bodies before they become latency samples. |
 | Config `version` | must be `1` | `1` or `"1"` accepted | Lenient on the wire, strict on everything else (duplicates, hosts, scheme). |
 | Config field names | `*_download_url`, `upload_url` | also accepts Apple/Cloudflare `*_https_*` names, preferring them | Interop with deployed servers. |
 | Cloudflare target | `mach` hardcodes `h3.speed.cloudflare.com` URLs | uses `aim.cloudflare.com/responsiveness/api/v1/config`, which returns the same URLs | Keeps discovery uniform. |

cmd/nq/e2e_test.go

@@ -104,7 +104,9 @@ func TestCancelledExitCode(t *testing.T) {
 
 func TestTruncatedStillExitsZero(t *testing.T) {
 	var out, errb bytes.Buffer
-	if c := run(base(startServer(t), "--download-only", "--max-bytes", "1MB"), &out, &errb); c != exitOK {
+	// Exercise byte-cap reporting without requiring a minimum loopback rate.
+	// One accounted byte trips the cap; duration is only a generous watchdog.
+	if c := run(base(startServer(t), "--download-only", "--max-bytes", "1", "--max-duration", "5s"), &out, &errb); c != exitOK {
 		t.Fatalf("exit %d: %s", c, errb.String())
 	}
 	if !strings.Contains(out.String(), "TRUNCATED: bytes_cap") || !strings.Contains(out.String(), "Warning    download: byte cap") {

cmd/nq/probe_size_test.go

@@ -0,0 +1,90 @@
+package main
+
+import (
+	"bytes"
+	"encoding/json"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"sync/atomic"
+	"testing"
+
+	"github.com/korya/netquality"
+	"github.com/korya/netquality/server"
+)
+
+func TestInvalidProbeOutput(t *testing.T) {
+	for _, mode := range []string{"json", "human", "events"} {
+		t.Run(mode, func(t *testing.T) {
+			var small atomic.Int64
+			h := server.Handler(server.Options{MaxClientBytes: -1})
+			srv := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				if r.URL.Path == server.SmallPath && small.Add(1) > 1 {
+					_, _ = io.WriteString(w, "01234567890")
+					return
+				}
+				h.ServeHTTP(w, r)
+			}))
+			srv.EnableHTTP2 = true
+			srv.StartTLS()
+			defer srv.Close()
+			args := base(srv.URL+server.ConfigPath, "--download-only", "--idle-probes", "3", "--max-bytes", "1MB")
+			if mode != "human" {
+				args = append(args, "--json")
+			}
+			if mode == "events" {
+				args = append(args, "--events")
+			}
+			var out, errb bytes.Buffer
+			if code := run(args, &out, &errb); code != exitOK {
+				t.Fatalf("invalid probe must not fail load: exit=%d stderr=%s", code, errb.String())
+			}
+			const diagnostic = "invalid probe response size"
+			if mode == "human" {
+				if !strings.Contains(out.String(), "Warning    idle latency: "+diagnostic) || !strings.Contains(lineWith(out.String(), "Idle"), "(1 probes)") {
+					t.Errorf("partial idle/warning missing:\n%s", out.String())
+				}
+				return
+			}
+			var res netquality.Result
+			if err := json.Unmarshal(out.Bytes(), &res); err != nil {
+				t.Fatal(err)
+			}
+			if res.Idle == nil || res.Idle.Samples != 1 || res.Download == nil || res.Download.Bytes == 0 || res.Cancelled {
+				t.Fatalf("partial idle and download missing: %+v", res)
+			}
+			var warnings int
+			for _, w := range res.Warnings {
+				if strings.HasPrefix(w, "idle latency: "+diagnostic) {
+					warnings++
+				}
+			}
+			if warnings != 1 {
+				t.Errorf("idle warning missing/repeated: %v", res.Warnings)
+			}
+			if mode == "json" {
+				if errb.Len() != 0 {
+					t.Errorf("JSON stderr must be quiet: %s", errb.String())
+				}
+				return
+			}
+			var samples, warningEvents int
+			for _, line := range strings.Split(strings.TrimSpace(errb.String()), "\n") {
+				var e netquality.Event
+				if err := json.Unmarshal([]byte(line), &e); err != nil {
+					t.Fatalf("bad event %q: %v", line, err)
+				}
+				if e.Kind == netquality.EventProbe {
+					samples++
+				}
+				if e.Kind == netquality.EventWarning && strings.HasPrefix(e.Message, "idle latency: "+diagnostic) {
+					warningEvents++
+				}
+			}
+			if samples != 1 || warningEvents != 1 {
+				t.Errorf("invalid successes or repeated warnings: samples=%d warnings=%d", samples, warningEvents)
+			}
+		})
+	}
+}

docs/architecture.md

@@ -49,7 +49,7 @@ real protocol in-process over TLS + HTTP/2 without a network.
 | The dial wrappers see every connection | `DialContext` implements `test_endpoint` and records remote/local IPs. A user `DialTLSContext`/`DialTLS` bypasses it for https, so it is wrapped too: its connections are tracked for teardown and recorded, but not redirected to `test_endpoint` (DISC-9). A custom `RoundTripper` is opaque (documented limitation). |
 | The engine is pure | `internal/engine` sees only `Observation`s (elapsed, bytes, flows, probe samples) and returns `Decision`s; it holds no clock, goroutine, or socket, so the same code runs against real transports and, in tests, against recorded series or a simulator. Public latency/confidence types are aliases of engine types. The interval loop and discovery/idle/load timeout creation in `run.go` go through the injectable clock; production deadlines use `context.WithTimeout`. |
 | Cancellation via context only | Flows read bodies until the context ends; the upload body reader stops on context. Runner goroutines join and owned sockets close before return. Caller code must return promptly; a custom TLS dialer still executing at teardown has its connection closed on handover (INV-4, LIM-9). |
-| Byte accounting is client-side | Upload bytes are counted when handed to the transport, so a few MB of HTTP/2 flow-control window may be in flight beyond `MaxBytes`. |
+| Byte accounting is client-side | Upload bytes are counted when handed to the transport. Loaded probes charge fixed estimates even on failure; idle/discovery are excluded. Probe body reads are bounded independently, but HTTP/TLS overhead, transport buffering and cancellation can consume traffic beyond `MaxBytes`; it is not a wire-byte meter. |
 | Interval default 1 s, not the draft's 5 s | A 12 s budget cannot fit four 5 s intervals; see README "Deviations" (INV-6). |
 
 ## Technology choices

docs/product-specs/latency.md

@@ -74,3 +74,14 @@ mean to zero. Windows therefore reads `QueryPerformanceCounter` instead. Where
 no such clock can be reached the run falls back to `time.Now` for every sample —
 never mixing clocks within one sample — reports its numbers, and records a
 warning saying they are quantised (RES-6, INV-3).
+
+### LAT-11: Small response validation
+Idle, foreign, and self probes accept only complete, nonempty response bodies
+of 1–10 bytes. The draft's one-byte response and Cloudflare's ten-byte
+response both fit this compatibility ceiling. A declared length above ten
+is rejected before reading payload; otherwise at most eleven body bytes are
+consumed to detect overflow. Receipt of ten bytes without response completion
+does not establish success: the existing phase/caller deadline still applies.
+Empty, oversized, and incomplete responses produce no latency sample or
+successful-probe event. Rejection closes the response body, preserving other
+streams on a shared HTTP/2 load connection. Invalid-size warnings follow RES-6.

docs/product-specs/limits.md

@@ -18,6 +18,10 @@ any speed. A caller on a metered link sets `MaxBytes` (> 0), which then
 bounds bytes moved per direction, counting load payload plus a fixed estimate
 per probe (5000 B foreign, 1000 B self); reaching it ends the phase with
 `reason=bytes_cap` and a warning. `MaxBytes` ≤ 0 means unlimited.
+Failed loaded probe attempts retain their fixed charge. Idle and discovery
+traffic are outside these per-direction totals. The accounting is an estimate,
+not a wire-byte meter: headers, TLS, transport read-ahead, in-flight requests,
+and cancellation can consume more traffic than the reported budget.
 
 ### LIM-3: Flow cap
 `MaxFlows` (default 16) is never exceeded in any direction.
@@ -71,3 +75,11 @@ not an unconditional wall-clock deadline for a descheduled process or
 blocking caller code. Supplied transports, dialers, body closers, event
 sinks and log handlers must cooperate with cancellation and return promptly;
 they cannot be forcibly stopped by the library (INV-4).
+
+### LIM-10: Small response consumption
+The small-response ceiling (LAT-11) applies independently of the optional
+byte budget. At most eleven bytes of each probe response body are read by
+the client, including the byte used to detect overflow; a declared oversize
+body is rejected without reading payload. Transports and sockets may buffer
+additional data before rejection. This consumption bound neither changes the
+fixed probe estimates nor promises an exact limit on transferred wire bytes.

docs/product-specs/result.md

@@ -44,6 +44,12 @@ RPM is round trips per minute; bytes are bytes.
 ### RES-6: Warnings
 Every cap hit, fallback, proxy finding, flow error, and idle failure appends a
 human-readable string to `warnings`, and is logged at warning level.
+Invalid probe sizes produce at most one warning per phase and probe kind:
+one for idle, and one each for foreign/self in each selected load direction.
+Idle retains valid samples even when another probe has an invalid size, and
+combines the size diagnostic with any idle timeout or all-failed warning.
+Caller cancellation retains its precedence. Loaded size failures discard
+samples without aborting load. Warning messages contain no response body.
 
 ### RES-7: Events
 `RunWithEvents` delivers `phase` events for discover/idle/download/upload/done,

docs/test-matrix.md

@@ -90,6 +90,11 @@ directory relative to the repo root; `.` is the library.
 
 | Feature | Scenario | Package | Test |
 |---|---|---|---|
+| Probe body cap | HTTP/1.1 and HTTP/2: complete 1–10 bytes accepted, empty/oversized rejected, application reads bounded to 11 bytes, truncation and deadlines preserved (LAT-11, LIM-10) | . | TestProbeResponseSize |
+| Probe isolation | Rejected or timed-out self probe shares an active HTTP/2 connection; load continues on that connection (LAT-11, INV-4) | . | TestProbeRejectionPreservesLoad |
+| Rejected probe cost | Known and unknown oversized probes still hit MaxBytes with their fixed estimates, producing no goodput or latency (LIM-2, LIM-10) | . | TestRejectedProbesHitByteCap |
+| Invalid probes | Partial idle retained; both loaded series reject invalid samples, keep estimated charges out of goodput, dedupe warning events per direction/kind and reset between runs (LAT-11, LIM-2, RES-6) | . | TestInvalidProbeResponses |
+| Invalid idle | All-invalid, mixed failures, timeout and parent cancellation retain samples and diagnostic precedence with one idle warning (LAT-11, LIM-8, RES-6) | . | TestInvalidIdleDiagnostics |
 | MaxBytes | Download truncated with `bytes_cap`, warning, mean throughput fallback | . | TestBytesCap |
 | MaxBytes | Upload truncated with `bytes_cap` | . | TestUploadBytesCap |
 | MaxBytes | Counter: limit 0 never trips, positive limit trips exactly once | . | TestByteCounterLimits |
@@ -129,6 +134,7 @@ directory relative to the repo root; `.` is the library.
 | Feature | Scenario | Package | Test |
 |---|---|---|---|
 | `--json` | stdout is a `Result`, stderr quiet | cmd/nq | TestJSONOutput |
+| Invalid probe output | JSON, human and event output retain partial idle statistics and a single size warning while load succeeds (LAT-11, RES-6) | cmd/nq | TestInvalidProbeOutput |
 | `--idle-timeout` | HTTP/1.1 and HTTP/2 header/body stalls retain idle samples and a JSON warning, then complete load with exit 0 (LIM-8, CLI-2) | cmd/nq | TestIdleTimeoutOutput |
 | Binary idle cap | Built CLI terminates a stalled idle body within its watchdog and prints partial idle statistics (LIM-8) | cmd/nq | TestIdleTimeoutBinary |
 | Human output | Table, progress on stderr, `not run` directions | cmd/nq | TestHumanOutput |

options.go

@@ -71,6 +71,8 @@ type Options struct {
 	// phase, probes included; hitting it truncates the phase with
 	// reason=bytes_cap. 0 (the default) means no byte cap: MaxDuration
 	// bounds each load phase. Set it on metered links.
+	// Probe attempts use fixed cost estimates; idle/discovery are excluded.
+	// Transport buffering and cancellation can exceed this accounting budget.
 	MaxBytes int64
 	// MaxFlows caps the number of concurrent load-generating connections
 	// (default 16, draft MNP).

probe.go

@@ -3,6 +3,8 @@ package netquality
 import (
 	"context"
 	"crypto/tls"
+	"errors"
+	"fmt"
 	"io"
 	"net/http"
 	"net/http/httptrace"
@@ -24,8 +26,12 @@ type probeTimes struct {
 const (
 	foreignProbeBytes = 5000
 	selfProbeBytes    = 1000
+	// The draft uses one byte; Cloudflare's supported endpoint returns ten.
+	maxProbeBodyBytes = 10
 )
 
+var errInvalidProbeSize = errors.New("invalid probe response size")
+
 // foreignProbe performs a GET of the small URL on a brand-new connection and
 // records per-stage timings. rt must not reuse connections.
 // observe, if non-nil, receives the TLS state of every successful handshake.
@@ -100,7 +106,8 @@ func foreignProbe(ctx context.Context, rt http.RoundTripper, url string, extra h
 	return s, nil
 }
 
-// doProbe issues the GET and drains the 1-byte body.
+// doProbe requires a complete, nonempty response within the compatibility
+// ceiling. The extra byte detects overflow without draining an arbitrary body.
 func doProbe(ctx context.Context, rt http.RoundTripper, url string, extra http.Header) error {
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
 	if err != nil {
@@ -115,8 +122,20 @@ func doProbe(ctx context.Context, rt http.RoundTripper, url string, extra http.H
 	if err := checkStatus(resp); err != nil {
 		return err
 	}
-	_, err = io.Copy(io.Discard, resp.Body)
-	return err
+	if resp.ContentLength > maxProbeBodyBytes {
+		return fmt.Errorf("%w: declared %d bytes exceeds %d-byte limit", errInvalidProbeSize, resp.ContentLength, maxProbeBodyBytes)
+	}
+	n, err := io.Copy(io.Discard, io.LimitReader(resp.Body, maxProbeBodyBytes+1))
+	if n > maxProbeBodyBytes {
+		return fmt.Errorf("%w: body exceeds %d-byte limit", errInvalidProbeSize, maxProbeBodyBytes)
+	}
+	if err != nil {
+		return err
+	}
+	if n == 0 {
+		return fmt.Errorf("%w: empty body (expected 1-%d bytes)", errInvalidProbeSize, maxProbeBodyBytes)
+	}
+	return nil
 }
 
 // selfProbe performs a GET of the small URL on an existing (load) transport.