Skip to content

CLAUDE.md

Latest commit

 

History

History
246 lines (214 loc) · 15.2 KB

CLAUDE.md

File metadata and controls

246 lines (214 loc) · 15.2 KB
Raw

HOT — read before reasoning about this repo

WHAT: the C reference for reliable — packet fragmentation, reassembly and acks over an unreliable transport. NOT reliable.rs / reliable.go (the ports).

DECISIONS THAT READ AS BUGS (they are not — do not "fix" them)

  • Release builds trust the caller on the hot paths. This is the design contract. Correct arguments to send, receive and copy are the programmer's responsibility in release; debug asserts catch mistakes during development and release carries no per-packet validation overhead. Do NOT add release-mode checks to those paths.
  • Endpoint creation is the exception and is fallible in every build. reliable_endpoint_create validates the whole config, checks the size arithmetic, checks every allocation, frees whatever it took, and returns NULL. Creation happens once, off the hot path, and the header and README both promise a NULL check, so the promise is kept rather than left to an assert release compiles out. reliable_endpoint_destroy checks each member rather than asserting it, because create hands it a partly built endpoint to unwind. Do not turn any of this back into asserts.
  • No authentication, no anti-spoofing, and that is out of scope. reliable assumes an authenticated encrypted transport beneath it — that is netcode's job. Forged packets, window-warping via a fake far-future sequence, spoofed acks: all netcode's to prevent. Do not add defenses here.
  • Fragmentation amplifies loss on purpose. One lost fragment loses the whole packet, and in-progress reassemblies evicted by newer traffic are the same trade. Latency comes first. Callers needing large reliable blocks build block transfer above this (yojimbo does exactly that) rather than sending very large packets.
  • No keepalives, because there are never lulls. reliable assumes continuous bidirectional exchange at ~60Hz; acks piggyback on outgoing packets. One-directional or bursty request/response traffic is out of scope, and the rtt/jitter/loss stats are fresh only under that same assumption.

THE WRITE/READ RULE — read this BEFORE reporting any assert as a missing bounds check Glenn, 2026-07-26: "intention is on write, user is responsible to not crash or do undefined behavior. asserts are there to help. callers responsibility. on read, obviously, we must check." Plus Postel: "be conservative in what you send, permissive in what you receive." WRITE / caller-supplied -> the CALLER validates. Assert-only is the DESIGN; -DNDEBUG removing it is correct. Do NOT add runtime checks here. READ / off the wire -> the library checks at runtime, for safety. DELIBERATELY ASSERT-ONLY, do NOT "fix" -- a future audit that finds these has found the contract, not a bug:

  • reliable_copy_string( dest, source, 0 ) (reliable.c:1729). dest_size is size_t, so i < dest_size - 1 wraps to SIZE_MAX and writes until it finds a NUL. I proved this under ASan (heap-buffer-overflow WRITE at reliable.c:1739). It is still caller responsibility.
  • reliable_endpoint_send_packet( ep, data, negative ) (reliable.c:762). The

    max_packet_size test at :768 is false for negatives, so it reaches the memcpy at :802 and the int becomes a huge size_t. Proved under ASan (negative-size-param). Note netcode's equivalent DOES range-check; that difference is not a defect here. THE RECEIVE PATH IS CLEAN AND UNUSUALLY WELL HARDENED -- two independent audits (Opus 5 and Fable 5) agree. read_fragment_header checks minimum length (:963), num_fragments > max_fragments (:982), fragment_id >= num_fragments (:988), fragment_bytes (:1038, :1044); reliable_store_fragment_data has an explicit pre-memcpy bounds test at :1089 with a comment naming the attacker case. WHY fragment_received[256] (reliable.c:472) CANNOT be overrun even though max_fragments is assert-only: fragment_id is read as a uint8_t at :979, so it is <= 255 by the WIRE TYPE regardless of any check or config value. Verified empirically, not just by reading -- a hostile-input harness under ASan+UBSan with -DNDEBUG hit that indexing site 24,384 times at a highest index of exactly 255.

CLAUDE.md

What this is

reliable is a single-file C library (reliable.c / reliable.h, ~2,600 lines including embedded tests) implementing packet acknowledgement, fragmentation/reassembly, and RTT/jitter/packet-loss/bandwidth estimation over UDP. It is transport-agnostic: the caller supplies transmit_packet_function / process_packet_function callbacks. The other files (test.cpp, example.c, soak.c, stats.c, fuzz.c) are thin harnesses around the library.

Build and test (CMake, 3.15+):

