Validate list and element counts; skip empty header lines - #89
Closed
gaoflow wants to merge 1 commit into
Closed
Conversation
A negative binary list count was never rejected: _read_array computed a negative size and stream.read() read the entire remaining file (BytesIO) or raised a misleading 'early end-of-file' (files). The same class of bug let a negative ASCII list count fall through as 'malformed input' and a negative element count leak a raw numpy ValueError. Check counts for non-negativity at the shared _read_array helper and at parse_element/_from_fields, carrying the real message through PlyElementParseError. The blank-line skip promised since 1.1.3 is now reachable: the stale pre-rewrite EOF sentinel in consume() fired on empty lines first and misreported them as 'early end-of-file'.
Owner
|
Hey @gaoflow, thanks for the interest. Yeah, I guess there's insufficient validation for item counts in general. IMO it's very unlikely to encounter this in practice, so it's not super urgent to patch this, but we do try to be pretty strict about validation, so we should do it anyway. In general this project doesn't accept AI-authored contributions, so I'll probably have to close this PR, but I do appreciate learning about the deficiency, even if it's minor. |
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
2 participants
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.
Three validation defects in the hand-rolled parser, all from counts never checked as non-negative:
_read_arraycomputed a negative size, so BytesIO read the whole rest of the file as the list (silent corruption) and real files raised a misleading "early end-of-file"; with int8 values and count -1 it silently swallowed the file on both stream types. Repro:PlyData.read(BytesIO(b"ply\nformat binary_little_endian 1.0\nelement f0 1\nproperty list int8 int8 v0\nend_header\n" + b"\xff\x01\x02\x03"))returns a 3-element list from count -1. Now rejected asnegative array length -1.element vertex -1leaked a raw numpyValueErrorinstead of the documentedPlyParseError.consume().The count check lives in the shared
_read_arrayhelper, which also fixes the ASCII list path (was "malformed input"). One existing test asserted the old buggy line number for(b"ply\n\n"); updated 2 -> 3. Suite: 130 passed (was 120).📚 Documentation preview 📚: https://python-plyfile--89.org.readthedocs.build/en/89/