Skip to content

Commit 5abb743

Browse files
gaborageclaude
andcommitted
docs+test: address CodeRabbit review on #711
- migrations.md C51.3 detect: use `git grep` to scan all tracked config sources (nested paths, .yml, env), not just root config*.yaml. - migrations.md C51.2 (gist/gate/verify): narrow "trailing bytes" to trailing NON-whitespace — trailing whitespace still binds. - observability.md: span name keeps the request method (<METHOD> placeholder), not always GET (a wrong-method request is POST /<prefix>/*). - startup_defaults.md: distinguish known-Content-Length rejection (before the handler) from chunked/unknown-length (limited reader trips during the read). - config_test.go: clearEnvironmentVariables now unsets SERVER_BODYLIMIT so the default assertion is isolated from the ambient environment. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 83f2449 commit 5abb743

4 files changed

Lines changed: 7 additions & 7 deletions

File tree

config/config_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -788,7 +788,7 @@ func clearEnvironmentVariables() {
788788
"SERVER_HOST", "SERVER_PORT", "SERVER_TIMEOUT_READ", "SERVER_TIMEOUT_WRITE",
789789
"SERVER_TIMEOUT_IDLE", "SERVER_TIMEOUT_MIDDLEWARE", "SERVER_TIMEOUT_SHUTDOWN",
790790
"SERVER_PATH_BASE", "SERVER_PATH_HEALTH", "SERVER_PATH_READY", "SERVER_GZIP_MINLENGTH",
791-
"SERVER_RESPONSETIME_ENABLED",
791+
"SERVER_BODYLIMIT", "SERVER_RESPONSETIME_ENABLED",
792792
"DATABASE_TYPE", "DATABASE_HOST", "DATABASE_PORT", testDatabaseDatabase,
793793
testDatabaseUsername, "DATABASE_PASSWORD", "DATABASE_TLS_MODE",
794794
testDatabaseMaxConns, "DATABASE_POOL_IDLE_CONNECTIONS",

wiki/migrations.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -611,7 +611,7 @@ v0.39.1 ─E40─ v0.40.0 ─E401─ v0.40.1 ─E41─ v0.41.0 ─E42─ v0.42.0
611611

612612
## E51 · v0.50.0 → v0.51.0 — echo/v5 v5.3.0 (group implicit-404 revert + stricter JSON bind + configurable body limit)
613613

614-
- gist: The `github.com/labstack/echo/v5` bump v5.2.1 → v5.3.0 is behavior-affecting, not a pure version bump — adopt-only for consumers (no code migration required, no exported go-bricks signature changes). Three observable shifts: (1) echo restored v4's behavior where a middleware-bearing group auto-registers an implicit `/*` catch-all, so group middleware (the scheduler `/_sys` CIDR gate, the debug auth gate, any app sub-group with middleware) now ALSO runs on unmatched sub-paths and wrong-method requests under its prefix — a defense-in-depth win — and a wrong-method request under such a group returns 404 (no `Allow` header) instead of 405; go-bricks intentionally KEEPS echo's new default (does NOT set `NoGroupAutoRegister404Routes`) to preserve the gate-coverage win and hardens `HandlerContext.PathParams()`/`RouteTemplate()` to still report "unmatched" for the catch-all. (2) JSON binding is stricter — a request body with trailing bytes after the top-level JSON value is now rejected (400) where v5.2.1 silently accepted it (echo switched `Deserialize` from `json.Decoder` to `json.Unmarshal` + a pooled buffer, also a small per-bind allocation win). (3) A new `server.bodylimit` config (int64 bytes, default 10 MB) makes the request body cap configurable.
614+
- gist: The `github.com/labstack/echo/v5` bump v5.2.1 → v5.3.0 is behavior-affecting, not a pure version bump — adopt-only for consumers (no code migration required, no exported go-bricks signature changes). Three observable shifts: (1) echo restored v4's behavior where a middleware-bearing group auto-registers an implicit `/*` catch-all, so group middleware (the scheduler `/_sys` CIDR gate, the debug auth gate, any app sub-group with middleware) now ALSO runs on unmatched sub-paths and wrong-method requests under its prefix — a defense-in-depth win — and a wrong-method request under such a group returns 404 (no `Allow` header) instead of 405; go-bricks intentionally KEEPS echo's new default (does NOT set `NoGroupAutoRegister404Routes`) to preserve the gate-coverage win and hardens `HandlerContext.PathParams()`/`RouteTemplate()` to still report "unmatched" for the catch-all. (2) JSON binding is stricter — a request body with trailing NON-whitespace after the top-level JSON value (a second value or stray bytes) is now rejected (400) where v5.2.1 silently accepted it; trailing whitespace still binds (echo switched `Deserialize` from `json.Decoder` to `json.Unmarshal` + a pooled buffer, also a small per-bind allocation win). (3) A new `server.bodylimit` config (int64 bytes, default 10 MB) makes the request body cap configurable.
615615
- build-caught: none
616616
- preflight: none
617617
- exit: `go get github.com/gaborage/go-bricks@v0.51.0 && go mod tidy && go build ./... && go test ./...`
@@ -627,14 +627,14 @@ v0.39.1 ─E40─ v0.40.0 ─E401─ v0.40.1 ─E41─ v0.41.0 ─E42─ v0.42.0
627627
### [C51.2] JSON bind rejects trailing bytes after the top-level value · silent-behavior · when: match
628628

629629
- detect: audit any client/producer that POSTs to this service and appends content after the JSON document (concatenated objects, a trailing newline-delimited record, stray bytes) — not reliably greppable in this repo
630-
- gate: match = a caller sends a request body with extra bytes after the top-level JSON value — v5.2.1's `json.Decoder`-based bind silently accepted (and ignored) the trailing content; v5.3.0's `json.Unmarshal`-based bind rejects the whole body with 400. Well-formed single-document bodies are unaffected and gain a small per-bind allocation win. no-match = your callers send exactly one JSON value per body, unaffected.
630+
- gate: match = a caller sends a request body with extra NON-whitespace after the top-level JSON value (a second JSON value or stray bytes) — v5.2.1's `json.Decoder`-based bind silently accepted (and ignored) the trailing content; v5.3.0's `json.Unmarshal`-based bind rejects the whole body with 400. Trailing whitespace (a newline, spaces) is still accepted, and well-formed single-document bodies are unaffected and gain a small per-bind allocation win. no-match = your callers send exactly one JSON value per body, unaffected.
631631
- apply: fix the offending client to send exactly one JSON value per request body; there is no opt-out.
632-
- verify: `go test ./...` # then POST a body with trailing bytes and confirm a 400 (previously 200)
632+
- verify: `go test ./...` # then POST a body with trailing non-whitespace (e.g. a second JSON value) and confirm a 400 (previously 200)
633633
- ref: echo/v5 v5.3.0 · CHANGELOG 0.51.0
634634

635635
### [C51.3] New `server.bodylimit` config caps request body size (default 10 MB) · silent-behavior · when: no-match
636636

637-
- detect: `grep -rniE '(^[[:space:]]*|\.)bodylimit[[:space:]]*:|SERVER_BODYLIMIT' config*.yaml 2>/dev/null`
637+
- detect: `git grep -nEi '(^[[:space:]]*|\.)bodylimit[[:space:]]*:|SERVER_BODYLIMIT'`
638638
- gate: no-match = you leave `server.bodylimit` unset, so the new default governs — the accepted request body is capped at 10 MB (10485760 bytes) and a larger body is rejected with 413 before the handler runs. match = you set an explicit **positive** byte count, which then governs (raises or lowers the cap); an explicit `0` resolves to the 10 MB default and a negative value is rejected at config validation.
639639
- apply: leave unset for the 10 MB default OR set `server.bodylimit` (int64 bytes, env `SERVER_BODYLIMIT`) to a positive value to raise it for large-upload/bulk-import endpoints or lower it to tighten the boundary.
640640
- verify: `make run` then POST a body larger than the configured cap # rejected with 413; a body under the cap is accepted

wiki/observability.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ GoBricks provides production-grade observability built on OpenTelemetry: distrib
1010

1111
**Per-Subsystem Instrumented Tracers:** The framework ships three OTel tracers under matching scopes. `go-bricks/database` emits CLIENT-kind spans per query. `go-bricks/messaging` emits PRODUCER/CONSUMER spans per AMQP publish/consume. `go-bricks/httpclient` emits CLIENT-kind spans per outbound HTTP call — one parent "Do" span (the logical request rollup) and one child attempt span per retry attempt — and injects `traceparent` headers via the OTel propagator so downstream services join the trace. When `observability.enabled` is false no spans are emitted; the `database` tracer additionally short-circuits before building any span attributes (true zero overhead), while `messaging`/`httpclient` route into the global no-op provider (spans dropped, attribute construction not yet skipped — a tracked follow-up). See [httpclient.md#tracing](httpclient.md#tracing) for the span tree, attribute reference, and status-mapping rules.
1212

13-
**Group-Scoped 404 Route Labels (echo v5.3.0):** After the echo v5.3.0 upgrade, a 404 for an unmatched sub-path (or a wrong-method request) under a middleware-bearing group — e.g. the scheduler `/_sys` CIDR-gated group or the debug group — now resolves to that group's implicit `/*` catch-all, so its incoming-request span and metrics carry `http.route = "/<group-prefix>/*"` (span name `GET /<group-prefix>/*`) instead of the previous empty-route / bare-`GET` bucket. This is a low-cardinality change (one new series per middleware-bearing group prefix); operators with dashboards or alerts keyed on `http.route` for those 404s should expect the new series and re-point any query that matched the old empty/`GET` bucket.
13+
**Group-Scoped 404 Route Labels (echo v5.3.0):** After the echo v5.3.0 upgrade, a 404 for an unmatched sub-path (or a wrong-method request) under a middleware-bearing group — e.g. the scheduler `/_sys` CIDR-gated group or the debug group — now resolves to that group's implicit `/*` catch-all, so its incoming-request span and metrics carry `http.route = "/<group-prefix>/*"` (span name `<METHOD> /<group-prefix>/*` — the span keeps the request method, e.g. `GET /<group-prefix>/*`, or `POST /<group-prefix>/*` for a wrong-method request) instead of the previous empty-route / bare-`GET` bucket. This is a low-cardinality change (one new series per middleware-bearing group prefix); operators with dashboards or alerts keyed on `http.route` for those 404s should expect the new series and re-point any query that matched the old empty/`GET` bucket.
1414

1515
**Go Runtime Metrics:** Auto-exports memory, goroutines, CPU, scheduler latency, GC config when `observability.enabled: true`. Follows [OpenTelemetry semantic conventions](https://opentelemetry.io/docs/specs/semconv/runtime/go-metrics/)
1616

wiki/startup_defaults.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ app:
3737

3838
## Server Request Body Limit
3939

40-
`server.bodylimit` (int64 bytes; env `SERVER_BODYLIMIT`) caps the accepted HTTP request body size. A request whose body exceeds the cap is rejected with `413 Request Entity Too Large` before the handler runs:
40+
`server.bodylimit` (int64 bytes; env `SERVER_BODYLIMIT`) caps the accepted HTTP request body size, rejecting an over-cap request with `413 Request Entity Too Large`. A request with a known `Content-Length` above the cap is rejected up front, before the handler runs; a chunked / unknown-length body is bounded by a limited reader instead, so the 413 surfaces when the read crosses the cap while the handler consumes the body:
4141

4242
| Setting | Default | Purpose |
4343
|---------|---------|---------|

0 commit comments

Comments
 (0)