cmake -B build -DCMAKE_BUILD_TYPE=Debug        # or Release; on Windows: cmake -B build -A x64
cmake --build build                            # on Windows add: --config Debug
ctest --test-dir build --output-on-failure     # runs the test suite + bounded fuzz and soak runs

Binaries land in build/bin (build/bin/<Config> on Windows). Add -DRELIABLE_SANITIZE=ON for ASan+UBSan. CI (.github/workflows/ci.yml) runs Debug+Release on Windows x64, macOS arm64, and Ubuntu LTS, plus a sanitizer job, plus a weekly 2M-iteration fresh-seed fuzz job under ASan/UBSan (manually triggerable via workflow_dispatch).

Tests live at the bottom of reliable.c behind RELIABLE_ENABLE_TESTS, driven by test.cpp. Debug/release is selected by RELIABLE_DEBUG / RELIABLE_RELEASE; asserts compile out entirely in release.

Status (2026-07-09): premake was replaced by CMake and CI was added in July 2026 (commit a579740 onward). CI is green across the full matrix, there are no open issues, and the working tree matches main.

Honest assessment

Overall

This is mature, production-quality code written by someone who knows exactly what they are doing. The design is small, focused, and allocation-disciplined; the wire format is compact and well thought out (variable-length ack encoding via prefix byte); the sequence-buffer data structure is the right tool and is implemented correctly, including 16-bit wraparound. The recent security-audit pass shows: header parsing bounds-checks every read, fragment reassembly validates sizes before copying (reliable_store_fragment_data, reliable.c:1071-1082), and the fuzzer exercises the real send→fragment→corrupt→reassemble path rather than just throwing random bytes at receive. I compiled with -Wall -Wextra (clean), ran the full test suite (passes), and ran 20k fuzz iterations under ASan+UBSan (clean).

The findings from that 2026-07 review are recorded below; everything actionable was fixed at the time (see "Found and fixed"). What remains is the design contract and the gotchas — read those before changing anything.

Design contract: release builds trust the caller on the hot paths (do not "fix" this)

Correct arguments are the programmer's responsibility in release. Debug asserts exist to help the programmer catch mistakes during development; release builds deliberately carry no per-packet validation overhead. Do not add release-mode checks to the send, receive or copy paths.

Endpoint creation is the one place that validates in every build. It runs once, off the hot path, and both the header and the README promise the caller a NULL check, so reliable_endpoint_create validates the config (every field range plus the two relationships, fragment_above against max_packet_size and max_fragments * fragment_size covering max_packet_size), checks the size arithmetic behind every buffer, checks every allocation result, frees what it already took and returns NULL. The caller supplies the allocator, and an allocator that returns NULL is a supported outcome rather than undefined behavior.

Open issues

None currently.

Found and fixed (2026-07, alongside the CMake migration)

  • No CI — the workflow had been deleted; .github/workflows/ci.yml now runs Debug+Release build + test suite + bounded fuzz on Windows x64 / macOS arm64 / Ubuntu, plus an ASan+UBSan job on Ubuntu.
  • Copy-paste bug in a test — test_acks_packet_loss fetched the sender's acks twice and never checked the receiver's; reliable.c:2155 now checks context.receiver.
  • README sample code bugs — the ack loop indexed acks[j] with loop variable i, and the stats printf was missing its opening quote.
  • Six allocation results lacked the debug assert that the rest of the code applies (sequence buffer struct, endpoint acks + rtt history buffers, both send-path scratch buffers, and the fragment reassembly buffer). All now have reliable_assert, keeping release behavior untouched per the design contract.
  • Dated endianness detection — unlisted architectures (e.g. RISC-V) fell through to RELIABLE_BIG_ENDIAN. Detection now prefers the compiler's __BYTE_ORDER__ macro (GCC/Clang), keeping the old architecture list as the MSVC fallback. Runtime-verified by test_endian.
  • Fragment 0 payload could be shifted by a non-canonical packet header — reassembly re-encodes the header canonically, so a non-canonically encoded header (e.g. an 0xFF ack_bits block included explicitly) made the re-encoded size differ from the received size, shifting where fragment 0's payload landed. No memory-safety impact (bounds check from f0e3be1), but such packets reassembled corrupted. reliable_read_fragment_header now rejects fragments whose packet header is not byte-identical to the canonical encoding — the library's own sender is always canonical, so only forged/corrupt packets are affected.
  • Duplicate packets within the receive window were delivered twice — per the maintainer (2026-07-09) this was an oversight, not design intent. reliable_endpoint_receive_packet now drops sequences already present in the received buffer and counts them in RELIABLE_ENDPOINT_COUNTER_NUM_PACKETS_DUPLICATE. Fragments for already-received sequences are dropped on arrival too, so a replayed fragment set costs nothing and a duplicated final fragment cannot spawn a zombie reassembly entry. Retransmits of packets the caller rejected (process function returned 0) are still processed, because only accepted packets enter the received buffer. Covered by test_duplicate_packets.

