Conversation
NewAppendWriter recovers its schema by re-parsing the file header, but had no way to receive parse-affecting SchemaOpts: WithSchemaOpts was a ReaderOpt only, and the header parse hardcoded nil opts. A file written with a lax-named schema (avro.WithLaxNames) could be written and read by this package but never reopened for append. Make WithSchemaOpts an Opt (the WithCodec pattern) and thread it into NewAppendWriter's header parse. NewWriter ignores it, documented: its schema is already parsed by the caller. Strict-named files still append with no options, and a lax-named header is still rejected without the explicit opt-in, mirroring NewReader.
…nst the complete node, bound encodeDefault
A record field default whose type subtree references a record still under
construction (a self- or mutual-recursive reference) was encoded inline at
build time against the record's partial fields slice. Binary Encode's
default-fill then emitted truncated, non-decodable wire — its own Decode
rejected it — while EncodeJSON and Resolve, which re-encode the default at
runtime against the completed node, produced the correct value. The
divergence was binary-vs-JSON, invisible to round-trip-from-typed-input.
nodeAwaitsForwardRef only treated a nil child as not-yet-ready. A self-ref
points at the enclosing record's node, which is non-nil but incomplete (its
fields slice holds only the fields declared before the current one), so the
default encoded inline and dropped the rest. Track in-construction records
in a builder-level set shared through nest, and defer their defaults to
finalize exactly like a nil forward-ref child, so encodeDefault runs once
the node is whole.
That deferral relocated the encode from the incomplete node — whose missing
fields had accidentally bounded the recursion — to the complete node,
exposing that encodeDefault fills absent fields from their own defaults
(unlike validateDefault, which skips absent fields and terminates
vacuously). An infinite default such as record R{ R self = {} } then
recursed to a stack overflow at Parse. Bound encodeDefault with the wire
codec's maxDepth (encodeDefaultDepth), turning it into a clean errTooDeep
parse rejection.
Adds TestRegression_SelfRefContainerDefaultEncodes (self array/map, deep
nesting, recursive-field-first, mutual recursion, plus empty/null/
non-recursive controls) and TestRegression_InfiniteRecursiveDefaultRejected.
A cyclic non-struct Go type (type P *P, type S []S, type M map[string]M) has a cyclic reflect type graph, so the walks that re-implement pointer indirection without the bound their canonical siblings (indirect / indirectAlloc / inferRecord's seen) carry run until the process stack overflows — an unrecoverable crash triggered by a caller-supplied decode target or SchemaFor field type, invisible to every wire/value/fuzz net. - setCustomResult: cap the decode-target pointer peel at maxIndirectDepth, returning the same SemanticError the non-custom path returns. - inferType: thread a depth bound (maxDepth, reset per record-field boundary so distinct nested structs stay bounded by seen) so a recursive non-struct type returns a clean error instead of overflowing the stack. - checkIntDefaultFitsGoKind: bound its pointer peel likewise.
- unsafe: bound tryCompileFieldSer's pointer-type peel at maxIndirectDepth.
A cyclic pointer field type (struct{ F P }, type P *P) recursed the field
type graph forever at compile time and fatally stack-overflowed the first
Encode; an over-deep chain compiled a fast path whose wire the reflect
Decode rejected. Declining at levels >= maxIndirectDepth keeps the fast
path's accept set identical to the reflect encoder's.
- reflect: tokenize the runtime struct tag with SchemaFor's splitTag grammar
(shared splitFieldTag). A naive comma split mis-read a comma inside a
default= value or an alias=[...] list as a separate option, so
"default=red,omitzero" / "alias=[x,inline,y]" spuriously fired
omitzero/inline — corrupting the zero value's wire or making SchemaFor's
own schema unencodable for its source type.
- resolve: resolveRecord re-applies a record-level CustomType (applyCustomToNode)
like every other resolve arm; previously any real evolution silently
dropped the record Decode callback and returned the raw map.
- schema,schema_node: preserve a float-syntax negative-zero default ("-0.0")
through normalizeJSONNumber and re-emit float syntax on Root().Schema()
rebuild, so the metadata Default matches the wire (-0.0) and survives the
rebuild. Integer-syntax "-0" stays the documented residual (NOT_BUGS 43).
- schema: validate the namespace attribute's components (validFullnameErr),
so the attribute spelling is rejected like the dotted spelling and an
accepted schema's Canonical() re-parses; WithLaxNames now sees namespace
components too.
- errors: render-truncate SemanticError.Field (registry-controlled, unbounded)
to prevent a per-datum error-message DoS, mirroring CompatibilityError.
A schema field/record name has no length cap at parse (validName grammar;
WithLaxNames permits anything), so a registry/remote schema can carry a
multi-megabyte name. Four runtime per-datum error paths echoed it raw,
amplifying 1:N — one oversized error string on every Encode/Decode call,
flooding logs, RPC error channels, and metric labels.
The binary type-mismatch path already bounds the name via
SemanticError.Field, which Error() render-truncates. These four bypassed
that bound: the JSON missing-field and alias-collision errors are plain
fmt.Errorf (not SemanticError), and the binary struct-mapping missing-field
rides the raw name in .Err rather than the truncated .Field. Wrap each
user-controlled name in truncForError at construction (a no-op under 80
chars, so normal schemas keep byte-identical messages), matching the
documented composed-sentence echo policy.
- json_codec.go: EncodeJSON missing-required-field + union-default no-match
- json_decode.go: DecodeJSON missing-required-field, alias collision (both
raw wire keys), and the four default-fill field-name wraps
- reflect.go: binary struct-mapping missing-field (name was in .Err)
TestRegression_FieldNameErrorEchoBounded pins all four against a 1 MiB
hostile name, with a control proving the SemanticError.Field path is
already bounded (the asymmetry these four were missing).
Cached named-type definitions are stored SELF-CONTAINED (transitive refs
inlined), so a schema referencing two types that share a transitive type —
the diamond A->{B,C}->D, or a nested type referenced before the container
whose definition carries it — spliced the shared definition twice. The
rebuild Parse rejected the duplicate, the lax-names retry failed the same
way, and the metadata forms silently fell back to the dangling original:
String()/Canonical()/Root().Schema() not re-parseable, Fingerprint()
diverging from the inline twin (SOE/registry interop), and the dangling
form was then captured into the def store, cascading into every downstream
referencing schema. Wire encode/decode were unaffected throughout (the
node tree shares one resolved type per name).
Fix: the splice walk now dedupes DEFINITIONS the way it already deduped
references. A definition of a name already defined earlier in the walk is
rewritten to a name reference (dupDefRef) — the same first-define-then-
reference rule Java's Schema toString applies via NamedSchema.writeNameRef
(Schema.java:863-870). Flat-form ("linkedin/goavro") field definitions
cannot become a bare string, so rewriteFlatFieldToRef converts them to the
equivalent normal-form reference field, keeping exactly the keys the
parser treats as field-only when lifting (name/default/order/aliases).
Reference spelling: a dotted fullname is an exact lookup (scopedRefKeys)
and always safe. A null-namespace name has only its bare short name, which
binds enclosing-namespace-first — the rewrite declines when a same-short-
name enclosing-qualified type is already defined at that point (eager
positional binding means only an EARLIER definition can capture the ref),
since a mis-bound rewrite would make the metadata describe a DIFFERENT
schema than the wire codec. That inexpressible corner (Avro has no
absolute-reference syntax; Java shares the limitation) keeps today's soft
degradation: wire correct, metadata dangling, never lying.
Pinned by TestRegression_SchemaCacheOverlappingSpliceDefs (diamond x
record/enum/fixed, null-namespace diamond, ref-order both ways, flat-form
duplicate — each against the logically identical inline twin: identical
wire bytes, canonical form, fingerprint, and re-parseable metadata forms),
TestRegression_SchemaCacheSpliceCascade (a rebuilt definition is itself a
usable cache def), and TestRegression_SchemaCacheShortNameShadowNoMisbind
(if the metadata forms re-parse they must encode identical wire bytes;
fails if the rewrite ever emits a capturable bare reference).
valueIsZero — the single source of truth for the ,omitzero zero-check
across the reflect, unsafe, and JSON encode paths — only consulted the
value method set via v.Interface().(interface{ IsZero() bool }). A field
whose type has a POINTER-receiver IsZero (the common Go idiom) was missed:
the method lives in *T's method set, not T's, so omitzero fell back to the
structural zero check and silently encoded the value instead of the field
default/null. This contradicted doc.go ("fields whose IsZero() method
returns true") and diverged from the sibling helper textOutFor, which
already reaches pointer-receiver MarshalText via v.Addr().
Reach the method through the address (boxing a non-addressable value so
Encode(v) and Encode(&v) agree), gated after the existing value-method-set
check and the nil short-circuit so the *time.Time and value-receiver cases
are unchanged.
Regression test pins both directions (a sentinel that disagrees with
structural zero) across all three encode paths, plus a value-receiver
non-regression control.
…and map reuse
Three decode paths re-implemented the natural binary decoder's target
handling and drifted from its documented contracts:
- resolveReaderUnion's TaggedUnions wrap errored on targets that
map[string]any is not assignable to (non-empty interfaces) where
deserUnion.maybeWrap skips the wrap silently; the closure now calls
maybeWrap itself on a single-branch name table so the two paths
cannot drift.
- resolvedRecord.deserInterface allocated a fresh map[string]any where
the natural decoder reuses an existing wrapped map (stale keys
retained, the documented streaming-decode contract); it now calls
reuseOrMakeStringAnyMap.
- The JSON decoder's wrapUnion applied the TaggedUnions envelope
unconditionally, so assignAny rejected it for non-empty interface
targets; it now mirrors maybeWrap's silent skip.
Resolved DecodeJSON funnels through the resolved binary deser, so the
{natural,resolved} x {binary,JSON} cells now agree on both contracts,
pinned per cell by the new parity tests.
… any depth)
doc.go promised depth-first field precedence ("the shallowest wins; among
fields at the same depth, a tagged field wins over an untagged one"), but
both field resolvers — reflect.go's typeFieldMapping and schema_for.go's
collectFields — deliberately run the tag tiebreak first: a tagged field
beats an untagged one at ANY depth, and depth only breaks ties among
same-tagged-status fields. The behavior has been pinned since the original
deserialization commit (TestTypeFieldMappingTaggedBeatsUntagged); the doc
sentence arrived later and never matched it. Rewrite the sentence to the
implemented rule, and stop quoting the old wording in collectFields'
dedup comment and repeated_embed_test's doc reference.
Two pins back the rewritten contract where coverage was thin:
- TestTypeFieldMappingSameDepthTaggedBeatsUntagged: a same-depth
tagged/untagged collision is a tiebreak (tagged wins), never an
ambiguous-collision error — the runtime twin of the existing
SchemaFor pin.
- TestRegression_EmbeddedPointerStructNoPanic gains a sub-test pinning
that decode through a PRE-ALLOCATED unexported embedded pointer
succeeds and fills the promoted exported field: the documented
refusal is specific to a nil embed (reflection cannot allocate it),
and must not widen into refusing non-nil unexported embeds.
toJSONWalk recurses items/values/branches/fields with only a pointer-cycle map to terminate it. A hand-built acyclic chain (array<array<...>> a million deep) has no repeated pointer, so the walk recursed until the goroutine stack overflowed and the process died uncatchably — reachable via the public SchemaNode.Schema(), and via SchemaFor with a hand-built CustomType.Schema, both before the eventual Parse (whose maxSchemaJSONDepth pre-scan would have rejected the JSON) could run. Thread a depth counter and cap the walk at maxSchemaJSONDepth, the same ceiling Parse enforces. Any tree shallow enough to encode/decode sits far below it (the wire codec's maxDepth is 4x smaller), so a usable tree is never rejected; a deeper one stops with a clean error on the dedup path or a truncated subtree Parse rejects on the bare path, instead of crashing.
The tree-walk depth bound covers the structural node nesting (items/values/branches/fields), but a Props value or a SchemaField.Default is an arbitrary user-supplied JSON tree that descends a SEPARATE recursion: jsonSerializableValue (needsJSONFixup/applyJSONFixup) and then json.Marshal, none of which bounds depth. A hand-built node one level deep carrying a deeply-nested Props value or default overflows the goroutine stack uncatchably -- recover cannot catch a stack overflow -- before Schema's eventual Parse (whose maxSchemaJSONDepth pre-scan would have rejected the JSON) ever runs. Reachable via the public SchemaNode.Schema() and via SchemaFor with a hand-built CustomType.Schema. Bound the value at the same maxSchemaJSONDepth ceiling as the structural walk, charging the structural depth already accrued so the total marshaled nesting stays within one ceiling. An over-deep value records the error on the dedup path (Schema returns it) and truncates on the bare path (the marshal cannot crash), mirroring the structural walk's own over-depth handling. The check short-circuits at the cap, so a hostile value cannot overflow the check itself (a 2M-deep value rejects in ~1ms).
…eld resolves it collectFields errored eagerly the instant two same-depth fields collided, before the rest of the walk could see a higher-priority field that resolves the name. reflect.go's typeFieldMapping -- which collectFields must agree with -- instead records the collision and lets a later shallower or tagged field clear it, deferring the genuine-ambiguity decision to the end. So a struct that embeds two structs sharing an untagged field name and then declares its own field of that name (the common embeds-first-own-fields-after layout) round-tripped fine through Encode/Decode and compiled fine in Go -- the direct field unambiguously shadows the deep ones -- yet SchemaFor rejected it as a duplicate. Record collisions and clear them on a resolving field, then reject only names left genuinely ambiguous at their winning depth after the full walk -- matching typeFieldMapping, Go's field promotion, and doc.go's contract that an ambiguous collision is one Go itself makes a compile error.
…atching binary
A logical type placed on an Avro kind it is not spec-valid for (uuid/duration
on bytes, big-decimal on fixed) is soft-dropped by validateLogical and restored
only when a registered CustomType matches — a match that also suppresses the
codec, so the contract is the raw Avro-native value on every wire format. The
binary decoder honored this (the suppressed build uses the base deserBytes/
deserFixed), but two JSON decode paths transformed:
- assignBytes (the typed-target decoder shared by decodeBytes and decodeFixed)
ran its uuid/duration/big-decimal arm regardless of node.kind, so a string
target read a hex-dash UUID, an avro.Duration target succeeded where binary
errored, and a *big.Rat target read a transformed big-decimal — all while
binary returned raw bytes.
- hasDecimalBareNumberArm (the lenient bare-number convenience, also shared by
decodeBytes and decodeFixed) was kind-agnostic, so fixed+big-decimal decoding
a bare number still produced *big.Rat.
jsonDecodeAppliesLogical's *any-probe correctly reported "no transform" for these
(decodeLogicalBytes/Fixed are per-kind and omit the wrong-kind arm), so no
suppression wrapper installed and the typed/bare-number paths ran un-suppressed —
the probe's correct-by-construction guarantee only holds when the shared typed
helpers' per-kind transform set matches the *any set.
Gate each assignBytes arm and hasDecimalBareNumberArm to the logical's spec-valid
kind (big-decimal->bytes; duration/uuid->fixed; decimal->both), restoring the
match. The encode side was already immune (appendAvroJSON's bytes and fixed arms
are separate per-kind switches); decodeInt/decodeLong are immune (separate
functions, kind-disjoint logicals); the metadata/parse-validate axes already
surface such defaults as raw []byte.
valueNestsTooDeep bounds a hand-built SchemaNode's Props/Default value before it reaches json.Marshal at SchemaNode.Schema(), so a deeply-nested value cannot overflow the goroutine stack uncatchably. It matched only map[string]any and []any — the shapes Schema.Root() produces from parse — but the map[string]any field accepts any Go value, and json.Marshal recurses into every container kind. A hand-built typed container ([]map[string]any, a struct, a []*T chain) nests just as deeply yet bypassed the type switch, reaching json.Marshal unbounded (a 2M-deep value stack-overflowed the process via the public SchemaNode.Schema()). Broaden the bound to a reflect walk over map/slice/array/struct/pointer/interface, mirroring json.Marshal's traversal, decrementing the budget on every descent so the check terminates at the bound rather than hanging on a cyclic Go type (type P *P); []byte/[N]byte short-circuit (a base64/number scalar, never a nested array). A 2M-deep typed container now rejects in ~3ms with a clean error instead of crashing. The SchemaFor channel (CustomType.Schema, embedded via toJSON) routes through the same bound; its existing pin was vacuous (SchemaFor[int32] errors before walking) and is corrected to a struct field that exercises the walk.
…(DoS)
The SchemaNode->JSON walk reached via SchemaNode.Schema() (and SchemaFor with a
hand-built CustomType.Schema) bounded only DEPTH. Depth is orthogonal to a
shared-reference DAG: the same *SchemaNode reached via a node's Items AND Values
pointer, or the same sub-value reached via two map keys ({"a":x,"b":x} repeated
per level), is tiny in memory yet fans out into a 2^depth tree when serialized --
neither toJSONWalk nor valueNestsTooDeep nor json.Marshal memoizes shared
references, and toJSONWalk's visited map is path-scoped (defer delete) so off-path
sharing is not a cycle. A ~40-node graph (shallow, so the depth cap never fires)
demands 2^40 emitted nodes and hangs/OOMs the process before Parse runs.
Add maxSchemaJSONNodes, a node-count budget shared across the whole walk
(structural nodes plus every Props/Default value), decremented per node and
checked before descent so the fan-out is pruned at the frontier and json.Marshal's
cost (the same expanded tree) is bounded too. Reject is ~211ms structural / ~45ms
value; benign shallow reuse, named-type dedup, and a 500-deep / 10k-field tree
still build.
Extend the committed battery to pin every cell so no bound can be silently
removed: depth on all four structural channels (Items/Values/Branches/Fields),
all three value sites (node Props, field Props, field Default), and every
container kind json.Marshal recurses into; plus the expansion axis on every
channel and a benign-sharing boundary.
…once on the raw value When a reader field is absent from the writer and filled from its default through resolution, resolveRecord wraps the reader field node's raw (logical-suppressed) deser with the custom decoder chain exactly once. Pin that the custom Decode fires a single time on the raw Avro-native value -- matching a natural decode -- on both the resolved binary and resolved JSON paths, guarding against a double-wrap or against feeding the callback the enriched logical value.
… budget (DoS)
SchemaNode.Schema()'s named-type dedup conflict check re-marshalled duplicate
subtrees via toJSON(), which allocated a fresh maxSchemaJSONNodes budget — so k
identical-bodied distinct-pointer copies of a w-node named definition drove
O(k*w) re-marshals outside the shared budget (the outer walk charges only 1 per
re-occurrence, since it emits a bare reference, not the body), reaching
~budget^2 work from a tiny output (one definition + k-1 references). Reachable
via the public SchemaNode.Schema() on a hand-built node; Parse→Root()→Schema()
is not affected (parsed re-references are bare {"type":"Name"} ref nodes that
fail isNamedKind).
Thread the shared budget into the comparison (toJSONShared, not toJSON); an
exhausted budget reports the over-budget error rather than a spurious body
conflict (asymmetric truncation could otherwise make identical bodies compare
unequal).
…lows fastavro (Java is record-only) The unqualified-name tier applies to record/enum/fixed, matching fastavro's match_types; Java's firstMatchingBranch does structural short-name matching only for records (enum and fixed require an exact full-name match inside a union). The comment previously read "matching Java/fastavro", which is imprecise for the enum/fixed union case. Comment-only.
…xt method A ,uuid-tagged [16]byte that also implements TextMarshaler/AppendText/ TextUnmarshaler (e.g. github.com/google/uuid.UUID) was inferred as a plain Avro "string", silently dropping the fixed(16) shape and the uuid logical type and diverging from an identical text-less [16]byte field. The codec trusts the raw bytes of a uuid-on-fixed [16]byte and never consults its text method, so the text-interface arm in inferType must not intercept it. Guard the arm to let a uuid [16]byte fall through to the fixed(16) Array case. Pinned by TestRegression_SchemaForUUIDByteArrayWithTextMethod.
decimalScaleLimit caps the scale, not the unscaled-value byte length, and the decimal deserializers never see precision. A legal-scale decimal with a multi-megabit unscaled value drove big.Rat.FloatString base-conversion (json.Number/string targets) or SetFrac GCD (high-scale targets) at O(M(n)*log n) -- a 1 MiB unscaled value spent ~1s. Java/fastavro/avro-rs store significand+scale and never base-convert. Add maxDecimalUnscaledBytes (32 KiB) plus a shared checkDecimalUnscaledLen, applied at every decode entry point (setDecimalValue, decodeLogicalBytes, decodeFixed's decimal arm, parseBigDecimalPayload) plus a defensive cap in bytesToRat for the public RatFromBytes. precision is parse-capped at decimalScaleLimit, so a minimally-encoded in-precision value needs <= ~27 KiB and is never rejected; the cap fires only on out-of-precision / sign-padded hostile input, uniformly across bytes/fixed/big-decimal x binary/JSON x every target type (including the resolution string->bytes promotion path). Pinned by TestRegression_DecimalUnscaledLengthDoS.
deflateCodec.Decompress read io.LimitReader(r, maxOut+1) to detect an
over-limit stream without materializing the whole bomb; at maxOut == MaxInt64
("effectively unlimited") the +1 wrapped to MinInt64, so io.LimitReader read
zero bytes and a valid block decoded as empty -- WithMaxDecompressedBlockBytes(
math.MaxInt64) then made a valid deflate file fail to read. Increment only when
maxOut is below MaxInt64.
Pinned by TestRegression_OCFDeflateDecompressLimitMaxInt.
The SchemaNode->JSON walk reached via SchemaNode.Schema() (and SchemaFor with a hand-built CustomType.Schema) bounded DEPTH (maxSchemaJSONDepth) and emitted NODE COUNT (maxSchemaJSONNodes), but never the SIZE of the per-node scalar payload. The node budget counts nodes; it cannot see a leaf's size, because the intermediate any-tree stores every string and []string BY REFERENCE (assigning n.Doc or n.Symbols is O(1), charging one node) while json.Marshal re-expands each one. So a single multi-MB Doc/Symbols, or a modest one shared across many distinct nodes (K nodes each emitting one L-byte shared string is O(K+L) in memory but K*L in the output, since Go strings/slices share backing storage and json.Marshal memoizes nothing), blows the output past memory while the node count stays tiny -- 200 enums sharing one 20k-symbol slice emitted 32 MiB from ~20k input symbols with no error; scaled to the node budget that is TB-scale output / OOM, reachable via the public API before Parse runs. Add maxSchemaJSONBytes, a byte budget shared across the whole walk (every type/name/namespace/doc/logicalType/enum-default string, every symbol and alias, every Props key and string/[]byte value, and every value-leaf walked by valueWalkLimit), and charge string-slice ELEMENT COUNT against the existing node budget (each element is an emitted array node). Type/Name/Namespace are charged before the fullname is hashed into the dedup map or emitted as a reference, so a huge shared Name cannot amplify via per-occurrence hashing. Over-budget payload is never handed to json.Marshal: the dedup path records a clean error, the bare path truncates deterministically (so the dedup conflict comparison stays meaningful, the byte-axis analogue of e76cd84's node-axis check). 64 MiB sits far above any real schema yet caps json.Marshal's peak. This closes the metadata-walk hostile-input surface across all three axes (depth, node count, payload bytes) in one pass rather than dribbling one bound per round. TestRegression_SchemaNodeWalkBudgetBattery drives the whole surface (every recursion point, fan-out point, and per-node cost x both paths x boundary checks) so a later schema_node-walk DoS find extends it; non-vacuity was confirmed by neutering takeBytes and takeNodes. Valid-schema behavior is unchanged (charges only alter output for pathological input that previously crashed/hung).
Consolidate resource-bound (DoS) coverage into one executable matrix of every public entry point x hostile-input class, so the dribble of one DoS fix per round is closed wholesale instead of reopened each round. dos_battery_test.go (40 cells) drives Parse/SchemaCache/SchemaFor, Decode/DecodeJSON/DecodeSingleObject, Encode/EncodeJSON/AppendSingleObject, Root/Canonical/String/SchemaNode.Schema, and Resolve/CheckCompatibility against deep nesting, large count/length, number-CPU, error-echo, metadata DAG/value, and cyclic-Go-type inputs. ocf/dos_battery_test.go (7 cells) covers NewReader/NewWriter against decompression amplification, block count/size, zero-run, header metadata, and error-echo. Each cell drives the real public API, asserts the bound holds (fast reject / bounded error / terminate, never hang/panic/OOM), and cites the dedicated regression test pinning the extreme case. Adds the one previously-untested cell: non-custom Decode into a cyclic-pointer target (type P *P), bounded by indirectAlloc/maxIndirectDepth -- the bound existed but was only inferred from the custom-decode path's pin.
When a CustomType matches a soft-dropped logical that sits on a kind it is not spec-valid for (uuid on bytes; a date/time/timestamp logical on string), buildComplex's general primitive path applied logicalSer(o.Logical) -- keyed only on the logical NAME, kind-blind -- so binary Encode wrote the logical form: serUUID emits a 36-char UUID string into a bytes value, serTimestampMillis emits a bare varint long into a string value. The binary decoder was already suppressed to raw on the custom match, the per-kind JSON encoder writes raw, and the JSON decoder reads raw, so binary Encode diverged from EncodeJSON, and a string-backed time logical produced a wire its own Decode rejected (a bare varint read back as a string length -> "short buffer"), i.e. self-incompatible. Gate the logicalSer application on logicalUnderlyingAccept[o.Logical](o), the same predicate validateLogical uses to soft-drop a wrong-kind logical. A resurrected wrong-kind logical now keeps the base (raw) serializer, matching the suppressed decoder, the JSON encoder, and the decode side. Spec-valid placements are unchanged -- there the logical serializer is a strict superset of the base. Only b.ser changes; the node, canonical form, and fingerprint are untouched. logicalSer is the lone kind-blind logical-codec selector: the fixed build's per-logical switch and appendAvroJSON's per-kind arms were already immune.
TestRegression_DecimalUnscaledLengthDoS asserted a 100ms wall-clock bound on every decimal decode entry point. The JSON codepoint path's residual cost is the unavoidable O(n) scan of the 1 MiB string -- the unscaled-length cap fires only after the scan materializes the bytes -- and race instrumentation inflates that scan past 100ms while the capped base conversion never runs, so the test failed under -race even though the cap works (the non-race suite passes). Use a generous threshold under -race (isRaceEnabled), matching the sibling TestRegression_ParseFloatLengthCapDoS; a real unbounded base conversion (~2.7s for a 1 MiB unscaled value, far more under -race) still trips it, so the test stays meaningful in both modes.
…e plain fixed
A CustomType registered for a logical name resurrects a logical that
validateLogical soft-dropped (buildComplex restores the dropped logicalType
when a matching CustomType exists). For a fixed underlying, uuid soft-drops at
size != 16 and duration at size != 12 (logicalUnderlyingAccept). The fixed
build's per-logical switch then selected serFixedUUIDReflect (always 16 bytes) /
serDuration (always 12), and appendAvroJSON's fixed arms wrote the same --
SIZE-blind -- while the suppressed decoder reads deserFixed{size}. So a
no-Encode custom on, e.g., {"type":"fixed","size":20,"logicalType":"uuid"} made
both Encode and EncodeJSON write a 16-byte wire their own Decode/DecodeJSON
reject (short buffer: need 20, have 16), silently breaking a round trip the
plain (no-custom, soft-dropped-to-raw) fixed completes.
Gate the fixed-build logical serializer on logicalUnderlyingAccept[logical](o)
(the same size predicate validateLogical soft-drops with) and the JSON uuid /
duration arms on node.size, so a resurrected wrong-size logical keeps the base
serSize{size} / size-checked raw path -- matching the suppressed decoder, the
plain fixed, and Java (LogicalTypes.Uuid/Duration.validate reject a wrong fixed
size; fromSchemaIgnoreInvalid soft-drops to a plain fixed). decimal-on-fixed is
unaffected (hard-errors on nil precision before resurrection; serFixedDecimal is
size-aware) and the unsafe fast path is declined when a custom is present. This
is the wrong-size sibling of the wrong-kind primitive case: the fixed switch and
the JSON per-kind arms were immune to wrong KIND but not wrong SIZE.
Pinned by TestRegression_CustomSuppressionWrongSizeFixedLogicalEncodeParity
(passive custom must match the plain fixed for a raw [size]byte and the
size-blind serializer's own logical-shaped input, both wires) plus the
uuid-on-fixed16 / duration-on-fixed12 boundary rows in
TestRegression_CustomSuppressionSpecValidLogicalStillApplied.
…ogical raw A logical type on an Avro underlying it is not spec-valid for is soft-dropped by validateLogical and resurrected only when a registered CustomType's LogicalType matches (buildComplex). A resurrected wrong-kind/wrong-size logical must fall through to the RAW size/kind-checked path on every axis -- the contract the plain (no-custom, soft-dropped) schema already obeys. Three prior rounds gated the ENCODE side on logicalUnderlyingAccept (25a6e66 JSON-decode wrong-kind; 8236008 binary-encode wrong-kind; ff86592 fixed-encode wrong-size) but left three DECODE-side counterparts: - The binary primitive deser selected logicalDeser(o.Logical) under the bare !hasMatchingCustomType gate. Resurrection keys on LogicalType ONLY, while suppression (hasMatchingCustomType) keys on LogicalType AND AvroType, so a CustomType whose AvroType names a different kind resurrects the logical yet does NOT suppress -- the un-gated branch then applied serUUID/serTimestamp*'s deser on the wrong kind (uuid->hex string off a bytes wire, a time logical -> time.Time off a string wire) while the kind-gated JSON path stayed raw. - The fixed build's duration/uuid deser selected deserDuration/ deserFixedUUIDReflect (which read a fixed 12/16 bytes) under the bare hasAny gate, so the same mismatched-AvroType resurrection decoded a size!=12/16 fixed via the size-blind logical deser, diverging from the plain fixed. - json_decode's assignBytes duration arm fired on any fixed kind regardless of size, so a wildcard CustomType resurrecting duration on a size!=12 fixed decoded into avro.Duration on JSON (DurationFromBytes reads the first 12 of N bytes) while binary's deserFixed{size} errored -- the wrong-SIZE sibling of the wrong-KIND assignBytes gates added in 25a6e66. Gate the decode deser on logicalUnderlyingAccept identically to the encode ser (new logicalUnderlyingAcceptsObject helper, shared by both primitive directions; hasAny || !logicalUnderlyingAccept[L](o) for the fixed build), and add len(b)==12 to the assignBytes duration arm so it matches decodeLogicalFixed's *any arm and the uuid arm's len(b)==16. Encode and decode now apply validateLogical's own predicate symmetrically, so a resurrected wrong-kind/size logical is raw on both directions, both wire formats, and natural + resolved (identity and promotion -- promotionDeserForLogical is already reader-kind-first, so it never re-applies a wrong-kind logical) deserialization. Pinned by TestRegression_CustomResurrectedLogicalFullMatrixParity, one battery driving the whole matrix: 13 logicals x every soft-droppable underlying (86 cells) x {wildcard, AvroType-match, AvroType-mismatch} custom shapes x encode/decode x binary/JSON x natural/resolved x {any, logical-typed} targets, each asserted byte/value/accept-identical to the plain soft-dropped schema and self-readable through its own reader. Non-vacuity verified by neutering each gate (all five sites go red). json_codec's per-kind JSON-encode arms were already kind/size-safe and need no change.
…oracle One generator crossing every type × context × boundary value, run through the calibration-free runCore battery plus an INDEPENDENT wire oracle the calibration-free path cannot provide. The oracle pins encode-side bit exactness: it confirms the binary float encoder preserves a signaling-NaN payload exactly, while documenting (via the jsonLossy split) the provable fact that Avro JSON text canonicalizes every NaN to one token and so cannot carry the payload. Adds the boundary values the legacy frag tables omit: 2^53±1, MaxInt64/MinInt64, ±Inf, signaling-NaN, empty, large.
Drives every typed scalar through bare/struct(unsafe)/[]T/map[string]T/*T at boundary values, asserting safe==unsafe==generic==container byte-identity plus the independent oracle. Confirms the float32 fast/slow split preserves a signaling-NaN payload through every typed position (the slow path is taken for non-bit-exact float32→float64→float32 values, and stays exact).
The tag tables have three guards and only one of them had a cell that could see it. The degrade in the logical-name pass is the operative one: it is the sole protection for deser.logicalNames, which is the tag the BINARY decoder wraps a value in. The round-trip matrix drove only the JSON wire, so removing the degrade left every asserted cell green while the binary wrap emitted a tag its own encoder routes to a different branch. The matrix now runs the same branch-preservation assertion through the binary decode wrap: decode the schema's own wire with the tag options, hand the resulting envelope straight back to Encode, and require the branch index to come back unchanged. Removing the degrade now reds six cells. The other two guards protect ser.branchNames and have no reachable input while the degrade stands, because a degraded qualifier equals its branch's exact name and the loop skips it. That is recorded where they live, with the measurement: either one alone leaves the tables byte-identical, and it takes dropping the degrade AND the qualifier's taken check together to move the ser table onto a branch the JSON decoder disagrees with.
A caller-written union tag is resolved by two implementations: findUnionBranch
scans, and ser.branchNames is a map built at parse time. findUnionBranch had
three tiers -- the branch's exact name, the goavro-interop "<kind>.<logicalType>"
qualifier, and the unqualified short name -- as three open-coded loops, and the
map restated two of the three by hand. The one it did not restate is the legacy
qualifier for a named fixed carrying a logical type, so a caller's
map[string]any{"fixed.uuid": v} encoded as JSON and was refused on binary,
under a comment claiming the two accept the same tagged input shape.
The tiers are now a slice both consumers walk. The accept-sets are equal by
construction rather than by agreement, and the hand-written copy of the
short-name tier is deleted rather than kept in sync. Adding a tier reaches both
sides; adding one by hand inside either reaches neither, and that is what the
guards refuse: one asserts each consumer walks the slice exactly once and holds
no branch scan of its own, another requires every tier to be reached by the
corpus so a tier cannot ship unexercised. Both were attacked in both directions
-- adding a tier, open-coding one, removing one, and unguarding one each fail a
different guard.
Deriving the set produced a behavioral change the report did not contain. The
qualifier tier had no ambiguity guard where the short-name tier did, so a tag
two branches could claim resolved to the first of them: two distinct named
fixeds may carry the same logical type, and {"fixed.uuid": v} silently decoded
as the first on JSON while binary refused it. Fixed is the only named kind in
that tier's vocabulary, so it is the only shape where this is possible. Both
wires now refuse such a tag rather than picking one. The qualifier tier's early
return was dropped as provably dead: a dotted name cannot match the short-name
tier, because an unqualified name never contains a dot.
Resolution stays allocation-free. A tier reports its claim as head, separator
and tail rather than building it, so matching compares in place; only the
parse-time table build ever joins the pieces. An earlier version passed a
scratch buffer into the tier instead, which escaped through the indirect call
and put an allocation on every resolve. That is pinned.
The budget batteries' wall-clock deadline now scales with race instrumentation.
It is a liveness backstop that turns a hang into a failure, and at 30s it sat
inside the band of legitimate work: those batteries are at the node budget by
construction, so under -race they run 21s and 33s in isolation and exceed 30s
under the suite's parallelism, with no data race reported.
A schema grows two ways: it nests deeper, or it declares more siblings at one level. Depth is capped by a pre-scan and pinned. Breadth is not capped and needs no cap -- a union of twenty thousand named branches, or a record of twenty thousand fields, is legal Avro that a registry, an RPC handshake, or an OCF file header can hand a reader. What it needs is for every pass over those siblings to stay linear in their count. Two were not. The union tag tables asked "does another branch claim this name" by scanning the branch slice, once per branch. The tier walk stays exactly as it was -- every branch is still offered to every tier of unionTagTiers, and the accept-set is unchanged -- but the ambiguity check now counts claims once per tier into a map allocated on first use and reused across tiers, so an ordinary union pays at most one allocation for the whole walk and a union with no guarded tier pays none. A megabyte of schema went from 1.89s to 95ms. The comment that chose the scan reasoned that a union has a handful of branches, which is a claim about input rather than about structure; the allocations it was avoiding had already been measured at 411 versus 412, stack-allocated. Matching a writer record to a reader one scanned the reader's fields once per writer field, from both Resolve and CheckCompatibility, so both were quadratic in field count where Java's lookup is a hash map. It is now one readerFieldLookup built per reader record and asked in constant time. The two maps stay separate because the rule is that every field NAME outranks every field ALIAS, not that a name outranks an alias on the same field: a writer name that is one reader field's alias and a later reader field's name resolves to whichever entry a merged map wrote last. That routing is what the parse-time rejection of field name/alias collisions is justified by, so it is a contract, and it is pinned against a hand-built record because the parse refuses the shape that shows it. resolveRecord also built a writer-name map that nothing read; it is gone. The battery gains a breadth column, and its axes are derived rather than listed. The tier axis is read from unionTagTiers, the lookup-site axis from the newReaderFieldLookup builders found in source, and the entry-point axis from the cell labels the battery's other columns already drive, so an entry point added to any column arrives here with no cell and fails. Exemptions carry the input they take instead and are checked for staleness. Each derivation was attacked in both directions. Two cells did not reach what they claimed to measure, and a guard caught both: the routing cells were all refused at parse, and the name-hit cost cell tripped Resolve's canonical-equality short-circuit and timed that. Both are now structurally asserted. The emit tables' degrade has the identical scan shape and is left alone on a structural bound -- its condition holds only for an unnamed kind carrying a logical type, and a union may hold at most one branch per unnamed kind, because a second is refused as a duplicate union type.
…e table A union's tag table and an enum's symbol index are the tier rule and the symbol list applied once, at parse time. Both were built for the binary encoder and hung off the serializer, and the JSON codec walks a *schemaNode, from which a *serUnion is not reachable -- so every question those tables answer was re-derived per value by scanning the siblings. A union's branch count and an enum's symbol count are set by the schema text and the value count is set by the data, so the cost was the product of two numbers a caller chooses, where the binary twin was constant. Each scanning site carried a comment saying it mirrors the binary path; every one of them was true about the answer and silent about the cost, which is why no parity or accept-set net could see this. The counterexample was already in the package: node.fieldIdx, the one lookup hung on the node, is the one both wires share. unionTags and the enum symbolIdx now live on schemaNode beside fieldIdx. The table is allocated once per union and refilled in place rather than reassigned, because finalizeUnionNames rebuilds it after forward references bind and a reassigned field would leave whichever holder was wired first pointing at the pre-finalize map. serUnion and serEnum hold that same allocation, so the two wires' accept-sets are equal by identity rather than by two walks agreeing. findUnionBranch asks the table; scanUnionBranch keeps the tier walk as the fallback for a node built without one and as the oracle the table is checked against. Resolution's synthesized nodes carry the reader's table alongside the reader's siblings -- defense in depth today, since Resolve returns the reader's node and keeps only the resolved tree's deser, but a node that holds the slice without the lookup is a scan waiting for its first reader. Matching a writer node to a reader union branch had the same shape one level up: findMatchingBranch scanned every reader branch and both CheckCompatibility and Resolve called it once per writer branch. Indexing a rule is where a rule quietly changes, and Java's Resolver.firstMatchingBranch scans per writer branch too, so there is no reference to re-derive the verdict from -- the constraint is that the verdict not move. It is stated once, as data: branchMatchTiers gives each rank the names a reader branch answers to and the name a writer node asks with, and both the builder and the query read that one table. The promotion rank derives its vocabulary from the promotions map. A fixed's size stays part of the match key rather than a post-selection check, so a wrong-size same-name branch is skipped and a later matching one wins. At twenty thousand siblings with the answer placed last, the JSON paths go from 19.3s, 19.9s, 6.8s, 6.25s, 9.7s and 3.1s to tens of milliseconds, and the tag's position stops changing the cost at all -- which is the table's signature, since a scan cannot help but tell first from last. Resolve goes 31.8s to 57ms and CheckCompatibility 15.6s to 11.7ms. Every new helper inlines, including at the three binary hot-path call sites. The breadth column gains the axis that would have caught this. Its entry points were already derived from the cells the rest of the battery drives, but every one of those cells used a wide record and encoded a single value, so neither the union and enum containers nor any once-per-value pass had a cell. The sibling kinds are now read out of schemaNode by reflection -- every slice-valued field, which is the mechanical form of "its length comes from the schema text" -- and each must be driven or exempted with the reason it cannot grow. Aliases and bare aliases had never been driven by anything. The cells drive many values with the answer placed last, because a cell that encodes one value cannot observe a pass that runs once per value. The two cells that parse a megabyte of schema text get a ceiling of their own. Parse and SchemaCache.Parse cost about 140ms and 300ms of measured-linear work at this size -- doubling the branch count doubles both across 5k through 40k -- so at a flat 500ms they sat within 1.7x of their own cost and a merely busy host reported a complexity change that had not happened. A bound only separates linear from quadratic when it clears the linear cost by more than noise; restoring the old quadratic dup scan still reds all nine of those cells at 2.0-2.8s.
The FULL round that found the class -- a parse-time lookup built for one wire, re-derived per value by the other -- and the fix round that closed it by hanging the lookup on the node both wires walk. Two lessons worth keeping. Mirroring is the tell: every scanning site carried a comment saying it mirrored the binary path, true about the answer and silent about the cost, so no parity net could see it. And a derived axis only protects the axes below it that are also derived -- each of the last three rounds derived one level and hand-listed the next, and the hand-listed level held the next finding every time.
A fixed's size is the one parse-time quantity whose VALUE is not bounded by the
length of the text declaring it -- nineteen characters name 2^63, and the parser
deliberately leaves the upper bound open to match the lenient majority, since a
size past the datum simply fails at encode/decode. Arithmetic that carries such
a magnitude therefore has to say what happens at the top of the range.
schemaMinBytesSeen did not. Its record arm sums field minimums under a guard
testing only `s >= math.MaxInt32`, and a wrapped-negative sum is not greater
than a positive number, so it passes straight through: a record of
{long, fixed(MaxInt64), fixed(MaxInt64)} sums to exactly -1. Every caller
computes `1 + that` and checkMapBlockBounds divides by it, so a one-byte payload
divided by zero. All four derivations of the bound reach it -- the parse-time
one, the resolver's rebuild, the skip compiled for a dropped writer field, and
the one a container reader derives from a schema it read out of a file header,
which is where the schema is supplied by the input rather than the caller.
checkArrayBlockBounds does not divide, but a non-positive minimum silently
routes an element that occupies real bytes through the ZERO-BYTE element cap
instead of the buffer-relative one, which is a misclassification rather than a
loose bound.
The union arm's shape is separate and was not the crash: `m := math.MaxInt`
means a branch minimum equal to the sentinel fails `v < m`, so the union reports
the no-branches answer, one byte. A found flag replaces the sentinel.
The ceiling is stated ONCE. maxDecimalDigits already clamped this magnitude to
its own ceiling and jsonDecodeAppliesLogical to a third, which is three numbers
for one question; maxSchemaMagnitude is now the single answer, chosen against
the widest multiplier any consumer applies to a magnitude so the product stays
inside a 32-bit int on every build, and maxDecimalDigits asks it rather than
reasoning its own. Its verdict is unchanged: validation rejects a precision
above decimalScaleLimit before it is called, and the saturated capacity is far
larger than that. The third ceiling stays where it is because it bounds an
ALLOCATION rather than arithmetic -- 128 MiB is a fine addend and a terrible
make() length -- and the accessor says so, so the reason lives in one place even
though the number cannot.
The set this belongs to is not readers of the size field. The wrap is a sum over
field count and holds no size read anywhere in its expression, while the field
itself has 27 non-test reads, most of them comparisons, which cannot overflow.
So the new guard derives the set by reachability: seeds are the magnitude-
bearing fields, magnitude propagates through arithmetic, through an integer-
returning function and through an integer-typed PARAMETER (the last is what
reaches maxDecimalDigits, whose magnitude arrives as an argument), sinks are the
arithmetic operators and make() lengths, and results and parameters are filtered
to integer types because a magnitude handed on as []byte or string has left the
domain. That yields 17 expressions in 13 functions, each carrying a verdict and
a reason including the over-reports -- reflect.Type.Size() reads as a magnitude,
since the derivation has no type information. The guard fails on an unrowed
site, a stale row, a drifted count, and on its own rot.
The battery had no column for a schema-declared magnitude; C11 adds one and
catches SchemaCache.Parse, which the new matrix does not drive. Census Q22
records the question.
…elds once A named type referenced twice binds BOTH references to one *schemaNode, so the graph these walks descend is a DAG rather than a tree. Two walks re-descended per reference, and both cost 2^depth on a schema whose text grows linearly. Neither needs deep nesting to get there: every level can be declared as a sibling field wired by forward reference, which puts the same fan-out at a JSON nesting depth of four, past any bracket pre-scan. Measured before the fix, a 2.1 KB schema took 3.8s and a 2.3 KB one 19s, through Parse, SchemaCache.Parse, Resolve, and ocf.NewReader, where the schema comes out of the file rather than from the caller. schemaMinBytes now memoizes. The interesting part is WHICH results it may remember, because the obvious condition is unsound. A back-edge does not return the referenced node's minimum -- it cannot, that computation is still running -- it returns a conservative stand-in, so a result reached through one is a property of the PATH. Asking only whether a back-edge escaped ABOVE the node while it was computed is not enough: a memo is also CONSUMED, and a later entry has a different path, so if any node currently being computed lies inside n's subtree, recomputing n would hit it as a back-edge and get a different answer. Mutually recursive A and X, where A is computed from outside and then consumed from inside X, gives 67 where a walk with no memory at all says 51. The exact condition is that n's subtree is entirely cycle-free, since an on-path node inside n's subtree that also reaches n is exactly a cycle through n. No cost test can see that distinction, because a wrong memo is FASTER rather than slower, and the fully-expanded twin cannot either: the discriminating schemas are cyclic and have no finite expansion. The oracle is a transcription of the walk with no memo at all, run per node -- whatever it computes is entry-independent by construction, because it never carries anything between entries. That is TestInvariant_MemoAgreesWithUnmemoizedWalk, whose corpus is the shapes where the distinction exists: mutual recursion, and a node whose true minimum is BELOW the cycle stand-in, where remembering the stand-in would make the bound too tight and refuse real data. The exact condition leaves a residue. A cyclic DAG cannot be memoized at all, so a chain of mutually recursive levels, or one strongly-connected component, still fans out per reference. maxMinBytesVisits bounds that. Exhausting it is sound in the one direction that matters: reaching the cap requires a subtree of cyclic references, which means the node the caller asked about is a record, union or container above them, and every one of those costs at least one wire byte, since the reference closing a cycle can be neither null nor an all-null record. The stand-in is therefore never ABOVE the true minimum, so the bound is loose rather than wrong, and nothing is refused for exceeding the budget -- it is classified in the cap table as not a bound. A budget and a memo bound different things and mask each other. With the budget in place, removing the memo leaves every cost cell green, because the budget stops the walk either way; the memo's observable is the VALUE. So the memo has a cell asserting the exact minimum at a scale only a memo reaches, and the expected number is arithmetic on the schema's own definition (fan^levels) rather than anything read off this package. Four attacks, four disjoint red sets: memo off reds only the exact-value cell, budget off reds only the cyclic cost cells, memoizing unconditionally reds only the un-memoized agreement, and the collector fix below reds only the placement family. schemaMinBytes is entered only where a container asks for a per-element wire minimum, so a cell built on a bare record DAG measures nothing at all. TestInvariant_MinBytesCallSites derives that set from source -- every call, rowed with the public entry point that reaches it -- and fails on an unrowed caller, a drifted count, and a row whose file no longer calls it. Every cost cell checks its own trigger before measuring. The second walk is collectFields, and its fix is a hoist rather than a repair. Its duplicate-name resolution sat at the end of the RECURSIVE collector, so it ran at every level while the index paths it resolves accumulate from the root. Below the root it therefore called t.FieldByIndex with a root-relative path against a nested type: a panic where the path steps into a non-struct, the wrong Go field names where it happens to resolve, and -- the worse half -- an ambiguity decided at a level that cannot see the outer scope, rejecting a type whose collision a shallower field resolves. Go promotes such a type unambiguously, encoding/json marshals it, and this package's own encoder already handled it, because typeFieldMapping keeps the same resolution outside its own recursive closure. resolvePromotedFields is now that block, called once from collectFields over the root's complete set. The index parameter is gone from collectFields, so there is no root-relative path left to mis-resolve. Re-basing the index would have fixed the panic and left the false rejection. Two implementations agreeing on a RULE and disagreeing on WHERE IT RUNS is invisible to everything that compares answers, since at the root both placements agree. censusAnswerer now carries the placement, checked against source. It names the walk whose collected set the rule ranges over, because "reachable from some recursion" is a different question that answers yes for nearly everything -- schema inference recurses too, and a collector running once per level of THAT walk is correct, each level being a different type. And it checks reachability rather than containment: extracting a rule into its own function and calling it from inside the walk moves the text and changes nothing. The guard fails on a whole-set fact for a rule now inside its walk, a per-level fact for one that no longer is, and a walk that does not recurse. The DoS battery already owned this axis -- C6 is titled metadata DAG / shared-reference fan-out -- and missed it because its rationale said the axis was reachable only by hand-building a SchemaNode. That premise was wrong: a named reference in ordinary schema text is the carrier, and it needs no depth. C6 now crosses the axis with the entry-point list in both spellings, and ocf's header column gained the cell where the schema is supplied by the file. Every one of them reds on a neutered memo. The doc string needed no change. doc.go and README already state the rule the collector now follows, and already name Go itself as its authority, so the matrix uses reflect.Type.FieldByName as an executed oracle -- it returns false for an ambiguously promoted name -- and the documentation became true.
The allowance added with the memo counted NODE COMPUTATIONS. Entering a record iterates that record's own fields, so the walk's cost was the product of two magnitudes the schema author picks independently, and only one of them was capped. Its rationale reasoned carefully about what EXHAUSTING the allowance costs in correctness -- the stand-in is never above the true minimum, so a bound derived from it goes loose rather than wrong -- and never about what REACHING it costs in time. A cyclic component whose widest record carries 16k fields took ocf.NewReader 21.7s on a 502 KB header, where the schema comes out of the file. It is now charged per CHILD EXAMINED, taken before descending, which makes the unit of the allowance the unit of the work. walkBudget.takeNodes charges the same way for the same reason, and adopting that discipline rather than inventing a second one is deliberate. The constant is renamed to maxMinBytesWork because its unit changed, and raised so an acyclic schema keeps its exact bound: the memo makes an acyclic walk cost the sum of the nodes' child counts, which the schema text bounds, so a schema would have to run to tens of megabytes before an honest parse traded the exact bound for the loose one. The sibling sweep is where the class gets its real statement. walkBudget has no such residue, and the reason is not that it charges bytes -- it is WHERE it charges. takeNode runs at the top of every entry, ahead of the cycle and dedup checks that can return early, so a child costs a unit whether or not the walk descends through it. The min-bytes walk charged AFTER its memo, which is exactly how a memo hit examined a child for free. So the question to ask of an allowance is whether any path reaches real work without passing the charge, and that immunity is now measured rather than read: TestInvariant_MetadataWalkChargesPerChild drives the same shape through Root().Schema(), String() and Canonical(). Both directions of the behavior change: a schema between the old and the new exhaustion points now computes the exact minimum where it used to get the stand-in, which tightens the derived block bound and rejects only wire claiming more elements than could fit; a single record past four million fields now takes the stand-in where it used to compute exactly, which loosens. Nothing asserted either. The cost cells already carried a cyclic shape and it passed, because its fan is 2 -- fan is what drives the path count, so every cyclic cell measured allowance times two. dagWideSCC is the width axis, and the constructor is the deliverable rather than the cell, because the shape is easy to build slightly wrong and a slightly wrong one proves the opposite of what it looks like. Three properties decide it, measured at a matched 124 KB rather than reasoned about: the graph must be CYCLIC, or the memo answers each node once and there is no repetition for the width to multiply (600x); the width must be CONCENTRATED at the node every root-to-leaf path ends at, since a node is recomputed once per path that reaches it and spreading the same total width over D levels divides each computation's cost by D (15x); and the filler fields must have wire minimum zero, because the chain above a wide record doubles its minimum per level and reaches the magnitude ceiling a dozen levels up, where a saturated running sum returns early -- before the field that continues the fan-out (3x). The charge and the work are two switches over one vocabulary, and a kind added to one alone is silent: an unaccounted arm restores the product, an over-counted one spends the allowance on descents that never happen. TestInvariant_MinBytes ChargeCoversEveryChildArm reads both out of the source and compares them, and was attacked in both directions. Four attacks, two of them in series rather than disjoint. Charging per entry reds only the width cells; removing the allowance reds the cyclic depth cells too. So the width cells' operative guard is the UNIT and the depth cells' is the allowance's existence, which is worth naming: without it a later round deletes one and reads the other's red as coverage.
schemaMinBytes' cost is a product of three magnitudes a schema author picks independently -- containers x paths-per-walk x children-per-node -- and the two prior rounds capped only the inner two: the done memo bounds the paths, the per-child charge bounds the children. The outer factor was still open. A fresh minBytesWalk (fresh memo, fresh allowance) was built on EVERY call, and the call fires once per array/map container, so the per-call cap bounded one walk while the number of walks was the caller's container count. N containers pointing at one cyclic SCC each re-walked it to allowance exhaustion; N over an acyclic chain each re-walked it with a fresh memo. Measured off file- and caller-supplied schemas: Parse 56 KB -> 138 s, Resolve 17 KB -> 32 s, first Decode of a resolved decoder that drops a writer field 17 KB -> 33 s, ocf.NewReader on a 17 KB header -> 30 s; the acyclic form is O(containers x levels) quadratic (600 KB -> 3.4 s). One newMinBytesWalk() is now threaded through each whole operation: finalize's forward-ref container-fixup loop, the resolveCtx (resolveArray/resolveMap and the dropped-field skip compiler), and each record's skip compile (buildSkip gained an mbw parameter; skipRecord's once.Do makes its own, because that compile fires once per wire reach so cross-record cost is already wire-bounded). The shared done memo now makes the acyclic case exact across all containers, and the shared allowance bounds the cyclic residue once for the operation instead of once per container. Every site drops to ~120 ms. Sharing across the build->finalize boundary is deliberately avoided: build-time calls run on unresolved forward-ref stubs and on inline nodes final at build, distinct from the resolved targets finalize walks, so finalize gets its own fresh walk and no build-time provisional leaks into it. Build keeps a per-call walk -- a backward-referenced container resolves its items to a cheap name stub, so build carries no container factor (measured 3.3 ms at 256 containers). The one observable change is the maintainer-ruled drained-allowance stand-in for later containers on a cyclic schema: looser than the exact value, both sides reject on it, and an acyclic schema barely touches the allowance so its later containers stay exact. The class is now an enumeration rather than a fourth factor waiting to be found. budgeted_walk_census_test.go rows every budgeted walk in the package with its cost as a product and the single bound that caps the product, classified by what it traverses: a shared graph (schema DAG, Go-type DAG) needs a memo or a budget because depth cannot bound a DAG; a value/wire/text tree has node count equal to input size and a depth cap suffices. The walk set is derived from source two ways -- a graph-cost-marker scan (allowance, walkBudget, nodePair memo, graph visited-sets, defer-delete) and a schema-graph self-recursion scan -- so a new walk that carries cost state, or that recurses over the schema graph with none, fails the guard; it is attacked both ways. The enumeration cleared every other schema-graph walk (checkCompat and resolveNode's pair memo, toJSONWalk's budget, the visited-memo walks, the no-marker tree walks) and left the Go-type walks to their compile-time bound. TestInvariant_MinBytesContainerCountBounded crosses the container-count axis with the paths axis through every entry point, and ocf's many-container-header-schema cell drives the file-supplied form that sets the severity; neutering the shared walk back to one-per-container reds every cell. dosRun takes a wider ceiling under -race (raceDosBudget), matching wantAcceptUnder: race instrumentation inflates a bounded walk several-fold, which false-tripped the fixed 4 s bound on the widest pre-existing metadata cell; the unraced bound is unchanged.
The prior commit shared one min-bytes walk across finalize, resolve, and skip but left the parse BUILD path constructing a fresh walk per container, on the premise that a backward name reference "uses a cheap name stub." That premise held only for one reference direction. When the cyclic type is defined FIRST and fully wired at build -- each level nests the next inline and references it a second time by name, the deepest closing to the enclosing type, so both spellings bind to one node and the whole cycle is built before any container -- a container's items reference resolves to the fully built node, and schemaMinBytes walks it in full at build. A schema can point any number of containers at that one type, so the build path paid the container count times a full walk: 258 ms to 7.4 s across 1..32 containers, and 8.3 s at 32 off a 64 KB header through ocf.NewReader. A forward reference is an unwired stub at build (cheap, fixed in finalize); only the backward direction pays, which is why the earlier measurement missed it. The builder now carries the shared minBytes walk (like named/building, copied across nests). Parse and SchemaCache.Parse seed it; a lazy seed at the top of build covers a builder constructed directly, before any nest copies the pointer, so the build container sites never dereference a nil walk. The build walk is kept separate from finalize's: build-time calls can run over subtrees whose forward references are not yet wired, and that provisional memo must not leak into finalize, which recomputes on the fully-wired graph. Every path is now flat ~120 ms. The census gains the axis it lacked. It enumerated WALKS, but this is one walk reached by several construction paths -- build, finalize, resolve, skip -- and a bound that holds on one path and not another is invisible to a per-walk row. Each row now names its reaching paths, and TestInvariant_MinBytesReachingPaths derives every walk-state constructor from source, requires each to be a per-operation site, forbids a fresh-per-container construction anywhere but the single standalone, and forbids a production caller of that standalone. TestInvariant_MinBytesContainerCount Bounded gains a backward-reference family and ocf a matching backward header cell; neutering only the build sites reds exactly the backward cells, so the reaching path is discriminated rather than merely covered.
checkArrayBlockBounds does not treat the per-item minimum as a magnitude, it selects a RULE: positive takes the buffer-relative bound, zero takes the zero-byte cap, and the two admit incomparable sets. So the stand-in of 1 that minBytes reported whenever it could not compute a value was not a loose bound but a different one, and it moved every legitimately zero-byte container onto a rule it cannot satisfy. Three encode/decode parity breaks, each the package's own encoder producing a wire its own decoder rejects: a drained allowance on the build path and on resolve, and -- needing no adversarial schema at all -- a plain forward reference to an empty record, where a 3-byte wire's accept or reject turned on field order alone. Every stand-in is now a sound lower bound or an explicit minBytesUnknown, whose array rule admits the union of what both computable rules admit and so can only loosen. The back-edge stand-in stays 1 and carries its proof: escaping a cycle costs a union index or a container terminator, so a cyclic type's minimum is at least 1. mapEntryMinBytes becomes the single constructor of the per-entry number for all four map sites, which is also what keeps 1 + (-1) = 0 out of a divisor, and inner.minBytes -- the unsafe reader's copy, computed by a second call and never patched by the forward-ref fixup -- now shares the one computation. With that, sharing the walk across records is safe by construction rather than by argument, so skipRecord takes the operation's walk instead of building one per record: 43.5s -> 147ms at 256 references, 5.4s -> 142ms through ocf.NewReader with a reader schema. minBytesOf locks, and is never in the decode path. The memo is what makes sharing worth it -- 496us against ~14.2ms for 64 records over one shared acyclic subtree -- so a per-record memo was not an alternative. Census rows now carry the counts their walk is shared across and the cell that DRIVES each at two or more values, read from the row so the cell cannot disagree with it. A row with no cell fails, and a cell holding its own factor constant fails -- which is what the skip row was doing behind a true sentence about the wire.
The nil-child stand-in is only reachable when the forward reference sits BELOW the container's direct child. An array whose items IS the reference is registered in containerFixups, so finalize re-derives its minimum from the resolved node and the build-time answer never reaches the wire path -- that form accepts on both declaration orders even before the fix. One level down nothing re-patches: the items is an inline record (resolvable, so not a fixup) whose own field is the reference. Both shapes are cells of the pin now, the direct one labelled as a control, so the requirement cannot be simplified away. The nil-arm neuter reds the nested/forward cell alone.
The measured-bound rule was stated for the min-bytes walk's construction sites and never applied to the wall-clock cost cells, so five of them pinned one magnitude each and the suite stayed green. A requirement whose guard passes its known violators is the defect the rule exists to catch. Measured before designing: every factor is flat with a correct bound -- width 80->8000 grows the schema text 65x and the parse 1.4x, because the walk dominates and the allowance caps it -- so all of them took a real second value and none needed an exemption. The derivation finds seven cells, not five. A cost generator is derived from source (a func taking magnitudes and returning a schema string), and any test calling one is a cost cell: that adds SharedSchemaNodeWalkedOnce, whose name reads like a value oracle and which the hand pass believed, and TestDoSBattery_C6_MetadataWalk, missed entirely. The first derivation was wrong in both directions -- a generator named in a comment counted as a call, one passed as a function value did not, and bodies ran to the next test rather than the closing brace -- so it now blanks comments and string literals in place and matches identifiers over code while brace-matching for extents. MetadataWalkChargesPerChild was named for a bound it does not measure: with takeNode disabled entirely it stays green, because a parsed schema is deduped before that walk and no parse-driven cell can reach the node budget. It reds on the min-bytes charge instead, SchemaNode.Schema ending in a re-Parse. Renamed to say what it drives, and the three cells that do own the node budget -- all hand-building trees Parse cannot express -- are named in its row and in toJSONWalk's. The helper now takes a build function returning the thunk to time, so per-magnitude preparation is structurally outside the clock; the metadata cell had its MustParse inside it, which is why it moved when the parse's bound was neutered and sat still when its own was. floor is documented as the largest cost the bound itself permits, which for a cell whose shapes include an un-memoizable one is a single exhausted allowance. Guard arms, both directions: unrowed caller fails, fewer than two distinct values fails, a cell that does not name itself to the helper fails, an exemption on a cell taking the wall-clock harness fails, values on a cell taking none fails, a row naming no test fails. Value oracles are rowed explicitly with what they assert instead.
…cond factor The -race relaxation was an absolute 3s FLOOR, so the cells whose normal bound had been deliberately RAISED got the least headroom: breadthParseBound is 1500ms against cells costing ~300ms, which left 2x, while every 500ms cell got 6x. C10a redded at 3.50/3.54/4.92s on correct code, and nondeterministically -- the same commit passed at 12.8s. All three of its tiers measure LINEAR under -race across a doubling (x1.63/x1.24/x1.99), so the ceiling was the defect. It is max(3s floor, 10x) now, both numbers measured: per-cell race/no-race ratios across the battery run 2.3x to 8.3x, and detection survives because the quadratic the widest cell catches is 32s unraced. Nothing below a 300ms normal bound changes. The rule turned out to be stated seven times -- a helper applying the floor, an inline copy of it, three hand-written ceilings at 6x, 10x and 30x, a dosRun budget pair whose comment claimed to mirror the helper and did not, and a build-tagged hangDeadline pair the new guard found only because it existed. All seven ask one authority now, and the two test packages share it through export_test.go rather than each declaring a mechanism. A guard derives every consult of the build-switched predicate -- by SHAPE, a bool declared true under `race` and false under `!race`, closed transitively over aliases and wrappers -- and fails on an unrowed one or a stale row. The cost-cell derivation was a name prefix (`dag|nContainers`) in one file in one package, so it passed its known violators: C9 drove dosChainSchema at a single magnitude through wantAcceptUnder, which was not even in its harness vocabulary, and the whole ocf battery was invisible because censusSourceFiles skips every _test.go. It derives by shape now -- an int parameter, a string result, a body writing an Avro "type" key -- over every test file in every package of the module: 21 generators against 9, 16 cells against 7. The six newly visible cells drove one magnitude each and drive two now, measured flat or linear. Cells that cannot reach the registry (a different package) are tied to their row by the values appearing in their file; cells whose magnitude is not schema text at all name their carrier, which is the one thing here the source cannot check for us. G3 closes with a cell driving BOTH factors of the reflect collectors' cost. The ruling that had closed it as cost-only credited a per-type sync.Map with amortizing repeats; that is true of typeFieldMapping and false of collectFieldsRaw, which has no memo, so SchemaFor re-pays the whole 2^depth walk on every call. The cell asserts the decode side's amortization and that neither collector accumulates, and logs the depth pair rather than bounding an exponential that is by design. The conformance table carried a comment saying this package accepted unknown \X escapes and omitted the row on that basis. It rejects them, at all eleven string positions, agreeing with encoding/json on all twelve escapes tested. Row added, class netted, and the sweep for the comment's siblings turned up one more: an exclusion citing a file and a test that both no longer exist. Nine neuters, nine distinct red sets. Two found holes in the new guards, and both surfaced by attacking with an ADDED member rather than a removed one: a declaration filter that swallowed any consult sharing a `var` line, and a magnitude that satisfied the cross-package check by being named in a COMMENT. The G3 cell was corrected mid-round when its first neuter came back green -- two caches in series amortize the decode and neither is visible alone.
…ly has race_on_test.go and race_off_test.go no longer exist -- the avro_test half of the pair was replaced by the export_test.go bridge, so the two packages share one build-tagged mechanism instead of mirroring two. The comment still named the deleted files. The archive gains only the distillation the size guard required.
Fingerprint's doc attributed big-endian output to the call and pointed at
single-object encoding for the little-endian form. That holds for
CRC-64-AVRO only. The byte order belongs to the algorithm: a crypto/sha256
digest is a byte string with no byte order, and it already matches the
other implementations -- reversing it would break a comparison that works.
Executed against fastavro 1.12.2 on
{"type":"record","name":"R","fields":[{"name":"a","type":"int"}]}:
twmb sha256 135f1e593a2f576b404e07c59cb3dbc00abd442f2b62aa6214e3b0bf0bdcadcf
fastavro sha256 135f1e593a2f576b404e07c59cb3dbc00abd442f2b62aa6214e3b0bf0bdcadcf
twmb CRC-64 35b061f31ba01537
fastavro CRC-64 3715a01bf361b035
Java draws the same line: SchemaNormalization.fingerprint reverses only
inside its CRC-64-AVRO branch, returning md.digest(data) untouched for
every MessageDigest algorithm.
fastavroRabinBytes had the two orders swapped, in its comment and in its
beHex / rabinBEHx names. The reversal it performs was always right, so no
pin was ever wrong -- but the names pointed the next reader at the wrong
direction for deriving a new vector. fastavro prints the single-object
wire order (little-endian); Fingerprint returns big-endian, which is how
Go writes every integer hash -- crc32, crc64, adler32 and fnv all put the
high byte first.
NewWriter adopted the caller's Codec and then returned an error on three paths without ever closing it. A failing constructor hands back no Writer, so nothing is left for the caller to Close -- and the documented idiom builds the codec inline in the call, WithCodec(MustZstdCodec(nil, nil)), so the caller has no handle either. NewReader has released on every error path since it grew a named return and a defer; NewAppendWriter releases on its seek path. NewWriter was the member that never got the sweep. Two changes. NewWriter takes NewReader's shape, with the defer registered BEFORE the option loop rather than after it: the closure reads wr.codec when it runs, so it covers the codec whenever the loop adopts it, and an error return added inside the loop later is guarded without anyone having to notice. Until WithCodec is seen wr.codec is nullCodec, whose Close is a no-op. And the reserved-metadata-key rejection moves below that loop. Inside it, the rejection returned before a later WithCodec had been adopted and after an earlier one had, so whether the codec was released turned on where the caller happened to write the option. Behavior change worth stating: with WithMetadata written first, a failed NewWriter now closes the caller's codec where before it never took it. From outside the codec those two states are indistinguishable -- which is exactly why uniform adoption is what makes the release checkable at all. Rejecting after collection is also cheaper than the accept path it replaces: 100k entries with the reserved key last rejects in 12.9ms against 17.3ms to accept 100k legal ones, and a 1 MiB reserved key rejects in 30us. The constructor set is derived from source rather than listed. A struct with a field of type Codec is codec-owning; a top-level function returning a pointer to one alongside an error is a constructor that can fail after adopting it. Asking go/ast for that shape instead of matching a "New" prefix keeps the set independent of how a future constructor is spelled -- a declaration using named results is caught the same as one using bare types. The guard fails in all four directions: a constructor with no row, a row naming a constructor the source dropped, a row naming a covering test that no longer exists, and a derived member whose row was deleted. Its scope, stated in the test: this package's own non-test files. TestConstructorErrorReleasesCodec crosses constructor x error arm x option order. The success cells pin the other side -- a constructor that returns a usable Writer must NOT have closed the codec it is about to compress with. The NewReader arms stay with their existing pins; the table links them rather than duplicating them. Ten neuters, ten distinct red sets: removing the release defer reds four NewWriter failure cells; moving the reserved-key check back into the loop reds only the codec-last cell; dropping NewAppendWriter's seek-path Close reds only its cell; removing NewReader's defer reds its two pins; making the defer unconditional reds the success cell; and the five guard attacks each name their own member. leakDetectCodec counts closes instead of recording a bool, so the pins assert exactly-once rather than at-least-once. WithCodec's and NopCloser's doc contracts widened: a constructor that fails after taking the codec closes it too.
Fingerprint wrote the canonical form into the caller's hash without clearing it first, so the digest depended on whatever that hash already held. Two calls on one h gave 35b061f31ba01537 and then 293315e405eebe12; a caller who had written their own bytes into h got a third answer. Only the first of those is a fingerprint. Reset on the way IN, not on the way out. Entry buys the property outright -- the digest is determined by the schema and the algorithm, whatever the caller did with the hash beforehand. Exit would only fix the repeat call, leaving a pre-written hash still contaminating the answer, and it would clear state callers legitimately read back: TestFingerprintArrayItemsMatches SpecVector and TestFingerprintRabin both take Sum64 off the hash they passed, and an exit reset returns them the empty-hash constant c15d213aa4d7a795 instead of the schema's CRC. No pin asserted the old behavior. Every in-repo site either hands over a fresh hash or writes an explicit h.Reset() between calls -- the tests worked around the missing reset rather than depending on it -- so the reset is a no-op for all of them, for the README, and for doc.go. The pickaxe over the changed lines reaches only the two commits that introduced the call. TestFingerprintIsAFunctionOfTheSchemaAlone crosses algorithm x prior hash state x schema shape: six algorithms spanning 4 to 64 output bytes, six prior states (fresh, after fingerprinting the same schema, after fingerprinting a different one, after the caller wrote its own bytes, after write-then-reset, and after two fingerprints), and three schemas including one whose canonical form spans more than one compression block. Each cell's expectation comes from a fresh hash of the same algorithm fed the canonical form -- the definition of the digest, computed without the code under test -- and each cell also asserts the digest is still readable off the hash afterward. The entry point set is derived rather than listed: any exported function or method with a parameter from the hash package takes a caller-owned accumulator and owes the same rule. The guard fails both ways, on an unrowed entry point and on a row the source no longer backs. Four neuters, four distinct red sets. Removing the reset reds 72 purity cells and the plain repeat-on-one-hash pin. Moving it to the exit reds 108 cells -- the 72 plus the 36 after-the-call state assertions -- while leaving the repeat pin green, which is the difference between the two placements in one observation. Emptying the row and adding a second hash-taking method each red the guard by name.
…it fails WithCodec offers a codec; at most one offer is taken. NewReader and NewAppendWriter take a supplied codec only when its Name matches the header's avro.codec, and NewWriter takes only the last WithCodec written. Every other offer was dropped on the floor -- and unlike the failed-constructor case fixed last, these constructors SUCCEED, so nothing signals the caller that their codec went unused. With the documented inline form, WithCodec(MustZstdCodec (nil, nil)), there is no handle left to close it with either. Measured before the fix: zero Close calls for all three, even after Writer.Close/Reader.Close, against a control showing the adopted codec closed exactly once. The answer to "which supplied codecs went unused" now comes from one place. resolveCodec already decides which offer it takes, so it returns that index alongside the codec (-1 for a built-in resolved by name), and releaseUnadopted turns index plus offers into the set to close. Both reader-side constructors get the answer from the same return rather than each working it out; NewWriter, whose chooser is the option loop rather than a name match, adopts after the loop instead of overwriting a field inside it, which is what makes a superseded offer visible as an offer rather than as a value that was there a moment ago. Each constructor registers the sweep as a defer before its option loop, for the reason the existing codec-release defer is registered there: the closure reads the slice and the index when it runs, so it covers whatever the loop collected however the constructor exits. An arm that returns before a codec has been chosen leaves the index at -1 and releases every offer, which is the correct answer for a constructor that adopted nothing -- NewReader's mutually-exclusive reader-schema rejection and both header-read failures reach exactly that. Position alone cannot decide whether to close, because one codec can occupy several positions. WithCodec(c), WithCodec(c) makes index 0 unadopted by position and the very codec the Writer is about to compress with by identity; closing it would turn a working call into a use-after-close, so the release is by distinct codec, not by index. Repeats are recognized through a map, whose key equality is the comparison this would otherwise write by hand. A codec whose dynamic type is uncomparable cannot be a key at all, and comparing two of them with == panics rather than answering, so those are tracked by type instead; two values of one uncomparable type are indistinguishable, and answering "same" there skips a Close rather than performing one, which is the safe direction. The split is on reflect.Type.Comparable, a property of the type, so the ordinary case stays linear: 10k offers went from 491ms to 921us. A nil offer is never closed. WithCodec(nil) compiles, and when a later WithCodec supersedes it the constructor succeeds today -- the nil never reaches a call site -- so closing the superseded offer without checking would turn a working call into a panic. A nil offer that is ADOPTED already fails earlier, in writeHeader and in resolveCodec's name scan; that was asserted in a comment and then executed, panicking on all three constructors as claimed. WithCodec's contract states the consequence rather than leaving it inferred: a caller sharing one codec must give it a Close returning nil or wrap it in NopCloser. That was already required -- an adopted codec is closed by Writer.Close and Reader.Close, so a bare shared codec was already closed out from under the next user -- and it now holds for a declined offer too, which makes the rule uniform instead of contingent on whether the offer was taken. The net crosses constructor x disposition x outcome x offer count and position: 21 cells over every derived constructor, adopted against declined, success against failure-after-choice against failure-before-choice, and one offer against two with the adopted one first and last. Expectations are not read off the code -- Codec.Close is documented to release the codec's resources, and a codec the caller handed over has exactly one moment where that can happen, so "closed exactly once by the time the caller is done with the returned object" is the only state honoring the contract, and the count is asserted rather than a boolean so a double close fails too. The adopted cells are the control that keeps adopted and declined from being made to converge. Separate pins carry the three reported instances, the nil offer, and the uncomparable type. The set of constructors owing the rule is derived from source, not listed: a new guard asks go/ast which derived constructors mention optCodec and which call releaseUnadopted, and reds on any that takes offers without releasing them, so the next constructor added is caught by taking offers rather than by being remembered. It fails on an added member, and the row table fails on a removed one. Thirteen neuters, thirteen distinct red sets. Removing the whole release reds 21 cells and no control; releasing everything including the adopted codec reds 27, the controls and the pre-existing adopted-release suite among them, which is the two dispositions told apart in one observation. Each constructor's own sweep reds only its own cells plus the source guard. Mis-reporting the adopted index reds exactly the three cells where that index is not zero. Dropping the adopted pre-marking reds the offered-twice cells; never recognizing a repeat reds the offered-twice-declined ones; never recognizing an uncomparable repeat reds only the uncomparable pin. The last two neuters panic, which is the defect each guard exists to prevent: dropping the nil check calls Close on a nil interface, and using every codec as a map key panics on an uncomparable one.
…e is reached for
WithCodec takes an interface, and nil has two spellings in Go. WithCodec(nil)
stores a nil interface; a constructor with a concrete return type that yields nil
-- func newCodec() *myCodec, the ordinary typed-nil shape -- stores a non-nil
interface wrapping a nil pointer. c == nil sees only the first, so the second
reached a method call and died there.
Three constructors answered the question three different ways, and both halves of
the split were observable from a plain call:
- releaseUnadopted closed a DECLINED offer holding a typed nil. All three
constructors, on the SUCCESS return, so nothing signalled it. This half
arrived with the previous commit's release sweep; the same three probes pass
against its parent, so a call that worked before it crashed after.
- resolveCodec's name scan asked Name() of every offer, untyped nils included.
The scan is what DECIDES adoption, so it runs over offers about to be
declined -- NewReader and NewAppendWriter therefore died on the very shape
NewWriter carries a regression test pinning as working. That half is as old
as the OCF reader and is independent of the sweep.
The pin already in the tree says a superseded WithCodec(nil) "works and must keep
working", and attributes the reader-side scan crash to a nil that gets ADOPTED.
Executed, that attribution is wrong: adoption is decided BY the scan, so a nil
that could never be adopted crashed it just the same.
isNilCodec answers the question once, by asking reflect for Kind and IsNil rather
than comparing the interface -- the same way Schema.Decode already decides whether
its target is a usable pointer. A Codec may be any kind, so the nilable kinds are
enumerated rather than pointer being required.
Three sites consult it, not the two that were reported. NewWriter's adoption is
the third, and skipping it would not have been a smaller fix but a differently
placed bug: with the reader side skipping nils and the writer still taking the
last offer whatever it is, WithCodec(nil) would be quietly ignored by NewReader
and fatal to NewWriter -- the same asymmetry, moved rather than closed. Last
NON-nil wins there now, and an all-nil offer set leaves the writer on nullCodec,
which is what "behaves as though it were not written" has to mean if it means
anything.
WithCodec's contract states the rule in both spellings, including the one arm
where a nil still changes the outcome: it is the only offer, the file names a
codec no built-in provides, and the constructor reports an unknown codec rather
than crashing or silently reading nothing.
The net crosses spelling x constructor x offer layout x reader-side adoption. The
constructor axis is the set the existing go/ast derivation yields, cross-checked
at the end, so a constructor added later is driven by being codec-owning rather
than by being remembered. Expectations come from WithCodec's own documented "as
though it were not written" and from Codec.Close's release-exactly-once contract,
with the real codec's close count as the control -- a fix that ignored every
codec rather than the nil ones fails there.
Severity here depends on the CALLER's code, which a crash-only matrix cannot see:
a Close with a pointer receiver that touches a field segfaults, while a nil-safe
Close returns cleanly and the wrong call leaves no trace. So the nil-safe rows
COUNT the Closes the library had no business making, and the cells recover panics
into ordinary failures -- without that, every neuter below would produce the same
dead test binary instead of a red set naming its own mechanism.
Five neuters, five distinct red sets, none of them a panic. Dropping the
predicate's typed-nil arm and reverting the release loop to == nil both red 75
cells but not the same 75: the first also loses the map-kind rows, the second the
nil-safe ones. Removing the scan's skip reds 34, removing NewWriter's filter 20.
The release pre-marking's nil test neuters GREEN, which is measured rather than
assumed: both choosers now refuse to adopt a nil, so nothing can hand it one.
Neuter both choosers so a nil CAN be adopted and it reds exactly the two cells
where an uncomparable nil is adopted alongside a real codec of that same type --
recording the nil puts its TYPE in the repeat list and the real codec is then
never closed. That combination is written at the site, and is why the line stays.
The question is registered as census Q24. The source guard derives its sites by
range-or-index over a []Codec: index because NewWriter's is an index loop, and a
range-only walk would have reported full coverage of the exact class it exists to
catch. It fails on an added site and on a removed one, both attacked.
Two independent costs, measured from the last full green run (29555886213):
go test -race ./... 2m04s
Fuzz 7m36s
differential (parallel) 37s
java-differential 51s
Fuzzing was 78% of the job. It is 39 targets x -fuzztime=10s run one after
another: a 390s serial floor plus ~1.7s of per-target process overhead. The
seed corpus is 5 files, so it was not replay -- it was the flat budget, 39
times, in a row.
That budget bought nothing a pull request needs. `go test` WITHOUT -fuzz
already runs every target's f.Add seeds and every recorded crasher under
testdata/fuzz/<Fuzz>/ as ordinary subtests, so the -race line above carries
the full regression value of every input fuzzing has ever found. What -fuzz
adds is DISCOVERY of new inputs, and ~10s per target on a cold corpus
discovers approximately nothing. Move it to fuzz.yml on a nightly schedule
with a 120s default budget, sharded four ways for wall clock, where it can
actually find something. A finding there lands as a normal change with the
crasher committed to testdata/fuzz/, which the pull-request suite then
replays forever after.
The shard a job runs comes from strategy.job-index / strategy.job-total, so
adding a Fuzz function needs no edit to the workflow. Dispatch over the real
39 targets was verified to be a partition: 10/10/10/9, 39 dispatched, 39
unique. An empty target list is a hard error rather than a silent green,
and crashers upload as an artifact so the input survives the runner.
Second cost: `push` fired on every branch while the concurrency group keys
on github.ref, which is refs/heads/<branch> for a push and
refs/pull/<n>/merge for a pull_request. Different groups, so neither
cancelled the other and every push to a branch with an open PR ran the
whole pipeline TWICE on the same commit -- confirmed on 7a83530, runs
30966631352 and 30966661698. Restrict push to main; pull_request already
covers branch work.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is now past the point of finding anything important, but it is interesting what minor things are being found.