Skip to content

Commit 24fdb9b

Browse files
chaokunyangRelease Validation Test
andauthored
chore: clean unnecessary buffer checks (#3988)
## Why? ## What does this PR do? ## Related issues #3987 ## AI Contribution Checklist - [ ] Substantial AI assistance was used in this PR: `yes` / `no` - [ ] If `yes`, I included a completed [AI Contribution Checklist](https://github.com/apache/fory/blob/main/AI_POLICY.md#9-contributor-checklist-for-ai-assisted-prs) in this PR description and the required `AI Usage Disclosure`. - [ ] If `yes`, my PR description includes the required `ai_review` summary and screenshot evidence or equivalent persisted links of the final clean AI review results from both fresh reviewers described in `AI_POLICY.md`, the Fory-guided reviewer and the independent general reviewer, on the current PR diff or current HEAD after the latest code changes. ## Does this PR introduce any user-facing change? - [ ] Does this PR introduce any public API change? - [ ] Does this PR introduce any binary protocol compatibility change? ## Benchmark --------- Co-authored-by: Release Validation Test <dev@fory.apache.org>
1 parent 74b7134 commit 24fdb9b

24 files changed

Lines changed: 824 additions & 909 deletions

File tree

‎.agents/languages/cpp.md‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,11 @@ Load this file when changing `cpp/`, Cython build plumbing, or C++ xlang behavio
1313
- When invoking a method that returns `Result`, use `FORY_TRY` unless you are in control-flow logic that cannot use it cleanly.
1414
- Wrap error checks with `FORY_PREDICT_FALSE` for branch prediction.
1515
- Continue on trivial errors; return early only for critical errors such as buffer overflow.
16-
- `ReadContext` and related codec owners intentionally accumulate non-critical
17-
errors and inspect them at established serializer or root safepoints. Bounds-safe
18-
work may continue after an error is recorded. Do not add eager per-field checks,
19-
cursor rollback, or tests that pin the first detection point unless deferral can
20-
cause out-of-bounds access, undefined behavior, resource amplification, state
21-
pollution, or success past the required safepoint.
16+
- `ReadContext` intentionally records codec errors for inspection at existing serializer or root
17+
safepoints. After an error, work may continue only while it remains bounds-safe and cannot cause
18+
resource amplification, publish reference or cache state that survives root cleanup, or return
19+
success past the required safepoint. Do not add per-field checks, cursor rollback, or tests that
20+
pin the first detection point solely to make an error earlier or more precise.
2221
- Put private methods last in class definitions, immediately before private fields.
2322
- Do not redesign alias-based or low-level public type shapes to add convenience methods unless the user explicitly asks for that API change.
2423
- For cross-language feature ports, match protocol behavior but use idiomatic C++ ownership and layering instead of mirroring Java structure literally.

‎.agents/languages/go.md‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,11 @@ Load this file when changing `go/fory/` or Go xlang behavior.
77
- Run Go commands from within `go/fory/`.
88
- Changes under `go/` must pass formatting and tests.
99
- The Go implementation focuses on fast serializers.
10-
- Go codec paths may intentionally record an error and continue bounds-safe work
11-
until an established serializer or root safepoint checks it. Do not add eager
12-
per-field or per-element error branches, cursor rollback, or tests that pin the
13-
first detection point unless deferral can cause a panic, out-of-bounds access,
14-
disproportionate work or allocation, persistent state pollution, or success
15-
past the required safepoint.
10+
- Go `ReadContext` intentionally defers codec errors to existing `HasError` or `CheckError`
11+
boundaries. After an error, work may continue only while it remains panic- and bounds-safe and
12+
cannot cause disproportionate work or allocation, publish state that survives root cleanup, or
13+
return success past the required boundary. Do not add per-field or per-element checks, cursor
14+
rollback, or tests that pin the first detection point solely to change error timing or precision.
1615
- Root deserialization graph memory budget state belongs to `ReadContext`.
1716
`WithMaxGraphMemoryBytes` uses a fixed `128 MiB` default; positive explicit
1817
values override it, and explicit non-positive values are invalid at config creation.

‎.agents/languages/java.md‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -159,22 +159,27 @@ Load this file when changing anything under `java/` or when Java drives a cross-
159159
- In `MemoryBuffer` and `MemoryOps` hot paths, duplicate small straight-line copy/read/write logic
160160
when that keeps control flow direct. Do not add private helper indirection to hot paths just to
161161
reduce local code duplication; keep helpers for slow, cold, or error paths.
162+
- `MemoryBuffer` semantic primitive-array `write*` and `read*` methods own the canonical
163+
little-endian element order. Keep native bulk copy as the little-endian hot path and isolate
164+
big-endian conversion in separate slow helpers; serializers must not duplicate that endian
165+
branch. The explicit `copyTo*Array` and `copyFrom*Array` methods remain raw native-memory copies,
166+
so their format owner must handle byte order when required.
162167
- Add a Java `MemoryBuffer` check only when its absence can cause a JVM or native crash, OOM, or
163168
attacker-controlled memory amplification. Delayed, masked, less precise, or differently typed
164169
failures do not justify a check, and neither does an incorrect decoded result without one of
165-
those crash or memory consequences. Do not duplicate an array, `ByteBuffer`, VarHandle, stream,
170+
those crash or memory consequences. Do not duplicate an array, `ByteBuffer`, `VarHandle`, stream,
166171
or other existing bounds owner merely to move or normalize an error.
167-
- The JDK 25 `MemoryBuffer` overlay is intentionally unchecked at its logical buffer boundary.
168-
Indexed array accesses, absolute `ByteBuffer` accesses, and VarHandle accesses own bounds
169-
enforcement and already provide a controlled failure; its exact exception type, message, and
170-
detection point are not contracts. Do not copy the JDK 8-24 Unsafe-path range checks into this
171-
overlay. Keep an explicit check only before allocation, capacity growth, or another side effect
172-
when it is required to prevent a crash, OOM, or attacker-controlled memory amplification that the
173-
access owner cannot contain.
174-
- `MemoryAllocator.grow` owns the postcondition that a successful return has supplied at least the
175-
requested capacity. `MemoryBuffer` must not recheck that postcondition after calling `grow` or
176-
`ensure`; an allocator that returns without satisfying it violates the allocator contract and
177-
must be fixed in the allocator implementation rather than burdening every buffer hot path.
172+
- The JDK 25 `MemoryBuffer` overlay intentionally does not duplicate logical range checks around
173+
indexed array, absolute `ByteBuffer`, or `VarHandle` access. Those JVM accessors own physical
174+
bounds enforcement and already provide a controlled failure; the exact exception type, message,
175+
and detection point are not contracts. Do not copy JDK 8-24 Unsafe-path checks into the overlay
176+
solely to make invalid access fail earlier or more precisely. Keep an explicit check before
177+
allocation, capacity growth, or another side effect when it is needed to prevent a crash, OOM, or
178+
attacker-controlled memory amplification that the access owner cannot contain.
179+
- `MemoryAllocator.grow` owns the postcondition that a successful return leaves the buffer capacity
180+
at least the requested capacity. Callers must reject invalid or overflowed requests before the
181+
call, but must not recheck the allocator postcondition afterward; fix a violating allocator at
182+
the allocator implementation.
178183
- In JDK 25 Fory JSON C2-sensitive code, preserve measured, naturally large hot-method boundaries.
179184
A method that exceeds HotSpot's 325-byte hot-inline limit through real representation, scalar,
180185
array, collection, or generated-schema work is an independent subtree owner. Generated group

‎AGENTS.md‎

Lines changed: 15 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,13 @@ This is the entry point for AI guidance in Apache Fory. Read this file first, th
3232
- Preserve architecture. Do not introduce new layers, parallel flows, or public APIs unless explicitly requested; prefer local repair in the existing owner over shared-infra expansion, and stop if a fix conflicts with an ADR, spec, or invariant.
3333
- Do not change an existing `RefReader`/`RefWriter` architecture or API to support compatible skip. Compatible skip must not add alternate reference slots or tables, alternate reference lookup or publication methods, or forwarding APIs in read/write contexts, builders, serializers, or generated-code plumbing. Keep ordinary reference publication and lookup unchanged and resolve the case in the existing compatible generated owner. For an authorized removed-field read of an unregistered Struct, the empty object created by the skip reader is that path's final owner: publish that same object for `RefValue`, consume the Struct fields, and let later `RefFlag` values resolve to it. This preserves reference numbering and identity without registering the Struct; an independent dynamic root still requires normal registration. Do not add parallel reference state, a sentinel, a rejection, or a common-path branch for this case.
3434
- Respect ownership. Keep logic, state, and helpers in their natural owner, and do not move serializer-local, context-local, runtime-type-local, or protocol-local problems into global utilities.
35+
- Low-level APIs explicitly named `unsafe` or `unchecked` may be public and may omit local bounds or
36+
capacity checks. Their callers own the proof required by the operation. Do not hide these APIs
37+
behind private access, checked forwarding wrappers, friend adapters, or duplicate implementations
38+
solely to prevent misuse; add validation only at the caller or owner that lacks the required proof.
39+
- Keep hardening changes causal. A shared wire defect may require aligned runtime fixes, but it does
40+
not justify unrelated buffer checks, performance rewrites, or cleanup without their own concrete
41+
consequence and evidence.
3542
- Check the spec before implementation. For wire behavior and xlang mapping, use the specs as the source of truth and never copy one runtime's bug into another runtime just to make tests pass.
3643
- `foryc` is a build-time compiler for trusted schema inputs and is never invoked by runtime serialization or deserialization. Schema provenance, package/namespace options, output-path options, and generated-source review belong to the application or build owner. Do not classify hostile-schema source injection or path traversal as a Fory runtime security vulnerability; `foryc` does not promise to sandbox untrusted schemas.
3744
- Row format accepts only trusted input and is outside Fory's untrusted binary-deserialization security boundary. Rust `check_string_read(false)` is likewise an explicit trusted-input mode that disables UTF-8 validation; its caller owns the validity guarantee. Classify issues in those paths as correctness, soundness, or hardening bugs when applicable, not as attacker-controlled deserialization vulnerabilities under the default security model.
@@ -40,25 +47,13 @@ This is the entry point for AI guidance in Apache Fory. Read this file first, th
4047
- Malformed input must surface as a controlled root-operation error and still run
4148
root cleanup, but the exact exception type, error code, message, detection
4249
layer, and detection point are not contracts unless a public API or
43-
specification explicitly says otherwise. An existing bounded downstream
44-
buffer, type, reference, depth, or serializer error is sufficient. Do not add
50+
specification explicitly says otherwise. Differences only in error type,
51+
message, layer, offset, or detection point are not security findings. An
52+
existing bounded downstream buffer, type, reference, depth, or serializer
53+
error is sufficient. Do not add
4554
hot-path branches, helper APIs, allocations, or generated-code expansion
4655
solely to make an error earlier, more specific, or more uniform, and do not
4756
write tests that force such error normalization.
48-
- Non-strict, non-precise, delayed, masked, differently typed, or differently layered controlled
49-
errors are not security findings. Security remediation must not add checks solely to normalize,
50-
sharpen, advance, or otherwise make such errors deterministic. Security tests must not require a
51-
particular error type, message, layer, offset, or detection point unless an explicit public
52-
contract makes that precision part of the policy boundary.
53-
- Runtimes with an established lazy-error accumulator, including C++ and Go,
54-
may keep executing bounds-safe codec work after recording an error and inspect
55-
it at an existing serializer or root-operation safepoint. Deferred inspection
56-
is an intentional hot-path design, not by itself a correctness or security
57-
defect. Do not add per-field or per-element error branches, cursor rollback,
58-
or tests that require immediate propagation solely because an earlier check is
59-
possible. Treat the behavior as a defect only when the deferred path has a
60-
concrete consequence listed below or an established safepoint can return
61-
success instead of a controlled root error.
6257
- Never add a reader-side check solely to produce a more precise malformed-input
6358
error. Retain or add a check only when the unchecked path has a concrete
6459
consequence such as a crash, panic, undefined behavior, out-of-bounds access,
@@ -69,15 +64,6 @@ This is the entry point for AI guidance in Apache Fory. Read this file first, th
6964
helper when the language supports it. A bounds-safe downstream operation that
7065
already raises a controlled root error is sufficient; do not duplicate it for
7166
error precision.
72-
- Buffer and memory-buffer checks require a stricter consequence test. Security remediation may add
73-
a check only when its absence can cause a process crash or panic, undefined memory access, OOM, or
74-
attacker-controlled memory amplification. Delayed, masked, less precise, or differently typed
75-
errors are not sufficient reasons. Neither is an incorrect decoded result when the unchecked
76-
path cannot cause one of those crash or memory consequences. A downstream bounds owner that
77-
already fails in a controlled way is sufficient; do not duplicate its check in a hotter wrapper.
78-
Apply this admission rule prospectively: do not remove an already completed and validated check
79-
solely by reclassifying it when it has no demonstrated performance cost. Preserve that check as
80-
a correctness fix unless an explicit task requires the behavioral rollback.
8167
- Before reporting or fixing a robustness finding, prove that the current path
8268
causes at least one concrete consequence: crash, panic, undefined behavior,
8369
or out-of-bounds access; disproportionate allocation, CPU work, or stream
@@ -88,16 +74,6 @@ This is the entry point for AI guidance in Apache Fory. Read this file first, th
8874
malformed or noncanonical flag, enum value, marker, length form, or reserved
8975
value is accepted, rejected late, decoded differently, or produces a less
9076
precise error.
91-
- Keep security remediation scope frozen. Do not fix a standalone non-security
92-
correctness, interoperability, API, or lifecycle issue merely because it is
93-
discovered during security work. Include an adjacent non-security change only
94-
when omitting it would leave the security fix incomplete or force the same
95-
owner into an ugly, duplicated, or knowingly unsound design; otherwise record
96-
the issue separately and leave production code unchanged. This rule prevents
97-
new scope; it does not authorize rolling back a non-security fix that is
98-
already implemented and validated. Preserve such fixes. If one may affect
99-
performance, inspect and measure the affected path first, then optimize it on
100-
evidence instead of removing it by classification.
10177
- Arbitrary-precision binary Decimal codecs accept only scales in
10278
`[-10_000, 10_000]` and an absolute unscaled magnitude of at most `10_000`
10379
binary bytes. The Java standalone `BigInteger` serializer uses the same
@@ -226,6 +202,10 @@ This is the entry point for AI guidance in Apache Fory. Read this file first, th
226202
- Do not make design tradeoffs the user did not request. If a refactor appears to require a behavior, logic, protocol, or performance tradeoff, stop and ask.
227203
- Treat existing low-level or optimized code as deliberate by default. During a refactor, preserve the current implementation strategy unless the user explicitly asks to redesign or optimize it.
228204
- Do not replace existing C, C++, Cython, unsafe, or other low-level optimized paths with simpler high-level implementations just to make a refactor easier.
205+
- When removing a redundant wrapper or helper, preserve any aggregate capacity proof, fused
206+
operation, reserved wide store, specialized overload, or unchecked primitive path in the natural
207+
owner. Do not route that work through a generic checked path unless matched benchmarks justify the
208+
implementation change.
229209
- If a refactor accidentally changes logic or implementation strategy, revert that part and re-implement the refactor around the existing logic.
230210
- Use English only in code, comments, and documentation.
231211
- Do not use emoji in documentation, including headings, feature lists, status

‎cpp/fory/serialization/array_serializer.h‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,8 @@ struct Serializer<
9393
buffer.grow(static_cast<uint32_t>(max_size));
9494
uint32_t writer_index = buffer.writer_index();
9595
// write array length in bytes
96-
writer_index += buffer.put_var_uint32(writer_index,
97-
static_cast<uint32_t>(N * sizeof(T)));
96+
writer_index += buffer.put_var_uint32_unchecked(
97+
writer_index, static_cast<uint32_t>(N * sizeof(T)));
9898

9999
// write data
100100
if constexpr (N > 0) {
@@ -110,7 +110,7 @@ struct Serializer<
110110
}
111111
}
112112
}
113-
buffer.writer_index(writer_index + N * sizeof(T));
113+
buffer.unsafe_set_writer_index(writer_index + N * sizeof(T));
114114
}
115115

