fix(csv): reject CSV and TSV files that span too many cells - #4518
Conversation
Pandas sizes the data-frame by the first record and pads every shorter record to that width. A few-KB file whose first line is a long run of delimiters therefore spans millions of cells; partition_csv() and partition_tsv() render every one of them to HTML and parse it back, at ~400 B per cell. Measure the span by streaming the records with the csv module before pd.read_csv, stopping as soon as it passes CSV_MAX_CELLS (default 5,000,000), and raise UnprocessableEntityError. When the delimiter is left to Pandas (sep=None), sniff it the same way Pandas does, from the first non-blank line, so the measured shape matches what Pandas reads. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
All reported issues were addressed across 7 files
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
…ings Review follow-ups to the CSV/TSV cell limit: - Scan in fixed-size chunks with a quote-aware counter for the first record instead of csv.reader, so a delimiter-heavy record is never built in memory, the csv module's 128 KiB field limit no longer rejects large TSV fields, and no "\n" delimiter is needed (Python 3.13 rejects it). - Count only "\r"/"\n" as line endings. codecs line splitting also broke on "\x0c" and other characters Pandas reads as content, which let a wide first record be measured as one column. - Treat whitespace-only lines as blank the way each Pandas engine does, so a blank first line no longer hides the wide record after it. - Normalize line endings to "\n" before Pandas reads the file. Pandas 2.x's C tokenizer reads a "\r" line ending followed by a whitespace-only line as 2^18 empty rows, so 5 bytes became 262,145 rows inside pd.read_csv(). - When the delimiter is sniffed, sniff it once from the normalized first line and pass it to Pandas, so the scan and the read agree; a file with no usable delimiter is read as one column. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
Normalizing every line ending to "\n" also rewrote "\r\n" inside quoted fields, changing cell text from CRLF files (e.g. multi-line cells in exports). Only a lone "\r" triggers Pandas 2.x's row explosion, so convert just that and let Pandas read "\r\n" natively. The delimiter is still sniffed from the first line with its ending normalized, the same text the size check sniffs. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 3 unresolved issues from previous reviews.
Re-trigger cubic
Review follow-ups: - Stream the file to Pandas through a reader that decodes it in chunks and converts each lone "\r", holding back a "\r" that ends a chunk until the next one shows whether it starts a "\r\n". The file is no longer read into memory whole, decoded and copied before parsing. - Drop leading byte-order marks before both the size check and Pandas. The check decoded UTF-8 without dropping the BOM that the read dropped, so a BOM-prefixed blank line could hide a wide record from it. - Sniff the delimiter from the first non-blank line in both paths, so a leading blank line no longer turns a delimited file into one column. - For Pandas' Python engine, treat a record as blank when its one value, quoted or not, is whitespace, as that engine skips it. - Use the ASCII unit separator as the delimiter of a file with no usable one in both paths, so a streamed file needs no whole-text scan to pick one. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Shadow auto-approve: would not auto-approve. This PR does not meet the repository auto-approval settings.
Re-trigger cubic
cragwolfe
left a comment
There was a problem hiding this comment.
GPT Pro review of exact head bdd02b8, verified against source and main. The pre-pandas guard and carriage-return handling improve the targeted amplification paths, but fallback delimiter lookahead introduces quadratic blank-prefix replay before parsing a tiny table; this prevents approval. See inline evidence and acceptance criteria. Exact-head run 36956091412 succeeded (53 checks successful, one skipped). No local tests or source changes.
Nonblocking follow-ups: describe the later physical-line count as a conservative upper bound (blank lines and quoted newlines can count, contrary to the PR summary); cover header/implicit-index width as residual hardening; add multibyte/BOM/quote/CRLF boundary and genuine early-stop tests. TSV now opens compressed filenames as raw bytes and requires tell/seek on caller streams, narrowing prior pandas input handling; restore these narrow compatibility paths or explicitly document their supported-input limits, ensuring any restored decompression feeds the same content to both guard and parser.
(authored by codex)
| end = len(self._buffer) | ||
| if size is not None and 0 <= size < end: | ||
| end = size | ||
| text, self._buffer = self._buffer[:end], self._buffer[end:] |
There was a problem hiding this comment.
[High] Avoid quadratic replay after fallback delimiter lookahead. peek_first_non_blank_line() retains the entire leading blank prefix in self._buffer; pandas' Python engine then consumes csv.reader over this TextIOBase stream, invoking readline(). Each self._buffer[end:] copies almost the whole remaining prefix. For b"\n" * (1 << 22) + b"a\tb\n1\t2\n", the restricted sample has no delimiter, the new lookahead eventually selects tab, and preflight skips the prefix and accepts a four-cell table. Replaying ~4M one-character lines entails O(N^2) copied characters (~8 TiB cumulatively), a source-derived operation count rather than a runtime measurement. Main has no added lookahead/replay buffer, so this amplification is introduced. Make lookahead plus consumption amortized-linear with bounded rewind/replay or chunk/cursor buffering. Preserve delimiter, blank-record, quoting and CRLF semantics; whitespace-only physical lines cannot simply all be dropped when whitespace is the delimiter. Add a many-chunk leading-blank fallback regression with correct output and deterministic bounded copying/buffering assertions. The fixed-size underlying-read test currently uses an explicit comma separator and misses this interaction.
(authored by codex)
Review follow-ups: - The reader kept text in one string and sliced the rest off on every read, so replaying a long blank prefix kept by the delimiter lookahead copied it once per line: quadratic in the prefix. Queue decoded chunks with a read offset instead, so each character is copied a bounded number of times however the text is read. The size check's own lookahead re-yields the original chunks rather than one grown string. - Count one more column when the first record is a header: Pandas takes a data field beyond the header's width as an implicit index. - partition_tsv() again decompresses a compressed filename (e.g. .tsv.gz), through Pandas' handle, the same content for the size check and the read; and a stream that cannot seek is spooled to a temporary file once rather than failing. - Cover chunk-boundary independence (multibyte, BOM, quotes, CRLF and lone "\r"), stopping to read once the limit is passed, and the replayed-blank-prefix fallback. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 5 files (changes from recent commits).
Shadow auto-approve: would not auto-approve. This PR does not meet the repository auto-approval settings.
Re-trigger cubic
cragwolfe
left a comment
There was a problem hiding this comment.
Fresh full-context GPT Pro re-review of b526efa, followed by source/dependency verification against main: approved under the requested main-relative standard. The prior quadratic blank-prefix replay is resolved: readline advances within queued chunks and drops consumed chunks instead of copying the remaining prefix; preflight replays original chunks without growing concatenation. The added fallback-output and deterministic copy-bound tests cover the reported interaction. Compressed-filename and non-seekable TSV handling are restored. The cumulative cell preflight and lone-CR normalization materially improve main's resource behavior.
Nonblocking residual, verified from pandas source: header width + one column is not a bound for multiple implicit-index columns. For example, a one-field header followed by a very wide first data row and many short rows can pass the new estimate while pandas builds a much wider padded/index structure. Oracle advised withholding approval for this gap; comparison with main shows the same C-parser path already has that amplification with no preflight at all. This is incomplete hardening of the earlier nonblocking header/index follow-up, rather than an introduced expansion, so it does not prevent approval under the requested policy.
Follow-up: measure the data/index width using the quote-aware bounded scanner, including the Python engine's index-name inference, and charge all admitted fields before pandas. Cover more than one implicit-index column and a ragged tail in TSV/C and CSV/Python paths, with a mocked-pandas rejection assertion and sufficient-budget output compatibility. Keep index semantics intact rather than forcing index_col=False. Also describe the scan as a conservative estimate rather than an unconditional memory/byte bound: retained blank-prefix lookahead and non-seekable spooling remain proportional to input size, and blank/quoted physical line endings can overcount.
Validation: complete untruncated production bundle, fresh holistic Pro response, and independent verification of all advisory findings against current source and main. Exact-head checks show two successful non-test checks and one neutral security result; no Actions test workflow is recorded at this SHA. The new tests still need normal CI execution. No local tests or source changes.
(authored by codex)
# Conflicts: # CHANGELOG.md # unstructured/__version__.py # unstructured/partition/utils/config.py
With a header, Pandas turns the fields a data row has beyond the header into implicit-index columns: its C engine takes their number from the first data row, and its Python engine also from the second, which can make the first data row the index names. Counting one extra column under-counted a short header followed by a very wide row. Measure the header and the first two data records with the same quote-aware scanner and count the widest; any later, wider row is an error in Pandas. Also describe the check as a conservative cell estimate rather than a memory or byte bound. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Shadow auto-approve: would not auto-approve. This PR does not meet the repository auto-approval settings.
Re-trigger cubic
# Conflicts: # CHANGELOG.md # unstructured/__version__.py # unstructured/partition/utils/config.py
Summary
pandas sizes the data-frame by the first record and pads every shorter record out to that width. A small CSV or TSV whose first line is a long run of delimiters can therefore span millions of cells.
partition_csv()andpartition_tsv()render every cell to HTML and parse it back, at about 400 B per cell.check_cell_count()streams it in fixed-size chunks and measures its span, stopping as soon as it passesCSV_MAX_CELLS(new env setting, default 5,000,000), and raisesUnprocessableEntityError.\rline ending followed by a whitespace-only line as 2^18 empty rows, sob",\n\r ,"(5 bytes) becomes 262,145 rows insidepd.read_csv, before any check can run (fixed in pandas 3.1; the project pins<3). pandas now reads the file through a streaming reader that converts each lone\rto\n.\r\nis left alone, so text inside quoted fields of CRLF files is unchanged.partition_tsv()still decompresses a compressed filename (e.g..tsv.gz), with the same content going to the check and the read, and copies a stream that cannot seek to a temporary file once.Measurements
b",\n\r ,"pd.read_csvThe check was fuzzed against pandas' actual output in a memory-capped container. About 27,000 generated inputs (quotes, CR/LF/CRLF, form-feeds, BOMs, leading blank lines, 3-byte chunks) gave 0 undercounts and 0 parser blowups. A further 10,000 inputs read with a header gave 13,727 comparisons, counting the header row and implicit-index columns, also with 0 undercounts.
Behaviour changes
CSV_MAX_CELLShigher to allow it.csv.Error, is now read as one column.Tests
csvmodule's 128 KiB limit, a.tsv.gzfilename, and a stream that cannot seek.🤖 Generated with Claude Code