fix: GetAsciiStringAt Python parity (predicate, bounds, maxLength, requireCstring)#29
Merged
Merged
Conversation
…quireCstring) NavigationExtensions.GetAsciiStringAt deviated from Python BinaryView.get_ascii_string_at (binaryview.py:8023) in four ways: 1. Wrong byte predicate. C# used printable-only 0x20..0x7e; Python uses 0 < c <= 0x7f (any non-NUL ASCII byte, including control characters such as tab 0x09 and newline 0x0a). Printable-only truncated runs containing control characters. 2. Missing address bounds check. Python returns None when addr < start or addr >= end; C# read unconditionally. 3. Missing maxLength parameter. Python caps the run at max_length; C# hardcoded a 4096 window. 4. Missing requireCstring parameter. Python requires a NUL terminator by default (require_cstring=True); C# never checked termination. Rewritten to mirror Python exactly via BinaryReader (Position + ReadByte), the same lazy byte loop Python uses (BinaryReader.seek/read8). Signature gains maxLength/requireCstring optional params with Python-matching defaults, so existing callers are unaffected; behavior changes toward parity.
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.
Problem
NavigationExtensions.GetAsciiStringAtdeviated from PythonBinaryView.get_ascii_string_at(binaryview.py:8023) in four ways:0 < c <= 0x7f(any non-NUL ASCII)0x20..0x7e(printable-only)addr < start or addr >= end → Nonemax_lengthcap paramrequire_cstring=True(NUL terminator)Fix
Rewritten to mirror Python exactly, using
BinaryReader(Position+ReadByte) — the same lazy byte loop Python uses (BinaryReader.seek/read8). Gains optionalmaxLength/requireCstringparams with Python-matching defaults; existing callers unaffected, behavior changes toward parity.Verification
dotnet build -c Releaseclean.ViewProbe.FindFirstAsciiCString) to the corrected spec (predicate0<c<=0x7f+ NUL terminator), plus 2 new tests (out-of-bounds → null; maxLength cap). 44/44 harness tests pass.Surfaced while auditing the navigation layer for Python parity (continues the 1:1 parity directive; IL operand-count cluster was PRs #26/#27/#28).