Scope and operating assumptions (per the maintainer, 2026-07-09 — not issues)

  • Authentication and packet trust are out of scope. reliable assumes an authenticated, encrypted transport beneath it — that is netcode's job (its sister library). Attacks that require forging packets (warping the receive window via a fake far-future sequence, spoofing acks, injecting payloads) are netcode's problem to prevent, not reliable's. Do not add authentication or spoofing defenses here.
  • Fragmentation trades loss amplification for latency, by design. If any fragment is lost the whole packet is lost; in-progress reassemblies evicted by newer traffic are the same trade. Time-sensitive delivery comes first. Callers who need large blocks delivered reliably should implement block transfer at a higher level (yojimbo does this on top of reliable) rather than routinely sending very large packets.
  • Continuous bidirectional packet exchange is assumed. reliable is designed for protocols like action games that send packets both ways at ~60Hz, continually. Acks piggyback on outgoing packets, so one-directional or bursty request/response traffic is out of scope — no keepalives exist because there are never lulls. Stats (rtt/jitter/loss over the recent history window) are fresh under the same assumption.
  • 16-bit sequence numbers are sized for this send rate. At game-style packet rates the wrap interval is ample; bulk-transfer rates are out of scope.

Improvements (2026-07-09, v1.3.0)

  • The send path no longer allocates. Each endpoint owns one persistent transmit scratch buffer (allocated at create, sized for the larger of a regular packet or a fragment), replacing the per-send allocate/free pair. Costs ~max_packet_size bytes per endpoint. New caller contract: the transmit packet callback must not send packets on the same endpoint (it would clobber the scratch buffer mid-send) — documented in the header. Sending on a different endpoint (e.g. loopback tests) remains fine.
  • The public API is documented in reliable.h — every function and config field.
  • Version defines added (RELIABLE_VERSION_*, 1.3.0), continuing the existing v1.2.7 tag lineage. Minor bump because behavior changed: duplicates dropped, counter count now 11.
  • New tests: test_stale_packets (replay outside window rejected), test_ack_buffer_overflow (drop + recovery-after-clear semantics), test_endpoint_reset (state cleared, works after reset, in-progress reassembly freed without double-free, via tracking allocator).
  • Install support for packaging (v1.3.1) — cmake --install installs the library (static by default, -DBUILD_SHARED_LIBS=ON for shared), reliable.h, and a pkg-config file. The installed library no longer embeds the test suite: the test binary compiles its own copy of reliable.c with RELIABLE_ENABLE_TESTS=1. Added for the homebrew formula.
  • OSS-Fuzz kit added — fuzz_target.c is a libFuzzer harness (fuzz input = a script of send/inject operations against a live endpoint pair, exact-size heap copies for redzone checking); oss-fuzz/ holds the project.yaml/Dockerfile/build.sh ready to submit to google/oss-fuzz (see oss-fuzz/README.md — submission requires the maintainer to open the PR). A standalone driver build of the same harness runs as the fuzz_target ctest so it cannot bitrot.

Design notes (intentional; documented in the README "Caveats" section since 2026-07)

  • The transmit packet callback must not send packets on the same endpoint — it is called synchronously while the endpoint's transmit scratch buffer is in use (since v1.3.0). Sending from the process packet callback is fine (that endpoint is not mid-send), as is sending on other endpoints.
  • Acks are dropped once the ack buffer fills — the caller must call reliable_endpoint_clear_acks regularly. Since 2026-07 a drop logs at error level instead of being silent; the unacked packet can still be reported on a later packet's ack bits while it remains within the 32-packet ack window.
  • Not thread-safe. Endpoints have no locking, and log level / printf / assert handlers are process-wide globals. One-endpoint-per-thread or external locking is required.

Style notes

  • The code is consistently C89-flavored C99 with the author's idiosyncratic spacing. Match it when editing; don't "modernize."
  • Error handling philosophy is assert-in-debug, trust-the-caller-in-release. This is the maintainer's explicit design contract (see above), not an accident.
  • Test coverage of the happy paths is good; adversarial coverage lives in fuzz.c, which is genuinely well constructed (loss, reorder, duplication, bit corruption, random injection over a simulated link).

Bottom line

A tight, battle-tested library that does one thing well, with real fuzzing and a recent security pass behind it. The code earns its "production ready" claim. Release builds trusting the caller on configuration and allocation is the maintainer's explicit design contract — respect it. No open issues as of 2026-07.