116116
static inline void write_data_generic(const std::array<T, N> &arr,
@@ -221,14 +221,14 @@ template <size_t N> struct Serializer<std::array<bool, N>> {
221221
uint32_t writer_index = buffer.writer_index();
222222
// write array length
223223
writer_index +=
224-
buffer.put_var_uint32(writer_index, static_cast<uint32_t>(N));
224+
buffer.put_var_uint32_unchecked(writer_index, static_cast<uint32_t>(N));
225225

226226
// write each boolean as a byte
227227
for (size_t i = 0; i < N; ++i) {
228228
buffer.unsafe_put_byte(writer_index + i,
229229
static_cast<uint8_t>(arr[i] ? 1 : 0));
230230
}
231-
buffer.writer_index(writer_index + N);
231+
buffer.unsafe_set_writer_index(writer_index + N);
232232
}
233233

234234
static inline void write_data_generic(const std::array<bool, N> &arr,
@@ -323,7 +323,7 @@ template <size_t N> struct Serializer<std::array<float16_t, N>> {
323323
constexpr size_t max_size = 8 + N * sizeof(float16_t);
324324
buffer.grow(static_cast<uint32_t>(max_size));
325325
uint32_t writer_index = buffer.writer_index();
326-
writer_index += buffer.put_var_uint32(
326+
writer_index += buffer.put_var_uint32_unchecked(
327327
writer_index, static_cast<uint32_t>(N * sizeof(float16_t)));
328328
if constexpr (N > 0) {
329329
if constexpr (FORY_LITTLE_ENDIAN) {
@@ -336,7 +336,7 @@ template <size_t N> struct Serializer<std::array<float16_t, N>> {
336336
}
337337
}
338338
}
339-
buffer.writer_index(writer_index + N * sizeof(float16_t));
339+
buffer.unsafe_set_writer_index(writer_index + N * sizeof(float16_t));
340340
}
341341

342342
static inline void write_data_generic(const std::array<float16_t, N> &arr,
@@ -436,7 +436,7 @@ template <size_t N> struct Serializer<std::array<bfloat16_t, N>> {
436436
constexpr size_t max_size = 8 + N * sizeof(bfloat16_t);
437437
buffer.grow(static_cast<uint32_t>(max_size));
438438
uint32_t writer_index = buffer.writer_index();
439-
writer_index += buffer.put_var_uint32(
439+
writer_index += buffer.put_var_uint32_unchecked(
440440
writer_index, static_cast<uint32_t>(N * sizeof(bfloat16_t)));
441441
if constexpr (N > 0) {
442442
if constexpr (FORY_LITTLE_ENDIAN) {
@@ -449,7 +449,7 @@ template <size_t N> struct Serializer<std::array<bfloat16_t, N>> {
449449
}
450450
}
451451
}
452-
buffer.writer_index(writer_index + N * sizeof(bfloat16_t));
452+
buffer.unsafe_set_writer_index(writer_index + N * sizeof(bfloat16_t));
453453
}
454454

455455
static inline void write_data_generic(const std::array<bfloat16_t, N> &arr,

0 commit comments

Comments
 (0)