Improve binary processing. Improve native CRC performance. Add benchmarks. - #3719
Open
ryanmelt wants to merge 4 commits into
Open
Improve binary processing. Improve native CRC performance. Add benchmarks.#3719ryanmelt wants to merge 4 commits into
ryanmelt wants to merge 4 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3719 +/- ##
==========================================
+ Coverage 79.30% 79.32% +0.01%
==========================================
Files 885 885
Lines 65365 65447 +82
Branches 2543 2591 +48
==========================================
+ Hits 51838 51914 +76
- Misses 12859 12860 +1
- Partials 668 673 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Reads of items outside an undersized buffer return nil again instead of rejecting the packet, and Packet skips conversions, states and format strings for those nil values. Variable bit sizes larger than the buffer are only an error when the packet does not allow short buffers. Harden the remaining integer overflow and unbounded allocation paths: little endian bitfields must span bytes inside the buffer, negative bit size and negative bit offset recalculations are done in 64 bits, and writes beyond the native int range raise a buffer error rather than RangeError. Mirror all of it in Python and add ALLOW_SHORT test coverage for both languages. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
jmthomas
previously approved these changes
Aug 17, 2026
jmthomas
left a comment
jmthomas
left a comment
Member
There was a problem hiding this comment.
ALLOW_SHORT was broken. I fixed and then ran a fuzz test on the existing logic and found another 2 other issues:
- LE bitfield exception allowed unbounded bit_size and CPU/memory DoS
- Signed overflow in the negative-bit_size recalculation (C only).
Contributor
There was a problem hiding this comment.
Pull request overview
This PR strengthens OpenC3’s binary/packet processing correctness (especially around short/undersized buffers and integer-overflow edge cases), improves CRC performance in the pure-Ruby and Python implementations, and adds repeatable benchmarks to compare native vs pure implementations and Ruby vs Python.
Changes:
- Add benchmark harnesses for C-extension vs pure Ruby throughput, plus a Ruby (YJIT on/off) vs Python comparison with correctness signatures.
- Optimize CRC table generation and reflected CRC calculation paths (Ruby + Python), with expanded CRC correctness tests.
- Harden packet/structure/binary accessor handling for short buffers and extreme offsets/sizes; add broad regression tests in both Ruby and Python.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| openc3/test/benchmarks/ruby_python_benchmark.py | New Ruby (YJIT on/off) vs Python benchmark runner with per-case correctness signatures. |
| openc3/test/benchmarks/README.md | Documents running benchmarks and supported environment variables/filters. |
| openc3/test/benchmarks/c_extensions_benchmark.rb | New benchmark runner comparing C extensions vs pure Ruby with correctness verification and summary stats. |
| openc3/spec/utilities/crc_spec.rb | Adds Ruby spec coverage for Crc32#calc using default seed when seed is nil. |
| openc3/spec/packets/structure_spec.rb | Adds Ruby specs covering short-buffer and variable-size edge cases (including overflow-adjacent offsets). |
| openc3/spec/packets/packet_spec.rb | Adds Ruby specs ensuring conversions/formatting/states don’t apply to nil values for short buffers. |
| openc3/spec/accessors/binary_accessor_spec.rb | Adds Ruby specs for little-endian bitfield bounds and overflow scenarios. |
| openc3/python/test/utilities/test_crc.py | Expands Python CRC tests and adds an “old algorithm” oracle for table/implementation equivalence. |
| openc3/python/test/packets/test_structure.py | Adds Python tests mirroring Ruby short-buffer and variable-size behaviors. |
| openc3/python/test/packets/test_packet.py | Adds Python tests for short-buffer behavior in Packet.read/limits/identify. |
| openc3/python/test/accessors/test_binary_accessor_write.py | Adds Python tests for little-endian bitfield write bounds. |
| openc3/python/test/accessors/test_binary_accessor_read.py | Adds Python tests for little-endian bitfield read bounds and negative-size overflow edge cases. |
| openc3/python/openc3/utilities/crc.py | CRC init/calc optimizations (precomputed masks/shifts, reflected tables, faster reflected loop). |
| openc3/python/openc3/packets/structure.py | Validates variable bit sizes against available buffer bits; aligns error behavior with Ruby. |
| openc3/python/openc3/packets/packet.py | Avoids applying conversions/states/formatting when reading None from a short buffer. |
| openc3/python/openc3/accessors/binary_accessor.py | Validates variable bit sizes; prevents out-of-range/negative derived sizing from reaching low-level access; tightens bounds checks. |
| openc3/lib/openc3/utilities/crc.rb | Pure-Ruby CRC performance improvements via cached params and reflected-table fast path. |
| openc3/lib/openc3/packets/structure.rb | Adds buffer-aware validation for variable bit sizes (mirrors Python behavior). |
| openc3/lib/openc3/packets/packet.rb | Avoids applying conversions/states/formatting when reading nil from a short buffer. |
| openc3/lib/openc3/accessors/binary_accessor.rb | Adds variable-size validation, native-int range checks, and improved bounds checks for short buffers. |
| openc3/ext/openc3/ext/structure/structure.c | Hardens bounds/size calculations against integer overflow; improves safety checks before allocations. |
| openc3/ext/openc3/ext/platform/platform.c | Replaces function-scope const int with macro for fixed-size filename buffer. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Assert the last accepted and first rejected little endian bitfield spans with their values so an off-by-one in the pre-check fails loudly instead of silently reading nil. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
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
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.
No description provided.