Read RECORD with the default csv reader, not str.splitlines() - #354
Open
dylanpulver wants to merge 1 commit into
Open
Read RECORD with the default csv reader, not str.splitlines()#354dylanpulver wants to merge 1 commit into
dylanpulver wants to merge 1 commit into
Conversation
`construct_record_file` writes RECORD with `csv.writer`, so a path containing a newline is correctly emitted as a quoted multi-line field. Both read sites called `str.splitlines()` first, which drops the terminator inside the quoted field and additionally splits on characters `csv` does not treat as row separators (\v, \f, \x1c-\x1e, \x85, U+2028, U+2029). `validate_record()` then reports a file as "not mentioned in RECORD" for a RECORD that does mention it. Pass the RECORD text through `io.StringIO(..., newline="")` instead, so the reader sees the document as written.
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.
construct_record_filewrites RECORD withcsv.writer, so a path containing a newline comes out correctly quoted. Both read sites —WheelFile.validate_recordandWheelFile.get_contents— calledstr.splitlines()before handing the result toparse_record_file, so installer cannot read the RECORD it just wrote.Measured on
59f0a65, using installer's own writer and then its own reader:splitlines()also splits on characterscsvdoes not treat as row separators. I ran the writer and then the reader for all ten:\nand\rcome back with the character silently deleted;\v,\f,\x1c,\x1d,\x1e,\x85, U+2028 and U+2029 raiseexpected 3 elements, got 1. All ten read back byte-identical throughcsv.readerover the same file.The recording-installed-projects spec pins the dialect: RECORD "must be readable with the default
readerof Python'scsvmodule", line terminator "either\r\nor\n". Interposingsplitlines()is a different, larger set of terminators.Fix is to hand the RECORD text to
parse_record_fileasio.StringIO(text, newline="").Ran
pytest tests/: 152 passed,coverage reportstill 100%. Reverting onlysrc/installer/sources.pytoorigin/mainfails both new cases (150 passed, 2 failed).splitlines(keepends=True)is the obvious narrower fix and it does pass the newline case — csv reassembles the quoted field from the kept terminators — but it still fails the U+2028 case, which is why the test parametrises both.ruff checkreports the same 3 pre-existingPLR1704on an unmodified checkout and none in what I touched.I did not change
parse_record_fileitself beyond one docstring line; it already accepts any iterable of strings.AI assistance: this change was drafted with Claude Opus 5 (
claude-opus-5). The outputs above were produced by running installer's own writer and reader locally against this branch.