From 4c555c1b767caf886ff0534ca544d3566df2a2d6 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 18 Aug 2026 10:37:32 +0000 Subject: [PATCH 1/3] fix(xlsx): grow ScanBuffer when a token fills the whole async window TryWithoutIO rewinds the cursor to the pre-parse position when a row's XML doesn't fit in the buffered data. If that position is 0 because the buffer was already completely full (e.g. a >64 KiB inline string), the subsequent RefillAsync could neither compact (start == 0) nor read more (no free space), so it silently reported success without adding any bytes -- causing the TryParseNext/RefillAsync loop to re-parse the same data forever. RefillAsync now doubles the buffer when it has no free space to read into. CanReadMore and the synchronous Refill() now size against the buffer's actual length instead of the original fixed constant so they stay correct after a grow. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01EFjpg2qcda2vq9B2u1Xe1U --- .../Internal/Readers/Xlsx/ScanBuffer.cs | 29 +++++++++++++- .../Readers/Xlsx/ScanBufferTests.cs | 33 ++++++++++++++++ .../Readers/Xlsx/SheetCursorTests.cs | 39 +++++++++++++++++++ 3 files changed, 99 insertions(+), 2 deletions(-) diff --git a/src/XLSight/Internal/Readers/Xlsx/ScanBuffer.cs b/src/XLSight/Internal/Readers/Xlsx/ScanBuffer.cs index a23c1eb..a221f62 100644 --- a/src/XLSight/Internal/Readers/Xlsx/ScanBuffer.cs +++ b/src/XLSight/Internal/Readers/Xlsx/ScanBuffer.cs @@ -45,7 +45,7 @@ internal ScanBuffer(Stream source) /// Current unconsumed window as a span. internal ReadOnlySpan Span => _buf.AsSpan(_start, _end - _start); - internal bool CanReadMore => !_streamDone && (_start > 0 || _end < BufferSize); + internal bool CanReadMore => !_streamDone && (_start > 0 || _end < _buf.Length); /// /// Resets the buffer pointers and refills from the underlying stream's current position. @@ -105,7 +105,7 @@ internal bool Refill() // (DeflateStream/zlib-ng returns data in variable-size chunks.) if (!_streamDone) { - int space = BufferSize - _end; + int space = _buf.Length - _end; if (space > 0) { int bytesRead = _source.Read(_buf, _end, space); @@ -132,6 +132,12 @@ internal bool Refill() /// Because skips compaction, the buffer start pointer is /// already restored before this method is called, so compaction shifts from the /// correct position. + /// + /// When a pending token already spans the entire buffer (e.g. a very long inline + /// string) there are zero consumed bytes to compact away, so the buffer is grown + /// instead — otherwise this method would report success without reading anything, + /// and the caller's parse/refill loop would spin forever re-parsing the same bytes. + /// /// internal async ValueTask RefillAsync(CancellationToken ct = default) { @@ -149,6 +155,12 @@ internal async ValueTask RefillAsync(CancellationToken ct = default) if (!_streamDone) { int space = _buf.Length - _end; + if (space == 0) + { + Grow(); + space = _buf.Length - _end; + } + while (space > 0) { int bytesRead = await _source.ReadAsync(_buf.AsMemory(_end, space), ct).ConfigureAwait(false); @@ -165,6 +177,19 @@ internal async ValueTask RefillAsync(CancellationToken ct = default) return _end > _start; } + /// + /// Doubles the buffer's capacity, preserving all unconsumed bytes at the front. + /// Only called when a pending token doesn't fit in the current buffer at all + /// (no bytes available to compact away). + /// + private void Grow() + { + byte[] grown = ArrayPool.Shared.Rent(_buf.Length * 2); + _buf.AsSpan(0, _end).CopyTo(grown); + ArrayPool.Shared.Return(_buf); + _buf = grown; + } + /// True when the stream is exhausted and is empty. internal bool IsExhausted => _streamDone && _start >= _end; diff --git a/tests/XLSight.Tests/Readers/Xlsx/ScanBufferTests.cs b/tests/XLSight.Tests/Readers/Xlsx/ScanBufferTests.cs index fa1423c..3fb72e1 100644 --- a/tests/XLSight.Tests/Readers/Xlsx/ScanBufferTests.cs +++ b/tests/XLSight.Tests/Readers/Xlsx/ScanBufferTests.cs @@ -195,6 +195,39 @@ public async Task RefillAsync_EmptyStreamAfterConsumingAll_ReturnsFalse() Assert.False(result); } + [Fact] + public async Task RefillAsync_AfterFullBufferRewind_GrowsInsteadOfStalling() + { + // Regression: when TryWithoutIO rewinds to a saved position of 0 because a + // single token doesn't fit in an already-full 64 KiB window, RefillAsync + // can't compact (start == 0) and can't read (no free space) -- it used to + // report success without moving _end, so the outer loop re-parsed the same + // bytes forever. RefillAsync must grow the buffer to make progress instead. + const int BufferSize = 65536; + var data = new byte[BufferSize + 10]; + data.AsSpan(BufferSize).Fill((byte)'Z'); + using var stream = new MemoryStream(data); + using var buf = new ScanBuffer(stream); + + // Constructor priming fills the buffer completely: start = 0, end = BufferSize. + Assert.Equal(BufferSize, buf.Span.Length); + + // Simulate a token that doesn't fit in the fully-buffered window: the parse + // consumes nothing and calls Refill(), triggering the IO-skip / rewind path. + bool parsed = buf.TryWithoutIO(() => buf.Refill()); + Assert.False(parsed); + Assert.True(buf.LastParseNeededIO); + Assert.Equal(BufferSize, buf.Span.Length); // rewound to the same (full) position + + bool result = await buf.RefillAsync(TestContext.Current.CancellationToken); + + Assert.True(result); + Assert.True( + buf.Span.Length > BufferSize, + "RefillAsync must grow the buffer to make progress when a token doesn't fit in a full window."); + Assert.Equal(BufferSize + 10, buf.Span.Length); + } + // ── IsExhausted ────────────────────────────────────────────────────────── [Fact] diff --git a/tests/XLSight.Tests/Readers/Xlsx/SheetCursorTests.cs b/tests/XLSight.Tests/Readers/Xlsx/SheetCursorTests.cs index 20786f3..ca756c6 100644 --- a/tests/XLSight.Tests/Readers/Xlsx/SheetCursorTests.cs +++ b/tests/XLSight.Tests/Readers/Xlsx/SheetCursorTests.cs @@ -304,6 +304,45 @@ public async Task TryParseNext_InlineStringTagNameCollision_Terminates() Assert.Equal("Item_3", rows[2].GetCell(1).AsText()); } + // Regression: a single row whose XML exceeds the 64 KiB ScanBuffer window (e.g. a + // very long inline string) makes TryWithoutIO rewind to buffer start on every + // attempt. Because the buffer is already full at that point, RefillAsync can + // neither compact (start == 0) nor read more (no free space) -- it must report + // success without adding any bytes, so the TryParseNext/RefillAsync loop spun + // forever re-parsing the same bytes instead of growing the buffer. + [Fact] + public async Task TryParseNext_RowLargerThanBuffer_Terminates() + { + string hugeText = new string('A', 100_000); + using var cursor = OpenCursor($""" + + + {hugeText} + 2 + + + """); + + var rows = new List(); + int attempts = 0; + while (attempts++ < 1000) + { + if (cursor.TryParseNext(out var row)) + { + rows.Add(row.ToSnapshot()); + continue; + } + + if (cursor.IsSheetDone) { break; } + if (!await cursor.RefillAsync()) { break; } + } + + Assert.True(attempts < 1000, "TryParseNext/RefillAsync loop did not terminate for a row larger than the buffer."); + Assert.Equal(2, rows.Count); + Assert.Equal(hugeText, rows[0].GetCell(1).AsText()); + Assert.Equal(2.0, rows[1].GetCell(1).AsNumber()); + } + // ── Column projection ───────────────────────────────────────────────────── [Fact] From b4fe44b2c341daedf8ce0b37adffcd8752e835ea Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 18 Aug 2026 10:40:35 +0000 Subject: [PATCH 2/3] test(xlsx): trim over-explained regression test comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Shorten the ScanBuffer/SheetCursor regression test comments to match the terser style used by the existing regression test in the same files. ScanBuffer's XML docs are left as-is — their fuller explanations are justified there. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01EFjpg2qcda2vq9B2u1Xe1U --- .../Readers/Xlsx/ScanBufferTests.cs | 18 ++++++------------ .../Readers/Xlsx/SheetCursorTests.cs | 9 +++------ 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/tests/XLSight.Tests/Readers/Xlsx/ScanBufferTests.cs b/tests/XLSight.Tests/Readers/Xlsx/ScanBufferTests.cs index 3fb72e1..3d4e456 100644 --- a/tests/XLSight.Tests/Readers/Xlsx/ScanBufferTests.cs +++ b/tests/XLSight.Tests/Readers/Xlsx/ScanBufferTests.cs @@ -195,36 +195,30 @@ public async Task RefillAsync_EmptyStreamAfterConsumingAll_ReturnsFalse() Assert.False(result); } + // Regression: a token that fills the buffer with start == 0 left RefillAsync unable + // to compact or read, so it reported success with zero new bytes. [Fact] public async Task RefillAsync_AfterFullBufferRewind_GrowsInsteadOfStalling() { - // Regression: when TryWithoutIO rewinds to a saved position of 0 because a - // single token doesn't fit in an already-full 64 KiB window, RefillAsync - // can't compact (start == 0) and can't read (no free space) -- it used to - // report success without moving _end, so the outer loop re-parsed the same - // bytes forever. RefillAsync must grow the buffer to make progress instead. const int BufferSize = 65536; var data = new byte[BufferSize + 10]; data.AsSpan(BufferSize).Fill((byte)'Z'); using var stream = new MemoryStream(data); using var buf = new ScanBuffer(stream); - // Constructor priming fills the buffer completely: start = 0, end = BufferSize. + // Constructor priming fills the buffer completely. Assert.Equal(BufferSize, buf.Span.Length); - // Simulate a token that doesn't fit in the fully-buffered window: the parse - // consumes nothing and calls Refill(), triggering the IO-skip / rewind path. + // A token that doesn't fit: the parse consumes nothing and rewinds. bool parsed = buf.TryWithoutIO(() => buf.Refill()); Assert.False(parsed); Assert.True(buf.LastParseNeededIO); - Assert.Equal(BufferSize, buf.Span.Length); // rewound to the same (full) position + Assert.Equal(BufferSize, buf.Span.Length); bool result = await buf.RefillAsync(TestContext.Current.CancellationToken); Assert.True(result); - Assert.True( - buf.Span.Length > BufferSize, - "RefillAsync must grow the buffer to make progress when a token doesn't fit in a full window."); + Assert.True(buf.Span.Length > BufferSize, "RefillAsync must grow the buffer to make progress."); Assert.Equal(BufferSize + 10, buf.Span.Length); } diff --git a/tests/XLSight.Tests/Readers/Xlsx/SheetCursorTests.cs b/tests/XLSight.Tests/Readers/Xlsx/SheetCursorTests.cs index ca756c6..5c0093c 100644 --- a/tests/XLSight.Tests/Readers/Xlsx/SheetCursorTests.cs +++ b/tests/XLSight.Tests/Readers/Xlsx/SheetCursorTests.cs @@ -304,12 +304,9 @@ public async Task TryParseNext_InlineStringTagNameCollision_Terminates() Assert.Equal("Item_3", rows[2].GetCell(1).AsText()); } - // Regression: a single row whose XML exceeds the 64 KiB ScanBuffer window (e.g. a - // very long inline string) makes TryWithoutIO rewind to buffer start on every - // attempt. Because the buffer is already full at that point, RefillAsync can - // neither compact (start == 0) nor read more (no free space) -- it must report - // success without adding any bytes, so the TryParseNext/RefillAsync loop spun - // forever re-parsing the same bytes instead of growing the buffer. + // Regression: a row larger than the 64 KiB ScanBuffer window (e.g. a long inline + // string) rewound the buffer to start on every attempt, and RefillAsync couldn't + // compact or read more space, so the loop never terminated. [Fact] public async Task TryParseNext_RowLargerThanBuffer_Terminates() { From 40500a50bc5b6014b178da0f009588042fe52d82 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 18 Aug 2026 11:12:33 +0000 Subject: [PATCH 3/3] fix(xlsx): cap ScanBuffer growth to prevent unbounded allocation Grow() doubled the buffer with no upper bound, so a token that never completes (corrupt or adversarial XML) would keep growing until OOM, and doubling past ~1 GB would overflow the int passed to ArrayPool.Shared.Rent. Cap growth at 16 MB, computed in a long to avoid the overflow, and raise MalformedWorkbookException past that point -- matching the pattern already used for oversized XLSB records and VBA streams. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01EFjpg2qcda2vq9B2u1Xe1U --- .../Internal/Readers/Xlsx/ScanBuffer.cs | 13 ++++++++++++- .../Readers/Xlsx/ScanBufferTests.cs | 19 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/XLSight/Internal/Readers/Xlsx/ScanBuffer.cs b/src/XLSight/Internal/Readers/Xlsx/ScanBuffer.cs index a221f62..85daf9a 100644 --- a/src/XLSight/Internal/Readers/Xlsx/ScanBuffer.cs +++ b/src/XLSight/Internal/Readers/Xlsx/ScanBuffer.cs @@ -9,6 +9,7 @@ namespace XLSight.Internal.Readers.Xlsx; internal sealed class ScanBuffer : IDisposable { private const int BufferSize = 65536; + private const int MaxBufferSize = 16 * 1024 * 1024; private readonly Stream _source; private byte[] _buf; @@ -182,9 +183,19 @@ internal async ValueTask RefillAsync(CancellationToken ct = default) /// Only called when a pending token doesn't fit in the current buffer at all /// (no bytes available to compact away). /// + /// + /// The doubled size would exceed — the pending row/token + /// is treated as corrupt rather than growing the buffer without bound. + /// private void Grow() { - byte[] grown = ArrayPool.Shared.Rent(_buf.Length * 2); + long doubled = (long)_buf.Length * 2; + if (doubled > MaxBufferSize) + { + throw new MalformedWorkbookException("Worksheet row exceeds the supported maximum size."); + } + + byte[] grown = ArrayPool.Shared.Rent((int)doubled); _buf.AsSpan(0, _end).CopyTo(grown); ArrayPool.Shared.Return(_buf); _buf = grown; diff --git a/tests/XLSight.Tests/Readers/Xlsx/ScanBufferTests.cs b/tests/XLSight.Tests/Readers/Xlsx/ScanBufferTests.cs index 3d4e456..fca8049 100644 --- a/tests/XLSight.Tests/Readers/Xlsx/ScanBufferTests.cs +++ b/tests/XLSight.Tests/Readers/Xlsx/ScanBufferTests.cs @@ -222,6 +222,25 @@ public async Task RefillAsync_AfterFullBufferRewind_GrowsInsteadOfStalling() Assert.Equal(BufferSize + 10, buf.Span.Length); } + // Regression: unbounded growth from a token that never completes (corrupt or + // adversarial input) must stop with a parse error, not grow until OOM. + [Fact] + public async Task RefillAsync_GrowthExceedsMax_ThrowsMalformedWorkbookException() + { + var data = new byte[32 * 1024 * 1024]; + using var stream = new MemoryStream(data); + using var buf = new ScanBuffer(stream); + + await Assert.ThrowsAsync(async () => + { + while (true) + { + buf.TryWithoutIO(() => buf.Refill()); + await buf.RefillAsync(TestContext.Current.CancellationToken); + } + }); + } + // ── IsExhausted ────────────────────────────────────────────────────────── [Fact]