Repository navigation
Fix offset rewrite collision in raw VTU parsing - #1574
Open
barlowa124 wants to merge 1 commit into
Open
barlowa124 wants to merge 1 commit into
barlowa124 wants to merge 1 commit into
Conversation
_parse_raw_binary rewrote DataArray offsets while looking each block's tag up by that offset. A rewritten offset could equal a later array's original offset, leaving that array un-rewritten and pointing into the middle of the base64 stream; reads then silently returned wrong data or failed with binascii errors. Snapshot the appended offsets first. Also: enforce encoding="raw", stop truncating the payload on a trailing newline byte, find the closing tag from the end, and turn bare asserts into ReadError. Fixes nschloe#1571
This branch has not been deployed
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.
Summary
Fixes #1571.
The file linked in the issue reads correctly on 5.3.5, so the reporter's
binascii.Error: Incorrect paddingcomes from a raw VTU with a differentblock layout. I found the mechanism that produces exactly this failure
class.
_parse_raw_binaryrewrites eachformat="appended"DataArray'soffsetto its position in the re-encoded base64 stream, then looks the next
block's tag up with
root.find(".//DataArray[@offset='{i}']"). When ajust-rewritten offset happens to equal a later array's original offset,
the find matches the wrong element again, bumping that array's offset a
second time while the later array is never rewritten at all. Its offset
then points at an arbitrary position in the base64 stream, so reads either
return the wrong array's data or hit
Incorrect padding.This snapshots the appended offsets into a map before rewriting anything,
so every array is located by its original offset. A small constructed file
where the connectivity block's new offset collides with the offsets
block's original offset reads back with connectivity and offsets swapped
before this change, correctly after.
Hardening in the same function, all matching its existing error contract.
Non-
rawencodings now raiseReadErrorinstead of being parsed as rawbinary, matching the reporter's expectation for every encoding other than
raw. The payload no longer loses its final byte when the binary happensto end in
0x0a, because the block loop consumes data by declared sizeand skips only trailing whitespace.
rfindreplacesfindfor theclosing tag so a payload containing that byte sequence can't truncate the
segment. Missing delimiters and a missing AppendedData element now raise
ReadErrorinstead ofIndexErrororAssertionError, and the remainingasserts became real checks sopython -Ocan't strip them.New tests cover the offset collision and the encoding check.
pytest tests/test_vtu.pygives 114 passed, including the three existingraw-binary fixture reads.
AI-assisted (Devin).