Skip to content

Fix #129: enforce MaxClientJsVarBytes on each browser write#147

Merged
linkdata merged 3 commits into
mainfrom
fix/129-jsvar-client-write-cap
Jul 19, 2026
Merged

Fix #129: enforce MaxClientJsVarBytes on each browser write#147
linkdata merged 3 commits into
mainfrom
fix/129-jsvar-client-write-cap

Conversation

@linkdata

@linkdata linkdata commented Jul 18, 2026

Copy link
Copy Markdown
Owner

Fixes #129.

Problem

MaxClientJsVarBytes did not bound a non-PathSetter JsVar after its normal initial render. The only cap check lived in JsVar.JawsRender, but a normally rendered JsVar is never rendered again. A browser could therefore append one slice element per Set write and keep growing server-side state.

Fix

Enforce the cap in the client input path while keeping the common path O(1):

  • Seed jsonBytes with the exact JSON length produced by the initial render.
  • Add the length of each raw client-write payload to that approximate count, including rejected appends that still grow a slice.
  • When the approximation crosses MaxClientJsVarBytes, use json.Marshal once to calculate the exact current length. Reset the approximation to that length when it remains under the cap; otherwise abort the request with ErrJsVarTooLarge.
  • Use a subtraction guard for the boundary check so the approximate count cannot overflow on 32-bit or 64-bit systems.
  • Treat an exact-size marshal failure as a failed size check: return ErrJsVarTooLarge, abort the request, and retain the underlying marshal error in the cancellation cause.
  • Format the cancellation cause and cancel the request after releasing JsVar.setMu and the caller-provided value lock.

This deliberately does not duplicate encoding/json sizing rules. Exact JSON work happens only when the raw-byte approximation looks too high.

Programmatic JawsSetPath writes remain trusted and uncapped. PathSetter values remain exempt because they enforce their own path and size policy. The render-time check remains for values already over the cap at initial render.

Tests and benchmark

Regression coverage includes:

  • valid, zero-value, and rejected append floods;
  • raw client-payload accumulation and exact-size reconciliation;
  • integer-overflow-safe boundary detection;
  • fail-closed exact marshaling without leaking handler-control errors;
  • request cancellation after internal locks are released;
  • PathSetter, disabled-cap, and server-write exemptions;
  • Linux/386 test-binary compilation.

BenchmarkJsVarClientWrite remains as the O(1)-per-write regression guard. Six-run benchstat comparison against the previous PR head (-benchtime=300ms -count=6) is statistically unchanged:

                              previous   final
SmallState                    883.6 ns   882.9 ns
LargeState                    892.9 ns   883.9 ns
Bytes/op                      272        272
Allocations/op                12         12

Verified with go test -race -count=1 ./..., go vet ./..., staticcheck ./..., golangci-lint run ./..., gosec ./..., gofumpt, go build ./..., and Linux/386 test compilation.

linkdata added 3 commits July 18, 2026 22:21
A normally rendered non-PathSetter JsVar is never rendered again, so the
render-time size check could never fire during an active request. A browser
could append one valid slice element per Set frame and grow server state
without bound (issue #129).

Enforce the cap in the client input path instead. JsVar keeps a running
serialized-size estimate seeded at render and updated after each browser
write by an upper bound on that write's growth (jq.Set only grows the value
by appending a single slice element). When the estimate crosses the cap the
true size is confirmed by one marshal of the bound value, which also reclaims
any over-count; the request is aborted with ErrJsVarTooLarge on the first
write whose confirmed size exceeds the cap. Folding an O(1) upper bound per
write and marshaling the whole value only at the boundary keeps an append
flood O(n) rather than O(n^2).

Programmatic JawsSetPath writes remain trusted and uncapped, and PathSetter
values remain exempt.

Tests cover the append-flood abort, the PathSetter and cap-disabled
exemptions, the server-write exemption, and over-count reclamation.
BenchmarkJsVarClientWrite guards against reintroducing per-write full-value
marshaling by showing per-write cost is independent of the total value size.
jq.Set grows a slice (SetLen) before assigning the appended element, so an
append enlarges the bound value even when the assign reports ErrValueUnchanged
(a zero-value append) or fails with a type mismatch (a wrong-typed append). The
cap only ran size accounting on a clean mutation, so either append flood grew
server state past MaxClientJsVarBytes without aborting the request, reproducing
the #129 unbounded-growth signature.

Account every client write once the cap applies, regardless of the mutation
error; withhold only the broadcast when the mutation did not cleanly change the
value. The boundary marshal of Ptr remains the ground-truth confirmation, so a
bounded value is reclaimed and never false-aborts while any real growth is
caught. Add regression tests for the zero-value and rejected append floods.
@linkdata
linkdata merged commit e208e9f into main Jul 19, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ui: MaxClientJsVarBytes does not stop post-render client growth

1 participant