diff --git a/.editorconfig b/.editorconfig index 137bb51d..abc53cb6 100644 --- a/.editorconfig +++ b/.editorconfig @@ -2,7 +2,11 @@ root = true [*] charset = utf-8 -end_of_line = crlf +# lf, not crlf: every tracked file is committed LF, and CI runs the `dotnet format whitespace` +# gate on Linux where nothing rewrites endings on checkout. Demanding crlf here only passes on +# Windows, where core.autocrlf converts on checkout and hides the mismatch. See .gitattributes, +# which pins eol=lf so no contributor's autocrlf setting can change what lands in a blob. +end_of_line = lf insert_final_newline = true trim_trailing_whitespace = true diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 00000000..ae4d0f4d --- /dev/null +++ b/.gitattributes @@ -0,0 +1,18 @@ +# Line endings are pinned here rather than left to each contributor's core.autocrlf, because +# CI runs `dotnet format whitespace --verify-no-changes` on Linux: if a blob's endings depend on +# who committed it, that gate passes or fails based on the author's machine instead of the code. +# Every tracked text file is already LF, and .editorconfig's end_of_line matches. +* text=auto eol=lf + +# Spreadsheet fixtures are ZIP (xlsx/xlsm/xlsb) and CFB (xls) containers. Never let text=auto's +# heuristic touch them — a single LF/CRLF substitution inside a compressed stream or an OLE sector +# corrupts the file, and the parsers would then be tested against garbage that git created. +*.xlsx binary +*.xlsm binary +*.xlsb binary +*.xls binary + +# The benchmark CSV fixture is deliberately left under the rule above rather than marked -text: +# its blob is already LF, and -text would freeze whatever a contributor's working tree happens to +# hold, which on a machine with core.autocrlf=true means silently rewriting it to CRLF. Tests that +# actually cover terminator handling build their own fixtures in code. diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md new file mode 100644 index 00000000..6e94039c --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -0,0 +1,36 @@ +--- +name: Bug report +about: Something isn't working as expected +title: "" +labels: bug +--- + +**Describe the bug** +A clear description of what's wrong. + +**File format(s) involved** +XLSX / XLSB / XLS / CSV + +**Reproduction** +Minimal code sample, and — if possible — a minimal file that reproduces the issue (strip any +sensitive data first). If the file can't be shared, describe its shape (row/column count, styles, +shared strings, etc.) as precisely as you can. + +```csharp +// minimal repro here +``` + +**Expected behavior** +What you expected to happen. + +**Actual behavior** +What actually happened — include the full exception message/stack trace if there is one. + +**Environment** +- ExcelReader.NET version: +- .NET version (8/10): +- OS: + +**Note on security issues:** if this bug is a potential vulnerability (crash, excessive +memory/CPU, or other issue triggerable by an untrusted file), please do **not** open a public +issue — see [SECURITY.md](../../SECURITY.md) for the private reporting channel instead. diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md new file mode 100644 index 00000000..2227a5a3 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feature_request.md @@ -0,0 +1,22 @@ +--- +name: Feature request +about: Suggest an addition or change to the library +title: "" +labels: enhancement +--- + +**What problem does this solve?** +Describe the use case — what are you trying to do that the library doesn't support today? + +**Proposed API/behavior** +Sketch the API shape you'd expect, if you have one in mind. + +```csharp +// proposed usage +``` + +**Alternatives considered** +Any workarounds you're using today, or other approaches you considered. + +**Additional context** +Anything else — links, related issues, prior art in other libraries. diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 00000000..efa33011 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,15 @@ +## What does this change? + + + +## Checklist + +- [ ] `dotnet build ExcelReader.slnx --configuration Release` builds clean (warnings are errors) +- [ ] `dotnet test tests/ExcelReader.Tests/ExcelReader.Tests.csproj --configuration Release` passes +- [ ] If this changes the public API: `PublicAPI.Unshipped.txt` updated for **both** `net8.0` and `net10.0` +- [ ] Tests added/updated for the behavior change +- [ ] One focused change — unrelated fixes are in a separate PR + +## Test plan + + diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 65c0584f..7a208a33 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -47,6 +47,10 @@ jobs: - name: Build (Release) run: dotnet build ExcelReader.slnx --configuration Release --no-restore -p:DeterministicSourcePaths=false + - name: Verify formatting (whitespace) + if: matrix.os == 'ubuntu-latest' + run: dotnet format whitespace ExcelReader.slnx --verify-no-changes + - name: Test (Release) + collect coverage run: >- dotnet test --project tests/ExcelReader.Tests/ExcelReader.Tests.csproj diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 0d401c9b..ac68a9fe 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -2,9 +2,9 @@ name: CodeQL on: push: - branches: [ master ] + branches: [ master, develop ] pull_request: - branches: [ master ] + branches: [ master, develop ] schedule: # Weekly scan (Mondays 06:00 UTC) to catch newly published query updates. - cron: '0 6 * * 1' diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index df7bead1..4a0aba4c 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -53,6 +53,13 @@ jobs: -p:Version=${{ steps.version.outputs.value }} --output ./artifacts + - name: Generate SBOM + uses: anchore/sbom-action@v0 + with: + path: ./artifacts + format: spdx-json + output-file: ./artifacts/excelreader.spdx.json + - name: NuGet login (Trusted Publishing / OIDC) uses: NuGet/login@v1 id: nuget-login diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md new file mode 100644 index 00000000..2459f0db --- /dev/null +++ b/ARCHITECTURE.md @@ -0,0 +1,109 @@ +# Architecture + +A map of how the codebase fits together, not a manual. Start here, then follow the file/type names +into the source — the code comments carry the detailed reasoning. + +## The four format families + +Each format has its own `Reader` and a writer implementing `IWorkbookWriter` +(`src/ExcelReader.Core/Writer/IWorkbookWriter.cs`): + +| Format | Reader | Writer | Sheet/row writer | +|---|---|---|---| +| XLSX | `XlsxReader` | `XlsxWorkbookWriter` | `XlsxSheetWriter`/`XlsxRowWriter` | +| XLSB | `XlsbReader` | `XlsbWorkbookWriter` | `XlsbSheetWriter`/`XlsbRowWriter` | +| XLS | `XlsReader` | `XlsWorkbookWriter` | `XlsSheetWriter`/`XlsRowWriter` | +| CSV | `CsvReader` | `CsvWorkbookWriter` | `CsvSheetWriter`/`CsvRowWriter` | + +CSV has one extra layer: `CsvWriter` is the low-level RFC4180 writer (buffered rows straight to the +stream, no sheets/styles/shared-strings machinery); `CsvWorkbookWriter` adapts it to the shared +`IWorkbookWriter` contract, exposing exactly one sheet. + +On top of all four readers sits the typed-parsing layer (`src/ExcelReader.Core/Parser/`): +`ExcelParser` (reflection/attribute-driven, allocates a model per row) and `RefParser` +(binds a `ref struct` model directly to `Cell.Value` spans — zero allocation for the container and, +for span-typed columns, for the values too). Both consume `Row`/`Cell` from any reader uniformly. + +## Shared plumbing + +Reader internals that would otherwise be duplicated four times over live in one place: + +- **`CellAccumulator`** — pooled per-row cell storage (raw decoded UTF-8 text + a `CellDesc[]` + describing each cell's column/type/style/offset). Used by every format's enumerator. Also hosts + the shared BIFF numeric-error-code → text lookup (`#DIV/0!` etc.) for XLS/XLSB. +- **`PooledStreamRowEnumerator`** — abstract base centralizing the pooled-buffer lifecycle + (`BufferedStreamCursor` + `CellAccumulator`, `Fill`/`FillAsync`/`Ensure` wrappers) that every + format's enumerator subclasses. `MoveNext`/`MoveNextAsync` stay concrete per format — this only + removes the buffer-lifecycle boilerplate, not the parsing itself. +- **`BufferedStreamCursor`** — the refill/compact-or-grow cursor behind the XLSX/XLSB/CSV + forward-only stream enumerators. Has a second constructor for the in-memory-ZIP path that wraps an + already-fully-decompressed `ReadOnlyMemory` instead of a `Stream` (`Eof = true` immediately, + no refills). +- **`WorkbookLookups`** — small lookups that were once duplicated identically across readers: sheet + name→index, sheet-index bounds checks, date-style flags, shared-string offsets, and (ZIP formats + only) worksheet entry resolution plus prefetch/limit-counting stream composition. Takes arrays as + parameters rather than requiring a shared interface, since each format's backing arrays differ in + shape. +- **`LimitChecks`** — the DoS/resource-limit guards (`ExcelReaderOptions`/`CsvReaderOptions`), including + the single buffer-growth-cap function (`NextBufferSize`) every pooled buffer in the stack grows + through, so one limit policy governs all of them consistently. + +## Why readers are split into partial classes + +`XlsxReader` and `XlsbReader` are large enough that one file would be unwieldy, so each is split by +concern rather than by size: + +- `XlsxReader.cs` / `XlsbReader.cs` — fields, constructors, sheet navigation, dispose. +- `*.Loading.cs` (XLSX only) — one-time workbook-level XML parsing (sheets, shared strings, date1904). +- `*.Memory.cs` — the in-memory path: constructs directly over `ZipMemoryIndex`/`ZipPart` instead of + a `Stream`/`ZipArchive`, so it never suspends even under `await foreach`. +- `*.Styles.cs` (XLSX only) — builds the cellXfs-index → is-date-style table. +- `*.Enumerator.cs` — the nested `Enumerator`: the actual streaming row/cell parser. By far the + largest file in each reader. + +All partials of one reader share one field set (C# partial classes are one type), so e.g. +`.Loading.cs`'s shared-string parse populates fields the nested `Enumerator` in `.Enumerator.cs` +reads back. `XlsReader` follows a reduced version of the same split (no `.Memory.cs` — the OLE +compound-file container has no in-memory-ZIP equivalent). + +## The sync/async twin convention + +Hot-path search/refill primitives (e.g. `IndexOf`/`IndexOfAsync`/`IndexOfSlowAsync`, +`EnsureRowBuffered`/`...Async`/`...SlowAsync` in `XlsxReader.Enumerator.cs`) come in three tiers, not +one generic async method: + +1. A blocking sync loop for the sync caller. +2. An async method whose common case — the data is already in the buffered window — is a synchronous + check returning an already-completed `ValueTask`, so no async state machine is allocated on the + hot path. +3. A separate `...SlowAsync` method holding the actual `await`-in-a-loop, split out so the rare + awaiting branch doesn't bloat the fast path's IL/JIT inlining. + +Once a row is fully buffered, parsing it (`ParseRow`) has no async twin at all — a fully-buffered +span never needs to await, so both `MoveNext` and `MoveNextAsync` call the same synchronous parse. + +A parity test suite (`tests/ExcelReader.Tests/SyncAsyncParityTests.cs`) asserts identical cell +snapshots across sync / async-open / `GetAsyncEnumerator` for all four formats, guarding against the +twins drifting apart. + +## The `Row`/`Cell` ref-struct lifetime model + +`Row` and `Cell` are `public readonly ref struct`s: zero-allocation views aliasing the reader's own +pooled buffers (the accumulator's cell/value arrays, the shared-string buffer, or — for XLSX's bare +`` fast path — the live read buffer directly). Being `ref struct` lets them hold `ReadOnlySpan` +fields, and the compiler physically prevents a caller from doing anything that would outlive them: +they can't be boxed, stored in a field, captured in a closure, or crossed over an `await`/`yield`. + +**Validity window:** exactly one enumeration step. `MoveNext`/`MoveNextAsync` resets the accumulator +and may compact or resize the buffer, so a `Row`/`Cell` from step *N* is invalidated the instant step +*N+1* starts. + +**Escape hatch:** `Cell.GetString()` materializes the value as a real `string` (deduplicated for +repeated shared-string cells via an internal cache), `TryFormat` copies raw text into a caller-owned +buffer without allocating a `string`, and `TryGetDouble`/`TryParse`/`TryGetDateTime` extract plain +value types — all safe to keep past the row's lifetime since none of them are spans. + +## Further reading + +- [`SECURITY.md`](SECURITY.md) — supported versions and how to report a vulnerability. +- [`CONTRIBUTING.md`](CONTRIBUTING.md) — build expectations and how to submit a change. diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md new file mode 100644 index 00000000..aa928b4e --- /dev/null +++ b/CODE_OF_CONDUCT.md @@ -0,0 +1,48 @@ +# Code of Conduct + +## Our Pledge + +We as members, contributors, and leaders pledge to make participation in our community a +harassment-free experience for everyone, regardless of age, body size, visible or invisible +disability, ethnicity, sex characteristics, gender identity and expression, level of experience, +education, socio-economic status, nationality, personal appearance, race, religion, or sexual +identity and orientation. + +## Our Standards + +Examples of behavior that contributes to a positive environment: + +- Demonstrating empathy and kindness toward other people +- Being respectful of differing opinions, viewpoints, and experiences +- Giving and gracefully accepting constructive feedback +- Accepting responsibility and apologizing for mistakes, and learning from the experience + +Examples of unacceptable behavior: + +- The use of sexualized language or imagery, and sexual attention or advances of any kind +- Trolling, insulting or derogatory comments, and personal or political attacks +- Public or private harassment +- Publishing others' private information without explicit permission +- Other conduct which could reasonably be considered inappropriate in a professional setting + +## Enforcement Responsibilities + +Maintainers are responsible for clarifying and enforcing these standards, and will take appropriate +and fair corrective action in response to any behavior deemed inappropriate, threatening, offensive, +or harmful. + +## Scope + +This Code of Conduct applies within all project spaces (issues, pull requests, discussions) and when +an individual is officially representing the project in public spaces. + +## Enforcement + +Instances of abusive, harassing, or otherwise unacceptable behavior may be reported to the +maintainers via GitHub's private reporting channel on this repository. All complaints will be +reviewed and investigated promptly and fairly. + +## Attribution + +This Code of Conduct is adapted from the [Contributor Covenant](https://www.contributor-covenant.org), +version 2.1. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 00000000..db0fd253 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,52 @@ +# Contributing + +Thanks for considering a contribution. This project takes small, focused pull requests over large +rewrites — see [ARCHITECTURE.md](ARCHITECTURE.md) for the shape of the codebase before diving in, and +[STYLEGUIDE.md](STYLEGUIDE.md) for the code style, which the analyzers only partly enforce. + +## Build expectations + +- **Warnings are errors.** `Directory.Build.props` sets `TreatWarningsAsErrors`, with a curated + `AnalysisMode=All` analyzer set (Sonar, Meziantou, Roslynator, AsyncFixer, and more). A PR that + doesn't build clean locally won't build clean in CI either — run a full build before pushing: + + ```bash + dotnet build ExcelReader.slnx --configuration Release + ``` + +- **Public API changes require a `PublicAPI.Unshipped.txt` entry.** `Microsoft.CodeAnalysis.PublicApiAnalyzers` + is active (arrives transitively via `Roslyn.Diagnostics.Analyzers`) and fails the build on any + unrecorded public member. If you add, change, or remove anything public, update **both** + `src/ExcelReader.Core/PublicAPI/net8.0/PublicAPI.Unshipped.txt` and + `src/ExcelReader.Core/PublicAPI/net10.0/PublicAPI.Unshipped.txt`. A bot promotes `Unshipped` → + `Shipped` automatically after each release — don't edit `Shipped.txt` by hand. + +- **Tests are required for behavior changes.** Run the suite before opening a PR: + + ```bash + dotnet test tests/ExcelReader.Tests/ExcelReader.Tests.csproj --configuration Release + ``` + + Untrusted-input paths (the CFB/OLE, BIFF8, BIFF12, and ZIP parsers) get extra scrutiny — new + parsing code should have a corresponding limit/fuzz-safety test in + `tests/ExcelReader.Tests/ReaderLimitTests.cs` or `FuzzTests.cs` where relevant. Read + [STYLEGUIDE.md § Untrusted Input](STYLEGUIDE.md#untrusted-input) before touching a parser: every + length, offset, and size read from the file must be bounded before it drives an allocation. + +## Pull requests + +- One focused change per PR — don't batch unrelated fixes into one commit or one PR. +- If a change is user-visible (new API, behavior change, performance claim), mention it in the PR + description; the README's benchmark tables and changelog are updated separately, not as part of + every PR. +- CI runs on Linux, Windows, and macOS across .NET 8 and .NET 10 — a change that only builds on one + OS/TFM combination isn't ready to merge. + +## Reporting bugs / requesting features + +Use GitHub Issues for bugs and feature requests. For suspected security vulnerabilities, do **not** +open a public issue — see [SECURITY.md](SECURITY.md) for the private reporting channel. + +## Code of conduct + +This project follows the [Code of Conduct](CODE_OF_CONDUCT.md). diff --git a/README.md b/README.md index 8172631c..a27fe107 100644 --- a/README.md +++ b/README.md @@ -10,11 +10,13 @@ High-performance Excel reading and writing for .NET 10. Reads `.xlsx`, `.xlsb`, `.xls`, and `.csv`; writes `.xlsx`, `.xlsb`, `.xls`, and `.csv`. -ExcelReader is built for streaming spreadsheet workloads where low allocations matter. It reads worksheet rows as lightweight `ref struct` values, resolves shared strings, recognizes date styles, handles sparse cells, and includes writers for producing `.xlsx` (Open XML), `.xlsb` (BIFF12), and `.xls` (BIFF8) workbooks. The library also supports opening workbook data directly from in-memory buffers without requiring a stream, which makes it convenient for API and network-based scenarios. +ExcelReader is built for streaming spreadsheet workloads where low allocations matter. It reads worksheet rows as lightweight `ref struct` values, resolves shared strings, recognizes date styles, handles sparse cells, and includes writers for producing `.xlsx` (Open XML), `.xlsb` (BIFF12), and `.xls` (BIFF8) workbooks. The library also supports opening workbook data directly from in-memory buffers without requiring a stream, which makes it convenient for API and network-based scenarios. `Excel.FromCsv(ReadOnlyMemory)` and `Excel.FromXls(ReadOnlyMemory)` now accept caller-owned buffers directly, and `Excel.Open(ReadOnlyMemory)` routes XLS workbooks through the same true-memory path instead of wrapping the bytes in `MemoryStream`. ## Benchmarks -Benchmarks were run with BenchmarkDotNet v0.15.8 on Windows 10 (22H2), AMD Ryzen 7 5700X, .NET 10.0.9 (SDK 11.0.100-preview.4). Generated-data benchmarks use 50,000 rows, except the string-heavy reads, which use 65,536. Raw results: [`tests/ExcelReader.Benchmarks/BenchmarkDotNet.Artifacts/results`](tests/ExcelReader.Benchmarks/BenchmarkDotNet.Artifacts/results). +Benchmarks were run with BenchmarkDotNet v0.15.8 on Windows 10 (22H2), AMD Ryzen 7 5700X, .NET 10.0.9 (SDK 11.0.100-preview.4), except where a table states its own machine. Generated-data benchmarks use 50,000 rows, except the string-heavy reads, which use 65,536. Raw results: [`tests/ExcelReader.Benchmarks/BenchmarkDotNet.Artifacts/results`](tests/ExcelReader.Benchmarks/BenchmarkDotNet.Artifacts/results). + +**Benchmark methodology.** In every table below, the **"Cell-by-cell read"** rows (including the CSV table's, and the per-format rows in "Real data reads" / "String-heavy reads") read ExcelReader's `cell.Value` — a zero-copy `ReadOnlySpan`, no decode or allocation — against each competitor's own idiomatic read call. For Sylvan, that's the ADO.NET-style `GetString(i)`, which is forced to materialize a UTF-16 `string`; Sylvan's API has no zero-copy accessor, so it cannot avoid that cost. These rows are therefore not matched work: part of the reported gap is "we parse faster" and part is "we skipped an allocation you were never offered a way to skip." Each affected benchmark class also has a `*_Materialized` sibling (calling `cell.GetString()`, the same UTF-16 materialization Sylvan pays) so the matched-work number is measurable — see `tests/ExcelReader.Benchmarks/BenchmarkAccumulators.cs`. **Those results are published under [Matched-work reads](#matched-work-reads-_materialized), and reading them is the honest way to judge the comparison:** treat the cell-by-cell ratios in the tables below as an upper bound, not a like-for-like number. The **"Typed row parsing"** / **"Typed record writing"** rows are unaffected — both sides already materialize real objects/strings there, so those comparisons are matched work as published. ### XLSX @@ -86,12 +88,22 @@ The latest real-data benchmark also measures the in-memory path for workbook con | Method | Mean | Error | StdDev | Allocated | |---|---:|---:|---:|---:| -| Xlsx_ExcelReader_Memory | 65.86 ms | 0.446 ms | 0.396 ms | 4.98 KB | -| Xlsx_ExcelReader_Memory_Prefetch | 42.88 ms | 0.198 ms | 0.166 ms | 71.33 KB | -| Xlsm_ExcelReader_Memory | 66.48 ms | 0.597 ms | 0.558 ms | 4.98 KB | -| Xlsm_ExcelReader_Memory_Prefetch | 43.59 ms | 0.574 ms | 0.509 ms | 72.99 KB | -| Xlsb_ExcelReader_Memory | 24.98 ms | 0.247 ms | 0.231 ms | 14.43 KB | -| Xlsb_ExcelReader_Memory_Prefetch | 19.40 ms | 0.256 ms | 0.200 ms | 33.06 KB | +| Xlsx_ExcelReader_Memory | 67.03 ms | 1.059 ms | 1.088 ms | 4.98 KB | +| Xlsx_ExcelReader_Memory_Prefetch | 53.17 ms | 3.265 ms | 9.626 ms | 94.57 KB | +| Xlsm_ExcelReader_Memory † | 67.06 ms | 1.295 ms | 1.148 ms | 4.98 KB | +| Xlsm_ExcelReader_Memory_Prefetch | 48.75 ms | 0.914 ms | 1.016 ms | 93.44 KB | +| Xlsb_ExcelReader_Memory † | 30.12 ms | 0.569 ms | 0.584 ms | 14.45 KB | +| Xlsb_ExcelReader_Memory_Prefetch | 18.46 ms | 0.124 ms | 0.110 ms | 59.70 KB | +| Xls_ExcelReader_Memory † | 11.02 ms | 0.182 ms | 0.161 ms | 10.05 KB | +| Csv_ExcelReader_Memory † | 5.91 ms | 0.072 ms | 0.064 ms | 168 B | + +† These four rows come from a later targeted re-run of only the `*_Memory` benchmarks; the unmarked rows are from the preceding full-suite run. Timings are not strictly comparable across the two — a smaller run has less GC and cache interference from neighboring benchmarks, which is the likely cause of the small `Xlsm`/`Xlsb` improvements (their allocation figures are byte-identical, and neither path was touched by the change described below). + +`Xls_ExcelReader_Memory` and `Csv_ExcelReader_Memory` are the two most recently added overloads; the rest predate them. Csv's memory path is both faster (5.91 ms vs. 6.21 ms for `Csv_ExcelReader`) and allocates less (168 B vs. 232 B). + +`Xls_ExcelReader_Memory` was initially ~11% *slower* than its stream twin: it re-derived every record's address through the FAT chain and materialized `ReadOnlyMemory.Span` per read, while the stream path amortized translation across a 64 KB sector window. `BiffCursor` now caches the enclosing contiguous sector run and slices the backing array directly, so a sequentially written Workbook stream translates with one compare per record. That took it from 14.77 ms to **11.02 ms** here, against 13.31 ms for `Xls_ExcelReader` in the full run — and 0.61x the stream path on the same-run 20K-row `MemorySourceSmokeBenchmark`, which is the more trustworthy ratio since both arms ran together. + +One caveat on that path: it allocates **80 B more** than the stream twin (10.05 KB vs. 9.97 KB), up from byte-identical before the change, because `BiffCursor` grew the cached-run fields and two cursors are constructed per read. Allocation here is fixed reader setup rather than per-record, so it does not scale with workbook size, but the memory overload is faster than the stream path rather than strictly cheaper than it. ### String-heavy reads @@ -104,6 +116,45 @@ The real-data corpus above is mostly numbers and dates — its shared-string tab Both formats now handle this well: ~3.2x and ~2.3x faster than Sylvan for XLSX and XLSB respectively, at roughly ~24x and ~23x less memory, with no garbage collections in either configuration. XLSB's shared-string path previously materialized its table eagerly (~27 MB here); `ParseSharedStreaming` brought it in line with the XLSX streaming/pooling path, cutting allocation by ~36x on this workload. +### Matched-work reads (`*_Materialized`) + +Every read benchmark class has a `*_Materialized` sibling that calls `cell.GetString()` per text cell — the same UTF-16 materialization Sylvan's ADO.NET-style API is forced to pay — while keeping `TryParse`/`TryGetDateTime` for numeric and date cells, exactly mirroring the competitor accumulator. These are the matched-work counterparts to the zero-copy rows above. + +**Machine for the three tables in this section only:** BenchmarkDotNet v0.15.8, Windows 11 (10.0.26200.8875/25H2), 13th Gen Intel Core i7-1355U 1.70 GHz (1 CPU, 12 logical / 10 physical cores), .NET 10.0.10, SDK 10.0.302, X64 RyuJIT x86-64-v3 — **not** the Ryzen 7 5700X used for every other table. + +> **Do not divide these timings against the span-based rows above.** The CPU differs, so a cross-table ratio folds the hardware change into what looks like a materialization cost. Isolating that needs a same-machine paired run. **Allocation figures, however, are deterministic and safe to compare across tables** — those are the rows to read for the real tradeoff. The XLSX/XLSM means below also carry wide error bars (a thermally-constrained laptop part), so medians are given alongside. + +Real-data workbook, per format: + +| Format | Mean | Median | StdDev | Allocated | +|---|---:|---:|---:|---:| +| XLSX | 95.46 ms | 82.47 ms | 24.44 ms | 99.41 KB | +| XLSM | 81.89 ms | 80.32 ms | 9.46 ms | 104.27 KB | +| XLSB | 32.44 ms | 32.51 ms | 0.50 ms | 74.86 KB | +| XLS | 17.96 ms | 17.89 ms | 0.29 ms | 47.92 KB | +| CSV | 21.30 ms | 21.26 ms | 0.52 ms | 36,567.43 KB | + +Generated 50,000-row XLSX workbook: + +| Scenario | Mean | Median | StdDev | Allocated | +|---|---:|---:|---:|---:| +| Cell-by-cell read, materialized | 14.44 ms | 13.77 ms | 2.14 ms | 1.59 MB | + +String-heavy workbook (65,536 rows, ~190,000 distinct shared strings): + +| Format | Mean | StdDev | Gen0 | Gen1 | Gen2 | Allocated | +|---|---:|---:|---:|---:|---:|---:| +| XLSX | 112.45 ms | 3.05 ms | 2,600 | 1,400 | 400 | 32.19 MB | +| XLSB | 99.64 ms | 4.69 ms | 2,600 | 1,400 | 400 | 36.83 MB | + +Reading the allocation columns against the tables above gives the honest shape of the tradeoff: + +- **CSV real data:** 36.57 MB materialized against 232 B for the span-based read of the same file. Every CSV field is a distinct string, so nothing dedupes — this is where zero-copy reading earns its keep outright. +- **XLSB real data:** 74.86 KB, essentially cheap. That corpus repeats a small set of values, so the shared-string table dedupes and the reader's string cache materializes each distinct value once. +- **String-heavy XLSX:** 32.19 MB materialized, against **Sylvan's 17.83 MB on the same workload** — doing matched work here, ExcelReader allocates roughly 1.8x *more* than Sylvan, and moves from zero collections to 400 Gen2 collections. The likely cause is the per-reader shared-string cache: with ~190,000 distinct values it holds every materialized string alive for the whole read and pays its dictionary's growth churn on top, whereas the span-based path never materializes them at all. That cache is a clear win on categorical data (the common case) and a liability at high cardinality. Stated as an observation with a hypothesis, not a measured attribution — quantifying it is an open item. + +The takeaway is not that one column beats the other: it is that ExcelReader's headline read numbers come from a zero-copy path competitors do not expose, and when it does the same work as them, the gap narrows sharply and can invert on allocation for high-cardinality string data. + ### Typed record writing `WorkbookRecordWriter`/`RecordWriter` (the header-plus-one-row-per-object API — see [Write typed records](#write-typed-records)) across all four formats, same 50,000-record source: @@ -642,6 +693,13 @@ dotnet build ExcelReader.slnx --configuration Release dotnet test --project tests/ExcelReader.Tests/ExcelReader.Tests.csproj --configuration Release ``` +## Contributing + +See [ARCHITECTURE.md](ARCHITECTURE.md) for a map of the codebase, [STYLEGUIDE.md](STYLEGUIDE.md) for +the code style, and [CONTRIBUTING.md](CONTRIBUTING.md) for build expectations and how to submit a +change. Security issues should go through the private channel in [SECURITY.md](SECURITY.md), not a +public issue. + ## License ExcelReader is licensed under the MIT License. See [LICENSE](LICENSE). diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 00000000..c5e98a2f --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,22 @@ +# Security Policy + +## Supported Versions + +Only the latest published release of `ExcelReader.Core` receives security fixes. +Older 1.x releases are not patched retroactively; upgrade to the latest version +before reporting an issue. + +## Reporting a Vulnerability + +Please **do not** open a public GitHub issue for security vulnerabilities. + +Report vulnerabilities privately via +[GitHub Private Vulnerability Reporting](https://github.com/GabrielMarquezMatte/ExcelReader/security/advisories/new). + +Include, where possible: +- The file format(s) involved (`.xlsx`, `.xlsb`, `.xls`, `.csv`) +- A minimal reproduction file or code sample +- The impact you observed (crash, excessive memory/CPU, incorrect data, etc.) + +You should receive an initial response within 5 business days. If the report is +confirmed, a fix will be prepared and released before any public disclosure. diff --git a/STYLEGUIDE.md b/STYLEGUIDE.md index 20c7cc20..2371d1e0 100644 --- a/STYLEGUIDE.md +++ b/STYLEGUIDE.md @@ -1,5 +1,8 @@ # ExcelReader Style Guide +Code style rules. For build/PR/test process, see [CONTRIBUTING.md](CONTRIBUTING.md); for the shape of +the codebase, see [ARCHITECTURE.md](ARCHITECTURE.md). + ## Priorities 1. **Correctness** — the code must be right. No shortcuts that sacrifice safety or accuracy. @@ -136,6 +139,14 @@ Common splits: - Local variables: `camelCase` - Prefer full words over abbreviations (`index` over `idx`, `source` over `src`) unless the abbreviation is idiomatic in the domain (`col`, `buf`, `len`, `pos`) +### Public API naming must be symmetric across formats + +The four formats are peers. A factory, option, or capability that exists for one format must use the +same name shape for all of them — `FromXls`/`FromXlsb`/`FromXlsx`/`FromCsv`, not three of those plus a +bare `From`. A caller who learns one format's entry point should be able to guess the others. When you +add a format-specific member, grep the sibling formats and match, or add the missing siblings in the +same change. + --- ## Comments @@ -150,12 +161,123 @@ Write a comment only when the **why** is not obvious from the code. Do not descr // Loop through each cell and parse its value. ``` +### A comment must stay true, or it is worse than no comment + +Comments carry maintenance cost. Two failure modes are specifically banned: + +- **Do not describe a state the code is no longer in.** A header that says "these lookups are + duplicated across the three readers" is actively misleading once the class exists precisely so they + are not duplicated. When you refactor, reread the comments on what you touched. +- **Do not cite a file that can disappear.** Never point a comment at a design doc, plan, or section + marker (`docs/foo.md`, "see step Z4") as the authoritative explanation. Docs get renamed, merged, + and deleted; the comment then sends a reader to nothing, which is worse than silence. Explain the + reasoning inline, on its own terms. A link may *supplement* a self-contained comment, never replace + it. + --- ## Error Handling Throw at trust boundaries: constructor arguments, public API parameters, malformed external data. Use `ArgumentOutOfRangeException.ThrowIfNegative` and similar .NET 6+ throw helpers. Do not add defensive checks for conditions that are impossible given the internal invariants. +Match the exception to whose fault it is. A malformed file is `InvalidDataException` (or +`ExcelLimitExceededException` when a configured cap is what rejected it) — never +`ArgumentException`/`ArgumentOutOfRangeException`, which tell the caller they made a mistake when the +input file is what is broken. If hostile input can reach a .NET throw helper, you are missing a +validation step upstream. + +--- + +## Untrusted Input + +Everything the parsers read is attacker-controlled: `.xlsx`/`.xlsb`/`.xls`/`.csv` arrive from uploads, +APIs, and mail attachments. These rules are not optional in reader code. + +- **Bound every length, offset, count, and size read from the file before it drives an allocation, an + index, or a loop bound.** A field claiming a multi-GB stream inside a 4 KB file is the canonical + attack: validate against what the container can actually hold (`source.Length`, the enclosing + buffer's length, the spec's fixed value) and reject before allocating, not after. + +- **When you add a guard for one header field, apply it to every sibling field in that header.** The + expensive bugs here have all been *inconsistency*, not ignorance — a validated sector count sitting + three lines above an unvalidated stream size, with a comment on the first one already explaining the + exact attack. If a field needs the guard, assume its neighbours do too. + +- **Use `checked` for arithmetic on values derived from file bytes.** Silent `int` overflow turns a + bounds check into a wrapped negative that sails past it. + +- **Never let a malformed value silently change semantics.** A truncating cast that turns a length into + a negative sentinel meaning "read everything" is a worse outcome than a thrown exception, because + nothing reports it. + +- **Validate before you allocate, not before you use.** Renting or allocating first and discovering the + inconsistency during the walk still hands the attacker the allocation. + +- New or changed parsing code needs a matching test in `ReaderLimitTests.cs` (forge the malformed + header) or `FuzzTests.cs`. Assert the exception type and the limit metadata, not merely that + something threw. + +--- + +## Pooled Buffers + +`ArrayPool.Shared` is used throughout the readers. Misuse here is a correctness bug, not a +performance nit. + +- Every `Rent` needs a matching `Return` on **every** exit path, including thrown exceptions. A method + that rents, then throws mid-walk, must `try`/`catch`/`Return`/`rethrow` — a dropped buffer silently + shrinks the pool for the whole process. +- Never `Return` a buffer that anything still references, and never touch a buffer after returning it. + Ownership transfers must be explicit in a comment when a rented buffer outlives the renting method. +- Pooled arrays come back oversized and dirty. Bound reads by the logical length you tracked, never by + `array.Length`. +- Buffers that live beyond one stack frame are returned in `Dispose`; guard against double-return with + a flag, since `Dispose` may be called twice. + +--- + +## Sync/Async Twins + +Several readers keep paired sync and async methods on purpose: an `async` method pays for a state +machine on every row even when the buffer never needs refilling, so the hot path is duplicated rather +than shared. + +The cost of that choice is that nothing in the compiler keeps the twins honest. So: + +- A behavior fix to one twin **must** be applied to the other in the same change. Read both before you + edit either. +- Behavior shared by both belongs in a sync helper that neither duplicates — the split should cover + only the awaiting, not the logic around it. +- Do not add a third variant to work around a bug in one twin. +- Changes to a twinned path need a parity test asserting both produce identical output, including at + buffer boundaries (a row straddling a refill, a cell forcing buffer growth) — that is exactly where + the two implementations drift. +- A synchronous method does not take a `CancellationToken`. + +--- + +## Tests and Benchmarks + +Harness code is held to the same standard as library code, because a broken harness reports success. + +- **Fail loudly, never silently.** A fixture that was never built, a setup that never ran, an input + that parsed to nothing — assert it. The dangerous case is not the harness that crashes; it is the one + that measures zero work and publishes a number. Do not rely on the code under test to reject an empty + input, because some formats accept it. +- **Register new benchmarks with their setup.** When a class targets its `[GlobalSetup]` at specific + methods, a new `[Benchmark]` absent from every target list runs against an unbuilt fixture. +- **Benchmarks against another library must compare matched work, or say plainly that they do not.** + If our side reads a zero-copy span while the competitor's API forces a string allocation, that gap is + partly "we are faster" and partly "we skipped work they cannot skip". Publish a matched-work sibling + next to it. An unlabelled comparison that flatters us is a defect. +- **State the machine for every published number**, and never derive a ratio across results measured on + different hardware. Allocation figures are deterministic and comparable across machines; timings are + not. +- Name tests for what they actually cover. A fixture hand-authored in code is not a "real world" file, + and calling it one hides the gap it was supposed to close. +- Assert real behavior — exact values, exception types, limit metadata. A test that calls a method and + asserts no throw documents nothing. + --- ## Performance Notes @@ -166,5 +288,10 @@ These rules apply only when a function is on a measured hot path (cell parsing, - Use `ArrayPool.Shared` for buffers that live beyond one stack frame. Return them in `Dispose`. - `ref struct` types stay on the stack; use them for row/cell value types. - Avoid LINQ in hot paths; use `IndexOf` on spans which benefits from SIMD. +- Split cold paths (growth, throw helpers) into `[MethodImpl(MethodImplOptions.NoInlining)]` methods so + they do not bloat the hot caller's IL and cost it inlining. +- Optimize against a measurement, and record what it showed in a comment. An optimization with no + number behind it is a guess that costs readability. Caches in particular are a trade, not a win: + note the workload where the cache pays and the one where it does not. Outside the hot path (workbook loading, shared-strings init), ordinary allocations are fine. diff --git a/global.json b/global.json index 8f73781c..c93bc05e 100644 --- a/global.json +++ b/global.json @@ -1,4 +1,9 @@ { + "sdk": { + "version": "10.0.100", + "rollForward": "latestMinor", + "allowPrerelease": false + }, "test": { "runner": "Microsoft.Testing.Platform" } diff --git a/src/ExcelReader.Core/Enums/CellType.cs b/src/ExcelReader.Core/Enums/CellType.cs index 6fee4349..fdd8b158 100644 --- a/src/ExcelReader.Core/Enums/CellType.cs +++ b/src/ExcelReader.Core/Enums/CellType.cs @@ -1,7 +1,7 @@ namespace ExcelReader.Core.Enums { /// - /// Identifies the kind of value a holds. + /// Identifies the kind of value a holds. /// public enum CellType { diff --git a/src/ExcelReader.Core/Internal/ZipArchiveDisposal.cs b/src/ExcelReader.Core/Internal/ZipArchiveDisposal.cs new file mode 100644 index 00000000..b200780a --- /dev/null +++ b/src/ExcelReader.Core/Internal/ZipArchiveDisposal.cs @@ -0,0 +1,22 @@ +using System.IO.Compression; + +namespace ExcelReader.Core.Internal +{ + // The "#if NET10_0_OR_GREATER await zip.DisposeAsync() #else zip.Dispose()" idiom, shared by every + // ZIP-backed reader and writer (XlsxReader, XlsbReader, XlsxWorkbookWriter, XlsbWorkbookWriter) + // across their dispose paths. + internal static class ZipArchiveDisposal + { + [System.Diagnostics.CodeAnalysis.SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP007:Don't dispose injected", + Justification = "Disposal helper: disposing the caller-owned ZipArchive is its sole purpose — callers delegate their own zip's disposal here.")] + internal static ValueTask DisposeAsync(ZipArchive zip) + { +#if NET10_0_OR_GREATER + return zip.DisposeAsync(); +#else + zip.Dispose(); + return ValueTask.CompletedTask; +#endif + } + } +} diff --git a/src/ExcelReader.Core/Parser/ExcelParserConfig.cs b/src/ExcelReader.Core/Parser/ExcelParserConfig.cs index f57e02f4..a860808b 100644 --- a/src/ExcelReader.Core/Parser/ExcelParserConfig.cs +++ b/src/ExcelReader.Core/Parser/ExcelParserConfig.cs @@ -14,7 +14,7 @@ public sealed class ExcelParserConfig /// Gets the comparer used to match header text to the names bound by property attributes. Defaults to . public StringComparer ColumnNameComparer { get; init; } = StringComparer.OrdinalIgnoreCase; - /// Gets how header text is normalized before it is compared against bound property names. Defaults to . + /// Gets how header text is normalized before it is compared against bound property names. Defaults to . public HeaderNormalization HeaderNormalization { get; init; } = HeaderNormalization.Trim; /// diff --git a/src/ExcelReader.Core/Parser/Internal/SparseRowProjection.cs b/src/ExcelReader.Core/Parser/Internal/SparseRowProjection.cs index 873faf33..5068c091 100644 --- a/src/ExcelReader.Core/Parser/Internal/SparseRowProjection.cs +++ b/src/ExcelReader.Core/Parser/Internal/SparseRowProjection.cs @@ -5,9 +5,9 @@ namespace ExcelReader.Core.Parser.Internal { // The merge-walk column-binding loop shared by RowProjector (class/struct models) and // NamedRefRowEnumerator (ref struct models, net9+). Both bind sparse Row.Cells to a - // header-resolved column map the same way; before this they carried two byte-identical copies (see - // docs/road-to-a.md F9). A `static` generic method — never storing TModel in a field — is what lets - // a ref-struct-constrained TModel flow through without CS8345. + // header-resolved column map the same way; before this they carried two byte-identical copies. A + // `static` generic method — never storing TModel in a field — is what lets a ref-struct-constrained + // TModel flow through without CS8345. // // Deliberately NOT shared with CsvRowProjector: CSV rows are dense (field index == column index, // no gaps), so its fast path is a direct indexed scan with no merge-walk at all — forcing it through diff --git a/src/ExcelReader.Core/Parser/RefParser.cs b/src/ExcelReader.Core/Parser/RefParser.cs index 0a215a36..69b17688 100644 --- a/src/ExcelReader.Core/Parser/RefParser.cs +++ b/src/ExcelReader.Core/Parser/RefParser.cs @@ -30,9 +30,9 @@ namespace ExcelReader.Core.Parser /// unsupported property type is silently left unbound (matching 's /// existing behavior) unless marked [ExcelRequired], which throws at type-map-build time /// instead. A model using only ReadOnlySpan<byte>/numeric/bool/date columns is fully - /// zero-alloc end to end — not just the container (see docs/performance-plan.md P2, which - /// measured the container-only win for a plain struct; this closes the remaining gap for genuine ref - /// structs too). + /// zero-alloc end to end — not just the container. A plain struct model with + /// already avoids allocating the container itself; this closes the + /// remaining gap by letting a text column bind to a span instead of allocating a string too. /// /// public static class RefParser diff --git a/src/ExcelReader.Core/PublicAPI/net10.0/PublicAPI.Unshipped.txt b/src/ExcelReader.Core/PublicAPI/net10.0/PublicAPI.Unshipped.txt index 7dc5c581..6bf1bff1 100644 --- a/src/ExcelReader.Core/PublicAPI/net10.0/PublicAPI.Unshipped.txt +++ b/src/ExcelReader.Core/PublicAPI/net10.0/PublicAPI.Unshipped.txt @@ -1 +1,8 @@ #nullable enable +static ExcelReader.Core.Reader.Excel.FromCsv(System.ReadOnlyMemory data, ExcelReader.Core.Reader.CsvReaderOptions? options = null) -> ExcelReader.Core.Reader.CsvReader! +static ExcelReader.Core.Reader.Excel.FromXls(System.ReadOnlyMemory data, ExcelReader.Core.Reader.ExcelReaderOptions? options = null) -> ExcelReader.Core.Reader.XlsReader! +static ExcelReader.Core.Reader.Excel.FromXlsx(System.IO.Stream! stream, bool leaveOpen = true, ExcelReader.Core.Reader.ExcelReaderOptions? options = null) -> ExcelReader.Core.Reader.XlsxReader! +static ExcelReader.Core.Reader.Excel.FromXlsx(System.ReadOnlyMemory data, ExcelReader.Core.Reader.ExcelReaderOptions? options = null) -> ExcelReader.Core.Reader.XlsxReader! +static ExcelReader.Core.Reader.Excel.FromXlsxAsync(System.IO.Stream! stream, bool leaveOpen = true, ExcelReader.Core.Reader.ExcelReaderOptions? options = null, System.Threading.CancellationToken ct = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.ValueTask +static ExcelReader.Core.Reader.Excel.FromXlsxFile(string! path, ExcelReader.Core.Reader.ExcelReaderOptions? options = null) -> ExcelReader.Core.Reader.XlsxReader! +static ExcelReader.Core.Reader.Excel.FromXlsxFileAsync(string! path, ExcelReader.Core.Reader.ExcelReaderOptions? options = null, System.Threading.CancellationToken ct = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.ValueTask diff --git a/src/ExcelReader.Core/PublicAPI/net8.0/PublicAPI.Unshipped.txt b/src/ExcelReader.Core/PublicAPI/net8.0/PublicAPI.Unshipped.txt index 7dc5c581..6bf1bff1 100644 --- a/src/ExcelReader.Core/PublicAPI/net8.0/PublicAPI.Unshipped.txt +++ b/src/ExcelReader.Core/PublicAPI/net8.0/PublicAPI.Unshipped.txt @@ -1 +1,8 @@ #nullable enable +static ExcelReader.Core.Reader.Excel.FromCsv(System.ReadOnlyMemory data, ExcelReader.Core.Reader.CsvReaderOptions? options = null) -> ExcelReader.Core.Reader.CsvReader! +static ExcelReader.Core.Reader.Excel.FromXls(System.ReadOnlyMemory data, ExcelReader.Core.Reader.ExcelReaderOptions? options = null) -> ExcelReader.Core.Reader.XlsReader! +static ExcelReader.Core.Reader.Excel.FromXlsx(System.IO.Stream! stream, bool leaveOpen = true, ExcelReader.Core.Reader.ExcelReaderOptions? options = null) -> ExcelReader.Core.Reader.XlsxReader! +static ExcelReader.Core.Reader.Excel.FromXlsx(System.ReadOnlyMemory data, ExcelReader.Core.Reader.ExcelReaderOptions? options = null) -> ExcelReader.Core.Reader.XlsxReader! +static ExcelReader.Core.Reader.Excel.FromXlsxAsync(System.IO.Stream! stream, bool leaveOpen = true, ExcelReader.Core.Reader.ExcelReaderOptions? options = null, System.Threading.CancellationToken ct = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.ValueTask +static ExcelReader.Core.Reader.Excel.FromXlsxFile(string! path, ExcelReader.Core.Reader.ExcelReaderOptions? options = null) -> ExcelReader.Core.Reader.XlsxReader! +static ExcelReader.Core.Reader.Excel.FromXlsxFileAsync(string! path, ExcelReader.Core.Reader.ExcelReaderOptions? options = null, System.Threading.CancellationToken ct = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.ValueTask diff --git a/src/ExcelReader.Core/Reader/BiffCursor.cs b/src/ExcelReader.Core/Reader/BiffCursor.cs index e0ba56b3..9b719ce9 100644 --- a/src/ExcelReader.Core/Reader/BiffCursor.cs +++ b/src/ExcelReader.Core/Reader/BiffCursor.cs @@ -1,5 +1,6 @@ using System.Buffers; using System.Buffers.Binary; +using System.Runtime.CompilerServices; namespace ExcelReader.Core.Reader { @@ -16,11 +17,24 @@ internal sealed class BiffCursor : IDisposable private int _loadedCount; // number of sectors loaded in _sector private byte[]? _scratch; // assembles records that span sectors + // In-memory modes: the whole buffer, copied into locals here so the per-record path never + // reloads them through _wb. + private readonly byte[] _file; + private readonly int _fileBase; + + // Chained mode's cached contiguous sector run (logical bounds + the run's buffer offset). + // Empty until the first read; -1 can never satisfy the fast-path compare. + private long _runStart = -1; + private long _runEnd = -1; + private long _runBufferOffset; + internal BiffCursor(WorkbookStream wb) { _wb = wb; _sectorSize = wb.SectorSize; - if (wb.IsMemory) + _file = wb.Buffer; + _fileBase = wb.BufferBase; + if (wb.Kind != WorkbookStream.SourceKind.Streamed) { _maxSectors = 0; _sector = null; @@ -67,10 +81,14 @@ internal bool TryReadRecord(out int id, out ReadOnlySpan data) // otherwise assembled into the scratch buffer. Valid only until the next cursor read. private ReadOnlySpan ReadSpan(long pos, int len) { - if (_wb.IsMemory) + if (_wb.Kind == WorkbookStream.SourceKind.Contiguous) { return _wb.Memory(pos, len); } + if (_wb.Kind == WorkbookStream.SourceKind.Chained) + { + return ReadChainedSpan(pos, len); + } int chainIndex = (int)(pos / _sectorSize); int within = (int)(pos % _sectorSize); LoadSector(chainIndex); @@ -84,13 +102,44 @@ private ReadOnlySpan ReadSpan(long pos, int len) return scratch.AsSpan(0, len); } + // Inlined into ReadSpan: for a sequentially written Workbook stream the cached run covers the + // whole file, so this collapses to one compare plus an array slice per record. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private ReadOnlySpan ReadChainedSpan(long pos, int len) + { + if (pos >= _runStart && pos + len <= _runEnd) + { + return _file.AsSpan(_fileBase + (int)(_runBufferOffset + pos - _runStart), len); + } + return ReadChainedSpanSlow(pos, len); + } + + private ReadOnlySpan ReadChainedSpanSlow(long pos, int len) + { + _wb.ResolveChainedRun(pos, out _runStart, out _runEnd, out _runBufferOffset); + if (pos + len <= _runEnd) + { + return _file.AsSpan(_fileBase + (int)(_runBufferOffset + pos - _runStart), len); + } + // Straddles a chain discontinuity: assemble sector-by-sector, as the streamed path does + // when a record crosses its sector window. + byte[] scratch = EnsureScratch(len); + _wb.CopyChained(pos, scratch.AsSpan(0, len)); + return scratch.AsSpan(0, len); + } + private void ReadInto(long pos, Span dest) { - if (_wb.IsMemory) + if (_wb.Kind == WorkbookStream.SourceKind.Contiguous) { _wb.Memory(pos, dest.Length).CopyTo(dest); return; } + if (_wb.Kind == WorkbookStream.SourceKind.Chained) + { + _wb.CopyChained(pos, dest); + return; + } int written = 0; while (written < dest.Length) { diff --git a/src/ExcelReader.Core/Reader/BufferedStreamCursor.cs b/src/ExcelReader.Core/Reader/BufferedStreamCursor.cs index d9a4e66d..1685dca7 100644 --- a/src/ExcelReader.Core/Reader/BufferedStreamCursor.cs +++ b/src/ExcelReader.Core/Reader/BufferedStreamCursor.cs @@ -35,7 +35,9 @@ internal BufferedStreamCursor(int maxCellBytes, string limitName, int initialCap // Pre-filled, EOF from the start: the whole ZIP part is already decompressed, so there is // nothing left to refill and no source to refill from. `content` may alias a sub-range of a // larger array (e.g. a stored entry sliced out of the whole-file buffer), so Pos/Len start at - // that sub-range's offsets rather than always at 0. + // that sub-range's absolute offsets rather than always at 0 — every consumer of this cursor + // (XlsxReader/XlsbReader included) indexes Buf directly via Pos/Len, so those offsets must stay + // absolute instead of being rebased to the segment's own 0-based range. internal BufferedStreamCursor(ReadOnlyMemory content, int maxCellBytes, string limitName) { _maxCellBytes = maxCellBytes; diff --git a/src/ExcelReader.Core/Reader/CsvReader.Enumerator.cs b/src/ExcelReader.Core/Reader/CsvReader.Enumerator.cs index 2639dc34..dd781a2b 100644 --- a/src/ExcelReader.Core/Reader/CsvReader.Enumerator.cs +++ b/src/ExcelReader.Core/Reader/CsvReader.Enumerator.cs @@ -55,6 +55,14 @@ internal Enumerator(Stream stream, CsvReaderOptions options, CancellationToken c _stripBom = options.DetectEncodingFromByteOrderMark; } + internal Enumerator(ReadOnlyMemory content, CsvReaderOptions options, CancellationToken ct = default) + : base(content, options.MaxCellBytes, nameof(CsvReaderOptions.MaxCellBytes), ct) + { + _delimiter = options.Delimiter; + _quote = options.Quote; + _stripBom = options.DetectEncodingFromByteOrderMark; + } + // Cells point either into _buf (the common, zero-copy case: unquoted or plain-quoted // fields are already contiguous bytes read straight from the stream) or into _acc's value // buffer (only for fields needing unescaping, e.g. a doubled "" quote, or malformed @@ -167,7 +175,7 @@ private void BeginRecord() // over trickling streams ever matter. private bool TryParseRecordFromBuffer() { - byte[] buf = _buf; + ReadOnlySpan buf = _buf.AsSpan(0, _len); int len = _len; byte delim = _delimiter; byte quote = _quote; @@ -180,7 +188,7 @@ private bool TryParseRecordFromBuffer() // directly with no FieldState/materialization machinery at all. Falls through to the // general per-field parser when a quote is hit first, or when the terminator/EOF isn't // resolvable yet. - ReadOnlySpan remaining = buf.AsSpan(pos, len - pos); + ReadOnlySpan remaining = buf[pos..len]; int first = remaining.IndexOfAny(Cr, Lf, quote); bool quoteFirst = first >= 0 && remaining[first] == quote; int lineTerm = quoteFirst ? -1 : first; @@ -331,7 +339,8 @@ private void EnsureBomStripped() return; } Ensure(3); - if (_len - _pos >= 3 && _buf[_pos] == 0xEF && _buf[_pos + 1] == 0xBB && _buf[_pos + 2] == 0xBF) + ReadOnlySpan buf = _buf.AsSpan(0, _len); + if (_len - _pos >= 3 && buf[_pos] == 0xEF && buf[_pos + 1] == 0xBB && buf[_pos + 2] == 0xBF) { _pos += 3; } @@ -349,7 +358,8 @@ private async ValueTask EnsureBomStrippedAsync() return; } await EnsureAsync(3).ConfigureAwait(false); - if (_len - _pos >= 3 && _buf[_pos] == 0xEF && _buf[_pos + 1] == 0xBB && _buf[_pos + 2] == 0xBF) + ReadOnlySpan buf = _buf.AsSpan(0, _len); + if (_len - _pos >= 3 && buf[_pos] == 0xEF && buf[_pos + 1] == 0xBB && buf[_pos + 2] == 0xBF) { _pos += 3; } diff --git a/src/ExcelReader.Core/Reader/CsvReader.cs b/src/ExcelReader.Core/Reader/CsvReader.cs index c11cfcc4..429057a9 100644 --- a/src/ExcelReader.Core/Reader/CsvReader.cs +++ b/src/ExcelReader.Core/Reader/CsvReader.cs @@ -12,9 +12,10 @@ public sealed partial class CsvReader : IExcelRowReader, IExcelRowReader _memory; private readonly long _startPosition = -1; private bool _enumeratedOnce; @@ -39,6 +40,29 @@ internal CsvReader(Stream stream, bool leaveOpen, CsvReaderOptions? options = nu { _startPosition = _stream.Position; } + _memory = default; + } + + internal CsvReader(ReadOnlyMemory data, CsvReaderOptions? options = null) + { + _options = options ?? CsvReaderOptions.Default; + ValidateOptions(_options); + _stream = null; + _leaveOpen = true; + _memory = Transcode(data, _options.Encoding); + } + + private static ReadOnlyMemory Transcode(ReadOnlyMemory data, Encoding? encoding) + { + if (encoding is null || encoding.CodePage == Encoding.UTF8.CodePage) + { + return data; + } + using MemoryStream source = XlsCompoundFile.AsStream(data); + using Stream transcoding = Encoding.CreateTranscodingStream(source, encoding, Encoding.UTF8, leaveOpen: true); + using MemoryStream target = new(data.Length); + transcoding.CopyTo(target); + return target.GetBuffer().AsMemory(0, (int)target.Length); } [SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", @@ -97,6 +121,10 @@ public void MoveToSheet(int index) public Enumerator GetEnumerator() { ResetToStart(); + if (_stream is null) + { + return new Enumerator(_memory, _options); + } return new Enumerator(_stream, _options); } @@ -126,6 +154,10 @@ public ValueTask GetAsyncEnumeratorAsync(CancellationToken ct = defa { ct.ThrowIfCancellationRequested(); ResetToStart(); + if (_stream is null) + { + return new ValueTask(new Enumerator(_memory, _options, ct)); + } return new ValueTask(new Enumerator(_stream, _options, ct)); } @@ -140,6 +172,10 @@ async ValueTask IExcelRowReader.GetAsy // would silently yield zero rows instead of replaying the file — fail loudly instead. private void ResetToStart() { + if (_stream is null) + { + return; + } if (_startPosition >= 0) { _stream.Position = _startPosition; @@ -156,7 +192,7 @@ private void ResetToStart() /// public void Dispose() { - if (!_leaveOpen) + if (!_leaveOpen && _stream is not null) { _stream.Dispose(); } @@ -165,7 +201,11 @@ public void Dispose() /// public ValueTask DisposeAsync() { - return _leaveOpen ? ValueTask.CompletedTask : _stream.DisposeAsync(); + if (_leaveOpen || _stream is null) + { + return ValueTask.CompletedTask; + } + return _stream.DisposeAsync(); } } } diff --git a/src/ExcelReader.Core/Reader/Excel.cs b/src/ExcelReader.Core/Reader/Excel.cs index 55122518..44c5f3c5 100644 --- a/src/ExcelReader.Core/Reader/Excel.cs +++ b/src/ExcelReader.Core/Reader/Excel.cs @@ -1,9 +1,8 @@ using System.Diagnostics.CodeAnalysis; using System.IO.Compression; using System.Runtime.CompilerServices; -using System.Runtime.InteropServices; using ExcelReader.Core.Enums; -using ExcelReader.Core.Writer.Internal; +using ExcelReader.Core.Internal; namespace ExcelReader.Core.Reader { @@ -33,7 +32,7 @@ public static XlsxReader From(Stream stream, bool leaveOpen = true, ExcelReaderO } /// - /// Opens an XLSX workbook directly from an in-memory buffer (docs/in-memory-zip.md). Reads the ZIP + /// Opens an XLSX workbook directly from an in-memory buffer. Reads the ZIP /// central directory and decompresses parts without a /// or intermediate — every part is fully materialized up front, so the returned /// reader never suspends, even under await foreach. @@ -45,6 +44,31 @@ public static XlsxReader From(ReadOnlyMemory data, ExcelReaderOptions? opt return XlsxReader.CreateFromMemory(data, options); } + /// Opens an XLSX workbook from a file path, taking ownership of the file stream. Alias for , for callers who grep for a format-named factory. + /// The path to the XLSX file. + /// Resource limits and behavior toggles; when . + public static XlsxReader FromXlsxFile(string path, ExcelReaderOptions? options = null) + { + return FromFile(path, options); + } + + /// Opens an XLSX workbook from an existing stream. Alias for , for callers who grep for a format-named factory. + /// The stream containing the XLSX data. + /// When (the default), is not disposed when the reader is disposed. + /// Resource limits and behavior toggles; when . + public static XlsxReader FromXlsx(Stream stream, bool leaveOpen = true, ExcelReaderOptions? options = null) + { + return From(stream, leaveOpen, options); + } + + /// Opens an XLSX workbook directly from an in-memory buffer. Alias for , for callers who grep for a format-named factory. + /// The whole XLSX file's bytes. Must outlive the returned reader. + /// Resource limits and behavior toggles; when . + public static XlsxReader FromXlsx(ReadOnlyMemory data, ExcelReaderOptions? options = null) + { + return From(data, options); + } + /// Opens a legacy binary (XLS) workbook from a file path, taking ownership of the file stream. /// The path to the XLS file. /// Resource limits and behavior toggles; when . @@ -64,6 +88,14 @@ public static XlsReader FromXls(Stream stream, bool leaveOpen = true, ExcelReade return new XlsReader(stream, leaveOpen, options); } + /// Opens a legacy binary (XLS) workbook directly from an in-memory buffer. + /// The whole XLS file's bytes. Must outlive the returned reader and must not be mutated while it is in use. + /// Resource limits and behavior toggles; when . + public static XlsReader FromXls(ReadOnlyMemory data, ExcelReaderOptions? options = null) + { + return new XlsReader(data, options); + } + /// Opens an XLSB (Excel binary) workbook from a file path, taking ownership of the file stream. /// The path to the XLSB file. /// Resource limits and behavior toggles; when . @@ -82,7 +114,7 @@ public static XlsbReader FromXlsb(Stream stream, bool leaveOpen = true, ExcelRea } /// - /// Opens an XLSB workbook directly from an in-memory buffer (docs/in-memory-zip.md). Reads the ZIP + /// Opens an XLSB workbook directly from an in-memory buffer. Reads the ZIP /// central directory and decompresses parts without a /// or intermediate — every part is fully materialized up front, so the returned /// reader never suspends, even under await foreach. @@ -116,6 +148,25 @@ public static ValueTask FromAsync(Stream stream, bool leaveOpen = tr return XlsxReader.CreateAsync(stream, leaveOpen, options, ct); } + /// Asynchronously opens an XLSX workbook from a file path, taking ownership of the file stream. Alias for , for callers who grep for a format-named factory. + /// The path to the XLSX file. + /// Resource limits and behavior toggles; when . + /// A token to cancel the open operation. + public static ValueTask FromXlsxFileAsync(string path, ExcelReaderOptions? options = null, CancellationToken ct = default) + { + return FromFileAsync(path, options, ct); + } + + /// Asynchronously opens an XLSX workbook from an existing stream. Alias for , for callers who grep for a format-named factory. + /// The stream containing the XLSX data. + /// When (the default), is not disposed when the reader is disposed. + /// Resource limits and behavior toggles; when . + /// A token to cancel the open operation. + public static ValueTask FromXlsxAsync(Stream stream, bool leaveOpen = true, ExcelReaderOptions? options = null, CancellationToken ct = default) + { + return FromAsync(stream, leaveOpen, options, ct); + } + /// Asynchronously opens a legacy binary (XLS) workbook from a file path, taking ownership of the file stream. /// The path to the XLS file. /// Resource limits and behavior toggles; when . @@ -177,6 +228,14 @@ public static CsvReader FromCsv(Stream stream, bool leaveOpen = true, CsvReaderO return new CsvReader(stream, leaveOpen, options); } + /// Opens a CSV (or other delimited-text) source directly from an in-memory buffer. + /// The whole CSV source's bytes. Must outlive the returned reader and must not be mutated while it is in use. + /// Delimiter, quote, encoding, and size-limit settings; when . + public static CsvReader FromCsv(ReadOnlyMemory data, CsvReaderOptions? options = null) + { + return new CsvReader(data, options); + } + /// Asynchronously opens a CSV (or other delimited-text) source from a file path, taking ownership of the file stream. /// The path to the CSV file. /// Delimiter, quote, encoding, and size-limit settings; when . @@ -241,8 +300,8 @@ public static IExcelRowReader Open(Stream stream, bool leaveOpen = true, ExcelRe /// /// Opens a workbook from an in-memory buffer, auto-detecting its format (XLSX/XLSB/XLS) from its - /// signature (docs/in-memory-zip.md). XLSX/XLSB route through instead of - /// a /, so the returned reader never + /// signature. XLSX/XLSB route through instead of + /// a /, so the returned reader never /// suspends, even under await foreach. /// /// The whole workbook file's bytes. Must outlive the returned reader. @@ -264,7 +323,7 @@ public static IExcelRowReader Open(ReadOnlyMemory data, ExcelReaderOptions } return format switch { - ExcelFileFormat.Xls => new XlsReader(ToMemoryStream(data), leaveOpen: false, effective), + ExcelFileFormat.Xls => new XlsReader(data, effective), ExcelFileFormat.Xlsb => XlsbReader.CreateFromMemory(memZip!, effective), ExcelFileFormat.Xlsx => XlsxReader.CreateFromMemory(memZip!, effective), _ => throw new System.Diagnostics.UnreachableException(), @@ -287,15 +346,6 @@ private static ExcelFileFormat ClassifyMemory(ReadOnlyMemory data, ExcelRe return memZip.TryGetEntry("xl/workbook.bin"u8, out _) ? ExcelFileFormat.Xlsb : ExcelFileFormat.Xlsx; } - private static MemoryStream ToMemoryStream(ReadOnlyMemory data) - { - if (MemoryMarshal.TryGetArray(data, out ArraySegment segment)) - { - return new MemoryStream(segment.Array!, segment.Offset, segment.Count, writable: false); - } - return new MemoryStream(data.ToArray(), writable: false); - } - /// /// Asynchronously opens a workbook from a file path, auto-detecting its format (XLSX/XLSB/XLS) from the /// file's signature and taking ownership of the file stream. diff --git a/src/ExcelReader.Core/Reader/ExcelReaderOptions.cs b/src/ExcelReader.Core/Reader/ExcelReaderOptions.cs index 1e49162b..cfbc444d 100644 --- a/src/ExcelReader.Core/Reader/ExcelReaderOptions.cs +++ b/src/ExcelReader.Core/Reader/ExcelReaderOptions.cs @@ -7,6 +7,10 @@ namespace ExcelReader.Core.Reader public sealed record ExcelReaderOptions { /// Gets the maximum total decompressed bytes allowed across the whole workbook. Defaults to 512 MiB. + /// Applies to ZIP-backed formats (XLSX/XLSB) as their decompressed byte budget, and to + /// the legacy CFB container (.xls) as the cap on its declared Workbook stream size — the CFB path + /// has nothing to decompress, but this is still the caller's budget for what that phase may + /// materialize. public long MaxTotalDecompressedBytes { get; init; } = 512L * 1024 * 1024; /// Gets the maximum byte length allowed for a single cell's value. Defaults to 32 MiB. diff --git a/src/ExcelReader.Core/Reader/IExcelRowReader.cs b/src/ExcelReader.Core/Reader/IExcelRowReader.cs index c6898b6f..633b4bfc 100644 --- a/src/ExcelReader.Core/Reader/IExcelRowReader.cs +++ b/src/ExcelReader.Core/Reader/IExcelRowReader.cs @@ -35,6 +35,13 @@ public interface IExcelRowReader /// plus a sheet-navigation surface and dispose; unifying them lets the typed parser drive a format-agnostic /// reader () and lets callers walk every sheet without /// downcasting to the concrete XlsxReader/XlsbReader/XlsReader type. + /// + /// Thread safety: no implementation is thread-safe. A reader instance carries mutable enumeration + /// state (current sheet, buffer positions, shared-string cache) with no synchronization; concurrent calls + /// from multiple threads — including two enumerators obtained from the same reader used concurrently — + /// produce undefined behavior. Use one reader instance per thread, or fully consume/dispose one enumerator + /// before starting another. + /// /// public interface IExcelRowReader : IExcelRowReader, IDisposable, IAsyncDisposable { diff --git a/src/ExcelReader.Core/Reader/PooledStreamRowEnumerator.cs b/src/ExcelReader.Core/Reader/PooledStreamRowEnumerator.cs index 77ea736b..80feb1d4 100644 --- a/src/ExcelReader.Core/Reader/PooledStreamRowEnumerator.cs +++ b/src/ExcelReader.Core/Reader/PooledStreamRowEnumerator.cs @@ -21,6 +21,14 @@ private protected PooledStreamRowEnumerator(Stream source, int maxCellBytes, str _acc = new CellAccumulator(maxCellBytes, limitName); } + private protected PooledStreamRowEnumerator(ReadOnlyMemory content, int maxCellBytes, string limitName, CancellationToken ct) + { + _source = null; + _ct = ct; + _io = new BufferedStreamCursor(content, maxCellBytes, limitName); + _acc = new CellAccumulator(maxCellBytes, limitName); + } + private protected void Fill() { _io.Fill(_source); diff --git a/src/ExcelReader.Core/Reader/WorkbookStream.cs b/src/ExcelReader.Core/Reader/WorkbookStream.cs index bca092c0..75fdd707 100644 --- a/src/ExcelReader.Core/Reader/WorkbookStream.cs +++ b/src/ExcelReader.Core/Reader/WorkbookStream.cs @@ -1,12 +1,14 @@ using System.Buffers; using System.Diagnostics.CodeAnalysis; +using System.Runtime.InteropServices; namespace ExcelReader.Core.Reader { - // The Workbook OLE stream, read on demand instead of materialized. Two modes: + // The Workbook OLE stream, read on demand instead of materialized. Three modes: // - streamed: a seekable source + the stream's physical FAT-sector chain. Only one sector // (plus a record-assembly scratch) is held at a time, so a 3 MB workbook costs ~KBs. - // - in-memory: a contiguous buffer (mini-stream workbooks, or a non-seekable fallback). + // - contiguous: a contiguous buffer (mini-stream workbooks, or a non-seekable fallback). + // - chained: the whole file in memory plus the workbook's FAT chain. // Immutable and shareable; each consumer reads through its own BiffCursor. [ExcludeFromCodeCoverage(Justification = "Exercised through XlsReader integration tests; guard-rail branches are corrupt-OLE only.")] internal sealed class WorkbookStream : IDisposable @@ -18,30 +20,66 @@ internal sealed class WorkbookStream : IDisposable private readonly int[] _chain; // physical sector numbers, in order (pooled, oversized) private readonly int _chainLength; // valid entry count in _chain; the pooled array is larger private bool _chainReturned; // guards against a double pool-return on repeated Dispose - private readonly ReadOnlyMemory _memory; + + // The in-memory modes' buffer, resolved to its backing array once here rather than kept as a + // ReadOnlyMemory: reads happen per BIFF record, and ReadOnlyMemory.Span is a property that + // has to disambiguate array/MemoryManager/string backing on every access, whereas an array plus + // a base offset slices with plain pointer arithmetic. + private readonly int _fileLength; // usable bytes from _fileBase internal int SectorSize { get; } internal long Length { get; } - private WorkbookStream(Stream? source, bool ownsSource, int[] chain, int chainLength, ReadOnlyMemory memory, int sectorSize, long length) + internal enum SourceKind + { + Streamed, + Contiguous, + Chained, + } + + internal SourceKind Kind { get; } + + private WorkbookStream(Stream? source, bool ownsSource, int[] chain, int chainLength, ReadOnlyMemory memory, int sectorSize, long length, SourceKind kind) { _source = source; _ownsSource = ownsSource; _chain = chain; _chainLength = chainLength; - _memory = memory; SectorSize = sectorSize; Length = length; + Kind = kind; + if (kind == SourceKind.Streamed) + { + Buffer = []; + return; + } + if (MemoryMarshal.TryGetArray(memory, out ArraySegment segment)) + { + Buffer = segment.Array!; + BufferBase = segment.Offset; + _fileLength = segment.Count; + return; + } + // Rare: a non-array-backed ReadOnlyMemory (a custom MemoryManager). One copy + // here keeps every subsequent record read on the array fast path, matching what + // BufferedStreamCursor and XlsCompoundFile.AsStream already do for this case. + Buffer = memory.ToArray(); + _fileLength = Buffer.Length; } internal static WorkbookStream Streamed(Stream source, bool ownsSource, int[] chain, int chainLength, int sectorSize, long length) { - return new WorkbookStream(source, ownsSource, chain, chainLength, default, sectorSize, length); + return new WorkbookStream(source, ownsSource, chain, chainLength, default, sectorSize, length, SourceKind.Streamed); } internal static WorkbookStream InMemory(ReadOnlyMemory data) { - return new WorkbookStream(null, ownsSource: false, [], 0, data, sectorSize: 1, data.Length); + return new WorkbookStream(null, ownsSource: false, [], 0, data, sectorSize: 1, data.Length, SourceKind.Contiguous); + } + + internal static WorkbookStream Chained(ReadOnlyMemory data, int[] chain, int chainLength, int sectorSize, long length) + { + return new WorkbookStream(null, ownsSource: false, chain, chainLength, data, sectorSize, length, SourceKind.Chained); } internal BiffCursor OpenCursor() @@ -49,11 +87,79 @@ internal BiffCursor OpenCursor() return new BiffCursor(this); } - internal bool IsMemory => _source is null; + + // The in-memory buffer and the offset of its first byte, handed to each BiffCursor so the hot + // record path slices the array directly instead of loading them back through this object. + internal byte[] Buffer { get; } + internal int BufferBase { get; } internal ReadOnlySpan Memory(long pos, int len) { - return _memory.Span.Slice((int)pos, len); + return Buffer.AsSpan(BufferBase + (int)pos, len); + } + + // The maximal run of physically consecutive sectors containing `pos`, in logical coordinates + // plus the run's byte offset into the buffer. BiffCursor caches this, so records inside one run + // translate with a compare and pointer arithmetic instead of a per-record chain walk — and + // Excel writes the Workbook stream as a single sequential run, so in practice one resolve + // covers the whole file. Walking both directions keeps a backward seek (the enumerator rewinds + // a record when a row ends) inside the cached run rather than re-resolving. + internal void ResolveChainedRun(long pos, out long runStart, out long runEnd, out long bufferOffset) + { + int chainIndex = (int)(pos / SectorSize); + RequireChainIndex(chainIndex); + int first = chainIndex; + while (first > 0 && _chain[first] == _chain[first - 1] + 1) + { + first--; + } + int last = chainIndex; + while (last + 1 < _chainLength && _chain[last + 1] == _chain[last] + 1) + { + last++; + } + runStart = (long)first * SectorSize; + runEnd = Math.Min(Length, (long)(last + 1) * SectorSize); + bufferOffset = HeaderSize + ((long)_chain[first] * SectorSize); + // Trust boundary: the chain comes from untrusted bytes, so the whole run is range-checked + // once here. Every cached-run read afterwards is provably inside the buffer. + if (_chain[first] < 0 || bufferOffset < 0 || bufferOffset + (runEnd - runStart) > _fileLength) + { + throw new InvalidDataException("The OLE sector chain points past the end of the buffer."); + } + } + + internal void CopyChained(long pos, Span dest) + { + int written = 0; + while (written < dest.Length) + { + long at = pos + written; + int chainIndex = (int)(at / SectorSize); + int within = (int)(at % SectorSize); + RequireChainIndex(chainIndex); + int take = Math.Min(SectorSize - within, dest.Length - written); + FileSlice(_chain[chainIndex], within, take).CopyTo(dest[written..]); + written += take; + } + } + + private void RequireChainIndex(int chainIndex) + { + if ((uint)chainIndex >= (uint)_chainLength) + { + throw new InvalidDataException("Invalid OLE sector chain index."); + } + } + + private ReadOnlySpan FileSlice(int sector, int within, int len) + { + long offset = HeaderSize + ((long)sector * SectorSize) + within; + if (sector < 0 || offset < 0 || offset + len > _fileLength) + { + throw new InvalidDataException("The OLE sector chain points past the end of the buffer."); + } + return Buffer.AsSpan(BufferBase + (int)offset, len); } // Reads contiguous physical sectors starting from chainIndex into dest. diff --git a/src/ExcelReader.Core/Reader/XlsCompoundFile.cs b/src/ExcelReader.Core/Reader/XlsCompoundFile.cs index 65b432b7..db69aa87 100644 --- a/src/ExcelReader.Core/Reader/XlsCompoundFile.cs +++ b/src/ExcelReader.Core/Reader/XlsCompoundFile.cs @@ -2,6 +2,7 @@ using System.Buffers.Binary; using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using static ExcelReader.Core.Reader.Biff12; namespace ExcelReader.Core.Reader @@ -19,12 +20,12 @@ internal static class XlsCompoundFile private const int FreeSector = unchecked((int)0xFFFFFFFF); internal static ReadOnlySpan Signature => [0xD0, 0xCF, 0x11, 0xE0, 0xA1, 0xB1, 0x1A, 0xE1]; - internal static WorkbookStream OpenWorkbook(Stream stream, bool leaveOpen) + internal static WorkbookStream OpenWorkbook(Stream stream, bool leaveOpen, ExcelReaderOptions? options = null) { (Stream source, bool ownsSource) = EnsureSeekable(stream, leaveOpen); try { - return BuildWorkbook(source, ownsSource); + return BuildWorkbook(source, ownsSource, options ?? ExcelReaderOptions.Default); } catch { @@ -36,12 +37,18 @@ internal static WorkbookStream OpenWorkbook(Stream stream, bool leaveOpen) } } - internal static async ValueTask OpenWorkbookAsync(Stream stream, bool leaveOpen, CancellationToken ct) + internal static WorkbookStream OpenWorkbook(ReadOnlyMemory data, ExcelReaderOptions? options = null) + { + using MemoryStream metadata = AsStream(data); + return BuildWorkbook(metadata, ownsSource: false, options ?? ExcelReaderOptions.Default, memory: data); + } + + internal static async ValueTask OpenWorkbookAsync(Stream stream, bool leaveOpen, ExcelReaderOptions? options, CancellationToken ct) { (Stream source, bool ownsSource) = await EnsureSeekableAsync(stream, leaveOpen, ct).ConfigureAwait(false); try { - return BuildWorkbook(source, ownsSource); + return BuildWorkbook(source, ownsSource, options ?? ExcelReaderOptions.Default); } catch { @@ -85,8 +92,17 @@ private static (Stream Source, bool OwnsSource) EnsureSeekable(Stream stream, bo return (ms, true); } + internal static MemoryStream AsStream(ReadOnlyMemory data) + { + if (MemoryMarshal.TryGetArray(data, out ArraySegment segment)) + { + return new MemoryStream(segment.Array!, segment.Offset, segment.Count, writable: false); + } + return new MemoryStream(data.ToArray(), writable: false); + } + [SkipLocalsInit] - private static WorkbookStream BuildWorkbook(Stream source, bool ownsSource) + private static WorkbookStream BuildWorkbook(Stream source, bool ownsSource, ExcelReaderOptions options, ReadOnlyMemory memory = default) { if (source.Length < HeaderSize) { @@ -113,6 +129,13 @@ private static WorkbookStream BuildWorkbook(Stream source, bool ownsSource) { throw new InvalidDataException("Unsupported OLE sector size."); } + // MS-CFB fixes the mini-stream cutoff at 4096 bytes. Without this bound a crafted header + // could push miniCutoff toward int.MaxValue, letting the mini-stream branch below take a + // multi-GB workbook.Size and materialize it as a single non-pooled byte[]. + if (miniCutoff != 4096) + { + throw new InvalidDataException("Unsupported OLE mini stream cutoff."); + } // A file cannot hold more sectors than its length allows, so a FAT/DIFAT sector count above // that is a crafted header. Reject it before allocating, or `new int[fatSectorCount]` below // would let a bogus count force a multi-GB allocation / OOM on untrusted input. @@ -137,12 +160,31 @@ private static WorkbookStream BuildWorkbook(Stream source, bool ownsSource) DirectoryEntry workbook = FindWorkbook(entries); + // A stream cannot hold more content than the container's own byte length, so an + // inflated Size field (the same attack class as fatSectorCount/difatSectorCount above) + // is a crafted header — reject it before it drives an allocation or a chain walk sized + // off it. The caller's byte budget applies here too, since this is the one choke point + // both the mini-stream and chained/streamed branches below pass through. + if (workbook.Size < 0 || workbook.Size > source.Length) + { + throw new InvalidDataException("The OLE Workbook stream size exceeds the container."); + } + LimitChecks.ThrowIfEntryLengthExceeds(workbook.Size, options.MaxTotalDecompressedBytes, nameof(ExcelReaderOptions.MaxTotalDecompressedBytes)); + // Mini-stream workbooks (tiny, rare) are materialized; everything else streams. if (workbook.Size < miniCutoff && workbook.StartSector >= 0) { int[] miniFat = firstMiniFatSector >= 0 && miniFatSectorCount > 0 ? ReadIntSectors(source, sectorSize, fat, firstMiniFatSector, miniFatSectorCount) : []; + // entries[0].Size (the root storage entry's mini-stream length) is a long; a value + // above int.MaxValue would truncate through the (int) cast into a negative byteLimit, + // which ReadChainBytes interprets as "read the entire chain" instead of "read N bytes" — + // bounded safely by the cycle check below, but a silent semantic flip worth closing. + if (entries[0].Size > int.MaxValue) + { + throw new InvalidDataException("The OLE root entry size exceeds the container."); + } byte[] miniStream = entries[0].StartSector >= 0 && entries[0].Size > 0 ? ReadChainBytes(source, sectorSize, fat, entries[0].StartSector, (int)entries[0].Size) : []; @@ -162,6 +204,10 @@ private static WorkbookStream BuildWorkbook(Stream source, bool ownsSource) int chainCount = SectorCount(workbook.Size, sectorSize); int[] chain = BuildChain(fat, workbook.StartSector, chainCount); + if (!memory.IsEmpty) + { + return WorkbookStream.Chained(memory, chain, chainCount, sectorSize, workbook.Size); + } return WorkbookStream.Streamed(source, ownsSource, chain, chainCount, sectorSize, workbook.Size); } finally @@ -186,7 +232,7 @@ private static DirectoryEntry FindWorkbook(ReadOnlySpan entries) private static int SectorCount(long size, int sectorSize) { - return (int)((size + sectorSize - 1) / sectorSize); + return checked((int)((size + sectorSize - 1) / sectorSize)); } [SuppressMessage("Performance", "HLQ013:Consider using 'foreach' loop instead of 'for' loop", @@ -196,17 +242,25 @@ private static int SectorCount(long size, int sectorSize) private static int[] BuildChain(ReadOnlySpan fat, int startSector, int sectorCount) { int[] chain = ArrayPool.Shared.Rent(sectorCount); - int sector = startSector; - for (int i = 0; i < sectorCount; i++) + try { - if (sector is < 0 or EndOfChain) + int sector = startSector; + for (int i = 0; i < sectorCount; i++) { - throw new InvalidDataException("The OLE Workbook chain ended early."); + if (sector is < 0 or EndOfChain) + { + throw new InvalidDataException("The OLE Workbook chain ended early."); + } + chain[i] = sector; + sector = NextSector(fat, sector); } - chain[i] = sector; - sector = NextSector(fat, sector); + return chain; + } + catch + { + ArrayPool.Shared.Return(chain); + throw; } - return chain; } private static void ReadAt(Stream source, long offset, Span dest) @@ -331,6 +385,10 @@ private static int[] ReadIntSectors(Stream source, int sectorSize, ReadOnlySpan< private static byte[] ReadMiniStream(ReadOnlySpan miniStream, ReadOnlySpan miniFat, int miniSectorSize, int startSector, int size) { + if (size < 0 || size > miniStream.Length) + { + throw new InvalidDataException("Invalid OLE mini stream size."); + } byte[] result = new byte[size]; int sector = startSector; int written = 0; diff --git a/src/ExcelReader.Core/Reader/XlsReader.cs b/src/ExcelReader.Core/Reader/XlsReader.cs index 02dda23b..63e66abb 100644 --- a/src/ExcelReader.Core/Reader/XlsReader.cs +++ b/src/ExcelReader.Core/Reader/XlsReader.cs @@ -22,7 +22,12 @@ public sealed partial class XlsReader : IExcelRowReader, IExcelRowReader data, ExcelReaderOptions? options = null) + : this(XlsCompoundFile.OpenWorkbook(data, options), options) { } @@ -45,7 +50,7 @@ private XlsReader(WorkbookStream workbook, ExcelReaderOptions? options = null) internal static async ValueTask CreateAsync(Stream stream, bool leaveOpen, ExcelReaderOptions? options = null, CancellationToken ct = default) { - WorkbookStream workbook = await XlsCompoundFile.OpenWorkbookAsync(stream, leaveOpen, ct).ConfigureAwait(false); + WorkbookStream workbook = await XlsCompoundFile.OpenWorkbookAsync(stream, leaveOpen, options, ct).ConfigureAwait(false); return new XlsReader(workbook, options); } diff --git a/src/ExcelReader.Core/Reader/XlsbReader.Enumerator.cs b/src/ExcelReader.Core/Reader/XlsbReader.Enumerator.cs index 2b5fe04d..d265275f 100644 --- a/src/ExcelReader.Core/Reader/XlsbReader.Enumerator.cs +++ b/src/ExcelReader.Core/Reader/XlsbReader.Enumerator.cs @@ -33,8 +33,8 @@ public sealed class Enumerator : PooledStreamRowEnumerator, IExcelRowEnumerator // On the next MoveNext call, skip the "seek to row header" step. private bool _pendingRowHdr; - // Also used by the in-memory ZIP path (docs/in-memory-zip.md): ZipMemoryIndex.OpenEntryStream - // hands back a DeflateStream/MemoryStream over the part's bytes, same as the ZipArchive path. + // Also used by the in-memory ZIP path: ZipMemoryIndex.OpenEntryStream hands back a + // DeflateStream/MemoryStream over the part's bytes, same as the ZipArchive path. internal Enumerator(XlsbReader reader, Stream sheet, long entryLength = 0, CancellationToken ct = default) : base(sheet, reader._options.MaxCellBytes, nameof(ExcelReaderOptions.MaxCellBytes), WorkbookLookups.InitialBufferCapacity(entryLength), ct) { diff --git a/src/ExcelReader.Core/Reader/XlsbReader.Memory.cs b/src/ExcelReader.Core/Reader/XlsbReader.Memory.cs index 16e36a76..1796867c 100644 --- a/src/ExcelReader.Core/Reader/XlsbReader.Memory.cs +++ b/src/ExcelReader.Core/Reader/XlsbReader.Memory.cs @@ -2,7 +2,7 @@ namespace ExcelReader.Core.Reader { - // In-memory ZIP path (docs/in-memory-zip.md, Z4): opens an XlsbReader directly over a + // In-memory ZIP path: opens an XlsbReader directly over a // ReadOnlyMemory via ZipMemoryIndex instead of ZipArchive/Stream. No refills, no async // suspension — every part is already fully decompressed before the reader is constructed. public sealed partial class XlsbReader diff --git a/src/ExcelReader.Core/Reader/XlsbReader.cs b/src/ExcelReader.Core/Reader/XlsbReader.cs index ddef9a48..5107466a 100644 --- a/src/ExcelReader.Core/Reader/XlsbReader.cs +++ b/src/ExcelReader.Core/Reader/XlsbReader.cs @@ -1,7 +1,7 @@ using System.Buffers; using System.Diagnostics.CodeAnalysis; using System.IO.Compression; -using ExcelReader.Core.Writer.Internal; +using ExcelReader.Core.Internal; namespace ExcelReader.Core.Reader { @@ -28,8 +28,8 @@ public sealed partial class XlsbReader : IExcelRowReader, IExcelRowReader VClose => _ns is null ? ""u8 : _ns.VClose; private ReadOnlySpan CClose => _ns is null ? ""u8 : _ns.CClose; - // Also used by the in-memory ZIP path (docs/in-memory-zip.md): ZipMemoryIndex.OpenEntryStream - // hands back a DeflateStream/MemoryStream over the part's bytes, same as the ZipArchive path. + // Also used by the in-memory ZIP path: ZipMemoryIndex.OpenEntryStream hands back a + // DeflateStream/MemoryStream over the part's bytes, same as the ZipArchive path. internal Enumerator(XlsxReader reader, Stream sheet, long entryLength = 0, CancellationToken ct = default) : base(sheet, reader._options.MaxCellBytes, nameof(ExcelReaderOptions.MaxCellBytes), WorkbookLookups.InitialBufferCapacity(entryLength), ct) { diff --git a/src/ExcelReader.Core/Reader/XlsxReader.Memory.cs b/src/ExcelReader.Core/Reader/XlsxReader.Memory.cs index 049fa219..21dbd7da 100644 --- a/src/ExcelReader.Core/Reader/XlsxReader.Memory.cs +++ b/src/ExcelReader.Core/Reader/XlsxReader.Memory.cs @@ -3,7 +3,7 @@ namespace ExcelReader.Core.Reader { - // In-memory ZIP path (docs/in-memory-zip.md, Z4): opens an XlsxReader directly over a + // In-memory ZIP path: opens an XlsxReader directly over a // ReadOnlyMemory via ZipMemoryIndex instead of ZipArchive/Stream. No refills, no async // suspension — every part is already fully decompressed before the reader is constructed. public sealed partial class XlsxReader diff --git a/src/ExcelReader.Core/Reader/XlsxReader.cs b/src/ExcelReader.Core/Reader/XlsxReader.cs index e3e392d3..e31f4f3f 100644 --- a/src/ExcelReader.Core/Reader/XlsxReader.cs +++ b/src/ExcelReader.Core/Reader/XlsxReader.cs @@ -1,7 +1,7 @@ using System.Buffers; using System.Diagnostics.CodeAnalysis; using System.IO.Compression; -using ExcelReader.Core.Writer.Internal; +using ExcelReader.Core.Internal; namespace ExcelReader.Core.Reader { @@ -13,8 +13,8 @@ public sealed partial class XlsxReader : IExcelRowReader, IExcelRowReader src, Span dest) // used as the literal fallback when the codepoint is malformed. private static int DecodeNumeric(ReadOnlySpan body, Span dest, ReadOnlySpan raw) { + // Bails once cp exceeds the highest valid codepoint rather than accumulating unchecked: + // an overlong run (e.g. "�") would otherwise wrap the int silently + // into a different, unrelated valid codepoint instead of being rejected as malformed. + const int MaxCodepoint = 0x10FFFF; int cp = 0; bool ok = false; if (body.Length > 0 && (body[0] == 'x' || body[0] == 'X')) @@ -143,7 +147,7 @@ private static int DecodeNumeric(ReadOnlySpan body, Span dest, ReadO foreach (ref readonly byte d in body[1..]) { int v = HexVal(d); - if (v < 0) { ok = false; break; } + if (v < 0 || cp > MaxCodepoint) { ok = false; break; } cp = (cp * 16) + v; ok = true; } @@ -152,7 +156,7 @@ private static int DecodeNumeric(ReadOnlySpan body, Span dest, ReadO { foreach (ref readonly byte d in body) { - if (d is < (byte)'0' or > (byte)'9') { ok = false; break; } + if (d is < (byte)'0' or > (byte)'9' || cp > MaxCodepoint) { ok = false; break; } cp = (cp * 10) + (d - '0'); ok = true; } diff --git a/src/ExcelReader.Core/Reader/ZipMemoryIndex.cs b/src/ExcelReader.Core/Reader/ZipMemoryIndex.cs index 11c1c54c..6b044b38 100644 --- a/src/ExcelReader.Core/Reader/ZipMemoryIndex.cs +++ b/src/ExcelReader.Core/Reader/ZipMemoryIndex.cs @@ -147,7 +147,7 @@ internal ZipPart OpenPart(in ZipEntryRef entry, DecompressedByteCounter counter, return part; } - // In-memory ZIP path's worksheet entry point (docs/in-memory-zip.md): opens a Stream over the + // In-memory ZIP path's worksheet entry point: opens a Stream over the // entry's compressed bytes instead of eagerly materializing the whole part, so the caller (the // XlsxReader/XlsbReader enumerator) can reuse the exact same PrefetchStream/LimitedReadStream // pipeline as the ZipArchive path (WorkbookLookups.Wrap) and overlap inflate with row parsing. @@ -443,8 +443,9 @@ private long ResolveDataOffset(in ZipEntryRef entry) return headerOffset + LocalHeaderFixedSize + nameLength + extraLength; } - // Phase 1 targets stdlib DeflateStream (docs/in-memory-zip.md, Z1); Phase 2 (D1-D3) replaces - // this with a one-shot span-based inflater and removes the array-backing requirement below. + // This is a known stopgap: it round-trips through stdlib DeflateStream, which needs an + // array-backed ReadOnlyMemory (or a one-time copy). A zero-allocation span-based + // inflater would remove that, but is a separate, larger, not-yet-started follow-up. private static ZipPart InflateToPart(ReadOnlyMemory compressed, long uncompressedSize) { int size = checked((int)uncompressedSize); diff --git a/src/ExcelReader.Core/Reader/ZipReaderOpen.cs b/src/ExcelReader.Core/Reader/ZipReaderOpen.cs index 1999576b..731005b3 100644 --- a/src/ExcelReader.Core/Reader/ZipReaderOpen.cs +++ b/src/ExcelReader.Core/Reader/ZipReaderOpen.cs @@ -1,6 +1,6 @@ using System.Diagnostics.CodeAnalysis; using System.IO.Compression; -using ExcelReader.Core.Writer.Internal; +using ExcelReader.Core.Internal; namespace ExcelReader.Core.Reader { diff --git a/src/ExcelReader.Core/ValueObjects/RowCellEnumerator.cs b/src/ExcelReader.Core/ValueObjects/RowCellEnumerator.cs index d28486d8..4d0ef751 100644 --- a/src/ExcelReader.Core/ValueObjects/RowCellEnumerator.cs +++ b/src/ExcelReader.Core/ValueObjects/RowCellEnumerator.cs @@ -3,7 +3,7 @@ namespace ExcelReader.Core.ValueObjects /// /// Enumerates the populated cells of a , in ascending column order, skipping gaps. /// Supports foreach via the duck-typed enumerator pattern (ref structs cannot implement - /// ). + /// ). /// public ref struct RowCellEnumerator { diff --git a/src/ExcelReader.Core/Writer/IWorkbookWriter.cs b/src/ExcelReader.Core/Writer/IWorkbookWriter.cs index 6f4649d1..e5f71e0d 100644 --- a/src/ExcelReader.Core/Writer/IWorkbookWriter.cs +++ b/src/ExcelReader.Core/Writer/IWorkbookWriter.cs @@ -7,6 +7,12 @@ namespace ExcelReader.Core.Writer /// (or call then dispose) when finished. /// /// The concrete this workbook produces. + /// + /// Thread safety: no implementation is thread-safe, nor are the / + /// instances it hands out. All of them carry mutable state (current + /// sheet/row, buffered output) with no synchronization. Use one workbook writer per thread; do not + /// call into a writer, its current sheet, or its current row from more than one thread at a time. + /// public interface IWorkbookWriter : IAsyncDisposable { /// diff --git a/src/ExcelReader.Core/Writer/Internal/WriterStateGuard.cs b/src/ExcelReader.Core/Writer/Internal/WriterStateGuard.cs index 17669129..68a25c14 100644 --- a/src/ExcelReader.Core/Writer/Internal/WriterStateGuard.cs +++ b/src/ExcelReader.Core/Writer/Internal/WriterStateGuard.cs @@ -1,5 +1,3 @@ -using System.IO.Compression; - namespace ExcelReader.Core.Writer.Internal { // Shared WriterState guard checks for the three state-tracking workbook writers (XlsxWorkbookWriter, @@ -60,22 +58,4 @@ internal static void ValidateSheetName(string name) } } } - - // The "#if NET10_0_OR_GREATER await zip.DisposeAsync() #else zip.Dispose()" idiom, shared by the - // two ZIP-backed writers (XlsxWorkbookWriter, XlsbWorkbookWriter) across their EndAsync/DisposeAsync - // paths. - internal static class ZipArchiveDisposal - { - [System.Diagnostics.CodeAnalysis.SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP007:Don't dispose injected", - Justification = "Disposal helper: disposing the caller-owned ZipArchive is its sole purpose — the two ZIP-backed writers delegate their own zip's disposal here.")] - internal static ValueTask DisposeAsync(ZipArchive zip) - { -#if NET10_0_OR_GREATER - return zip.DisposeAsync(); -#else - zip.Dispose(); - return ValueTask.CompletedTask; -#endif - } - } } diff --git a/src/ExcelReader.Core/Writer/XlsbWorkbookWriter.cs b/src/ExcelReader.Core/Writer/XlsbWorkbookWriter.cs index 46ee8497..155f84b9 100644 --- a/src/ExcelReader.Core/Writer/XlsbWorkbookWriter.cs +++ b/src/ExcelReader.Core/Writer/XlsbWorkbookWriter.cs @@ -4,6 +4,7 @@ using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Text; +using ExcelReader.Core.Internal; using ExcelReader.Core.Reader; using ExcelReader.Core.Writer.Internal; diff --git a/src/ExcelReader.Core/Writer/XlsxWorkbookWriter.cs b/src/ExcelReader.Core/Writer/XlsxWorkbookWriter.cs index a8106818..4654fd81 100644 --- a/src/ExcelReader.Core/Writer/XlsxWorkbookWriter.cs +++ b/src/ExcelReader.Core/Writer/XlsxWorkbookWriter.cs @@ -3,6 +3,7 @@ using System.IO.Compression; using System.Security; using System.Text; +using ExcelReader.Core.Internal; using ExcelReader.Core.Writer.Internal; namespace ExcelReader.Core.Writer diff --git a/tests/ExcelReader.Benchmarks/BenchmarkAccumulators.cs b/tests/ExcelReader.Benchmarks/BenchmarkAccumulators.cs index 23323f1e..1fe85485 100644 --- a/tests/ExcelReader.Benchmarks/BenchmarkAccumulators.cs +++ b/tests/ExcelReader.Benchmarks/BenchmarkAccumulators.cs @@ -31,6 +31,32 @@ internal static long AccumulateRow(Row row) return acc; } + // Same shape as AccumulateRow, but calls Cell.GetString() for text — matching the allocation + // Sylvan's ADO.NET-style GetString(i) is forced to pay on its side. This is the fair, + // matched-work counterpart to AccumulateRow's zero-copy span read (see the README's + // "Benchmark methodology" note). + internal static long AccumulateRowMaterialized(Row row) + { + long acc = 0; + foreach (RowCell rowCell in row.Cells) + { + Cell cell = rowCell.Value; + switch (cell.Type) + { + case CellType.ExcelString: + acc += cell.GetString().Length; + break; + case CellType.Number: + if (cell.TryParse(null, out double n)) { acc += (long)n; } + break; + case CellType.Date: + if (cell.TryGetDateTime(out DateTime d)) { acc += d.Ticks; } + break; + } + } + return acc; + } + internal static long AccumulateSylvanExcel(ExcelDataReader reader) { long acc = 0; diff --git a/tests/ExcelReader.Benchmarks/ReadBenchmark.cs b/tests/ExcelReader.Benchmarks/ReadBenchmark.cs index bd677106..52fe93bb 100644 --- a/tests/ExcelReader.Benchmarks/ReadBenchmark.cs +++ b/tests/ExcelReader.Benchmarks/ReadBenchmark.cs @@ -3,6 +3,7 @@ using ExcelReader.Core.Reader; using MiniExcelLibs; using Sylvan.Data.Excel; +using static ExcelReader.Benchmarks.BenchmarkAccumulators; namespace ExcelReader.Benchmarks { @@ -149,6 +150,19 @@ public async Task ExcelReaderXlsbAsync() return acc; } + // Matched-work counterpart to ExcelReader: materializes a string per cell like Sylvan's + // GetString below is forced to, instead of reading the zero-copy span (see the README's + // "Benchmark methodology" note). + [Benchmark] + public long ExcelReaderMaterialized() + { + using var ms = new MemoryStream(_workbook, writable: false); + using var reader = Excel.From(ms); + long acc = 0; + foreach (var row in reader) { acc += AccumulateRowMaterialized(row); } + return acc; + } + [Benchmark] public long MiniExcel() { diff --git a/tests/ExcelReader.Benchmarks/RealDataReadBenchmark.cs b/tests/ExcelReader.Benchmarks/RealDataReadBenchmark.cs index 72d8ea90..19029e09 100644 --- a/tests/ExcelReader.Benchmarks/RealDataReadBenchmark.cs +++ b/tests/ExcelReader.Benchmarks/RealDataReadBenchmark.cs @@ -58,6 +58,19 @@ public long Xlsx_Sylvan() return AccumulateSylvanExcel(reader); } + // Matched-work counterpart to Xlsx_ExcelReader: materializes a string per cell like + // Xlsx_Sylvan is forced to, instead of reading the zero-copy span (see the README's + // "Benchmark methodology" note). + [Benchmark] + public long Xlsx_ExcelReader_Materialized() + { + using MemoryStream ms = new(_xlsx, writable: false); + using XlsxReader reader = Excel.From(ms); + long acc = 0; + foreach (Row row in reader) { acc += AccumulateRowMaterialized(row); } + return acc; + } + [Benchmark] public long Xlsx_ExcelReader_Prefetch() { @@ -68,8 +81,8 @@ public long Xlsx_ExcelReader_Prefetch() return acc; } - // In-memory ZIP path (docs/in-memory-zip.md): no ZipArchive/Stream, central directory read - // directly out of _xlsx. Phase 1 gate — compare against Xlsx_ExcelReader before starting D1. + // In-memory ZIP path: no ZipArchive/Stream, central directory read + // directly out of _xlsx. Compare against Xlsx_ExcelReader to see the memory path's overhead. [Benchmark] public long Xlsx_ExcelReader_Memory() { @@ -108,6 +121,17 @@ public long Xlsm_Sylvan() return AccumulateSylvanExcel(reader); } + // Matched-work counterpart to Xlsm_ExcelReader — see Xlsx_ExcelReader_Materialized. + [Benchmark] + public long Xlsm_ExcelReader_Materialized() + { + using MemoryStream ms = new(_xlsm, writable: false); + using XlsxReader reader = Excel.From(ms); + long acc = 0; + foreach (Row row in reader) { acc += AccumulateRowMaterialized(row); } + return acc; + } + [Benchmark] public long Xlsm_ExcelReader_Prefetch() { @@ -156,6 +180,17 @@ public long Xlsb_Sylvan() return AccumulateSylvanExcel(reader); } + // Matched-work counterpart to Xlsb_ExcelReader — see Xlsx_ExcelReader_Materialized. + [Benchmark] + public long Xlsb_ExcelReader_Materialized() + { + using MemoryStream ms = new(_xlsb, writable: false); + using XlsbReader reader = Excel.FromXlsb(ms); + long acc = 0; + foreach (Row row in reader) { acc += AccumulateRowMaterialized(row); } + return acc; + } + [Benchmark] public long Xlsb_ExcelReader_Prefetch() { @@ -204,8 +239,30 @@ public long Xls_Sylvan() return AccumulateSylvanExcel(reader); } - // --- CSV --- (CSV cells are always plain text on both sides — no style-driven date/number - // typing is possible — so both benchmarks just sum text length for a like-for-like comparison.) + // Matched-work counterpart to Xls_ExcelReader — see Xlsx_ExcelReader_Materialized. + [Benchmark] + public long Xls_ExcelReader_Materialized() + { + using MemoryStream ms = new(_xls, writable: false); + using XlsReader reader = Excel.FromXls(ms); + long acc = 0; + foreach (Row row in reader) { acc += AccumulateRowMaterialized(row); } + return acc; + } + + [Benchmark] + public long Xls_ExcelReader_Memory() + { + using XlsReader reader = Excel.FromXls(_xls.AsMemory()); + long acc = 0; + foreach (Row row in reader) { acc += AccumulateRow(row); } + return acc; + } + + // --- CSV --- (CSV cells are always plain text on both sides, so no style-driven date/number + // typing is possible. The ExcelReader benchmark below reads the raw UTF-8 span with no decode + // or allocation, while the Sylvan side is forced to materialize a UTF-16 string, so these two + // are not matched work. The materialized benchmark further down is the fair counterpart.) [Benchmark] public long Csv_ExcelReader() @@ -240,5 +297,38 @@ public long Csv_Sylvan() } return acc; } + + // Matched-work counterpart to Csv_ExcelReader: materializes a string per cell like + // Csv_Sylvan's GetString(i) is forced to, instead of reading the zero-copy span. + [Benchmark] + public long Csv_ExcelReader_Materialized() + { + using MemoryStream ms = new(_csv, writable: false); + using CsvReader reader = Excel.FromCsv(ms); + long acc = 0; + foreach (Row row in reader) + { + foreach (RowCell rowCell in row.Cells) + { + acc += rowCell.Value.GetString().Length; + } + } + return acc; + } + + [Benchmark] + public long Csv_ExcelReader_Memory() + { + using CsvReader reader = Excel.FromCsv(_csv.AsMemory()); + long acc = 0; + foreach (Row row in reader) + { + foreach (RowCell rowCell in row.Cells) + { + acc += rowCell.Value.Value.Length; + } + } + return acc; + } } } diff --git a/tests/ExcelReader.Benchmarks/StringHeavyReadBenchmark.cs b/tests/ExcelReader.Benchmarks/StringHeavyReadBenchmark.cs index ba0064af..5b751dec 100644 --- a/tests/ExcelReader.Benchmarks/StringHeavyReadBenchmark.cs +++ b/tests/ExcelReader.Benchmarks/StringHeavyReadBenchmark.cs @@ -26,24 +26,41 @@ public class StringHeavyReadBenchmark // BenchmarkDotNet runs each [Benchmark] in its own process, so an unsplit setup would rebuild // both 65K-row fixtures for every method — doubling suite wall time to build one it never reads. - [GlobalSetup(Targets = [nameof(Xlsx_ExcelReader), nameof(Xlsx_ExcelReader_Prefetch), nameof(Xlsx_Sylvan)])] + // Every new [Benchmark] MUST be added to the matching Targets list: only the setup whose Targets + // name the running method executes, so an unlisted method reads a never-built empty fixture. + // Open() below is what turns that mistake into a readable failure instead of a silent one. + [GlobalSetup(Targets = [nameof(Xlsx_ExcelReader), nameof(Xlsx_ExcelReader_Prefetch), nameof(Xlsx_Sylvan), nameof(Xlsx_ExcelReader_Materialized)])] public async Task SetupXlsxAsync() { _xlsx = await StringHeavyWorkbookGenerator.BuildXlsxAsync(Rows); } - [GlobalSetup(Targets = [nameof(Xlsb_ExcelReader), nameof(Xlsb_ExcelReader_Prefetch), nameof(Xlsb_Sylvan)])] + [GlobalSetup(Targets = [nameof(Xlsb_ExcelReader), nameof(Xlsb_ExcelReader_Prefetch), nameof(Xlsb_Sylvan), nameof(Xlsb_ExcelReader_Materialized)])] public async Task SetupXlsbAsync() { _xlsb = await StringHeavyWorkbookGenerator.BuildXlsbAsync(Rows); } + // An empty fixture means the benchmark was never registered in a Targets list above. ZipArchive + // happens to surface that as "Central Directory corrupt", but a format that tolerates a + // zero-length input would instead measure no work at all and publish a meaningless number — + // so fail loudly here rather than trusting each reader to reject it. + private static MemoryStream Open(byte[] fixture, string benchmark) + { + if (fixture.Length == 0) + { + throw new InvalidOperationException( + $"{benchmark} is absent from every [GlobalSetup(Targets = ...)] list, so its fixture was never built."); + } + return new MemoryStream(fixture, writable: false); + } + // --- XLSX --- [Benchmark(Baseline = true)] public long Xlsx_ExcelReader() { - using MemoryStream ms = new(_xlsx, writable: false); + using MemoryStream ms = Open(_xlsx, nameof(Xlsx_ExcelReader)); using XlsxReader reader = Excel.From(ms); long acc = 0; foreach (Row row in reader) { acc += AccumulateRow(row); } @@ -53,7 +70,7 @@ public long Xlsx_ExcelReader() [Benchmark] public long Xlsx_ExcelReader_Prefetch() { - using MemoryStream ms = new(_xlsx, writable: false); + using MemoryStream ms = Open(_xlsx, nameof(Xlsx_ExcelReader_Prefetch)); using XlsxReader reader = Excel.From(ms, options: _prefetchOptions); long acc = 0; foreach (Row row in reader) { acc += AccumulateRow(row); } @@ -63,17 +80,29 @@ public long Xlsx_ExcelReader_Prefetch() [Benchmark] public long Xlsx_Sylvan() { - using MemoryStream ms = new(_xlsx, writable: false); + using MemoryStream ms = Open(_xlsx, nameof(Xlsx_Sylvan)); using ExcelDataReader reader = ExcelDataReader.Create(ms, ExcelWorkbookType.ExcelXml, new ExcelDataReaderOptions()); return AccumulateSylvanExcel(reader); } + // Matched-work counterpart to Xlsx_ExcelReader: materializes a string per cell like + // Xlsx_Sylvan is forced to, instead of reading the zero-copy span. + [Benchmark] + public long Xlsx_ExcelReader_Materialized() + { + using MemoryStream ms = Open(_xlsx, nameof(Xlsx_ExcelReader_Materialized)); + using XlsxReader reader = Excel.From(ms); + long acc = 0; + foreach (Row row in reader) { acc += AccumulateRowMaterialized(row); } + return acc; + } + // --- XLSB --- [Benchmark] public long Xlsb_ExcelReader() { - using MemoryStream ms = new(_xlsb, writable: false); + using MemoryStream ms = Open(_xlsb, nameof(Xlsb_ExcelReader)); using XlsbReader reader = Excel.FromXlsb(ms); long acc = 0; foreach (Row row in reader) { acc += AccumulateRow(row); } @@ -83,7 +112,7 @@ public long Xlsb_ExcelReader() [Benchmark] public long Xlsb_ExcelReader_Prefetch() { - using MemoryStream ms = new(_xlsb, writable: false); + using MemoryStream ms = Open(_xlsb, nameof(Xlsb_ExcelReader_Prefetch)); using XlsbReader reader = Excel.FromXlsb(ms, options: _prefetchOptions); long acc = 0; foreach (Row row in reader) { acc += AccumulateRow(row); } @@ -93,9 +122,20 @@ public long Xlsb_ExcelReader_Prefetch() [Benchmark] public long Xlsb_Sylvan() { - using MemoryStream ms = new(_xlsb, writable: false); + using MemoryStream ms = Open(_xlsb, nameof(Xlsb_Sylvan)); using ExcelDataReader reader = ExcelDataReader.Create(ms, ExcelWorkbookType.ExcelBinary, new ExcelDataReaderOptions()); return AccumulateSylvanExcel(reader); } + + // Matched-work counterpart to Xlsb_ExcelReader — see Xlsx_ExcelReader_Materialized. + [Benchmark] + public long Xlsb_ExcelReader_Materialized() + { + using MemoryStream ms = Open(_xlsb, nameof(Xlsb_ExcelReader_Materialized)); + using XlsbReader reader = Excel.FromXlsb(ms); + long acc = 0; + foreach (Row row in reader) { acc += AccumulateRowMaterialized(row); } + return acc; + } } } diff --git a/tests/ExcelReader.Tests/BufferedStreamCursorTests.cs b/tests/ExcelReader.Tests/BufferedStreamCursorTests.cs index e284fb89..81e8ade4 100644 --- a/tests/ExcelReader.Tests/BufferedStreamCursorTests.cs +++ b/tests/ExcelReader.Tests/BufferedStreamCursorTests.cs @@ -5,8 +5,8 @@ namespace ExcelReader.Tests { - // Z3 in docs/in-memory-zip.md: the memory-backed BufferedStreamCursor ctor used by the in-memory - // ZIP path once a ZipPart is fully decompressed. No source stream, no refills — these tests cover + // The memory-backed BufferedStreamCursor ctor used by the in-memory ZIP path once a ZipPart is + // fully decompressed. No source stream, no refills — these tests cover // the ctor's aliasing/offset behavior and the one sharp edge: Return() must never hand a // caller-owned or ZipPart-owned array back to ArrayPool. public class BufferedStreamCursorTests diff --git a/tests/ExcelReader.Tests/ChainedWorkbookStreamTests.cs b/tests/ExcelReader.Tests/ChainedWorkbookStreamTests.cs new file mode 100644 index 00000000..e8ee8b84 --- /dev/null +++ b/tests/ExcelReader.Tests/ChainedWorkbookStreamTests.cs @@ -0,0 +1,103 @@ +using System.Buffers; +using ExcelReader.Core.Reader; + +namespace ExcelReader.Tests +{ + // The chained (whole-file-in-memory + FAT chain) WorkbookStream mode, driven directly rather than + // through XlsReader: XlsWorkbookBuilder always emits a strictly sequential sector chain, so the + // discontinuity handling below — run resolution and the sector-by-sector assembly a straddling + // record falls back to — is unreachable from the builder-based fixtures. + public class ChainedWorkbookStreamTests + { + private const int SectorSize = 512; + private const int HeaderSize = 512; + private const int FileSectors = 3; + + // Logical sector 0 -> file sector 2, logical sector 1 -> file sector 0. The chain is therefore + // discontinuous at the boundary (0 != 2 + 1), so a record crossing it cannot be served as one + // zero-copy slice. + private static (byte[] Buffer, int[] Chain) BuildFragmented(int firstSector = 2) + { + byte[] buffer = new byte[HeaderSize + (FileSectors * SectorSize)]; + int[] chain = ArrayPool.Shared.Rent(2); + chain[0] = firstSector; + chain[1] = 0; + return (buffer, chain); + } + + private static int FileOffset(int sector, int within) + { + return HeaderSize + (sector * SectorSize) + within; + } + + [Fact] + public void ReadsARecordThatStraddlesAChainDiscontinuity() + { + (byte[] buffer, int[] chain) = BuildFragmented(); + + // Header at logical 504 (inside logical sector 0), so its 8 data bytes span logical + // 508..515 — four bytes at the tail of file sector 2, four at the head of file sector 0. + buffer[FileOffset(2, 504)] = 0x03; + buffer[FileOffset(2, 505)] = 0x02; // id 0x0203 + buffer[FileOffset(2, 506)] = 8; + buffer[FileOffset(2, 507)] = 0; // len 8 + for (int i = 0; i < 4; i++) + { + buffer[FileOffset(2, 508 + i)] = (byte)(i + 1); // logical 508..511 -> 1,2,3,4 + buffer[FileOffset(0, i)] = (byte)(i + 5); // logical 512..515 -> 5,6,7,8 + } + + using WorkbookStream wb = WorkbookStream.Chained(buffer, chain, chainLength: 2, SectorSize, length: 2 * SectorSize); + using BiffCursor cursor = wb.OpenCursor(); + cursor.Position = 504; + + Assert.True(cursor.TryReadRecord(out int id, out ReadOnlySpan data)); + Assert.Equal(0x0203, id); + Assert.Equal([1, 2, 3, 4, 5, 6, 7, 8], data.ToArray()); + Assert.Equal(516, cursor.Position); + } + + [Fact] + public void ReadsRecordsWhollyInsideOneSectorFromTheCachedRun() + { + (byte[] buffer, int[] chain) = BuildFragmented(); + + // Two back-to-back records inside logical sector 0; the second must come from the run + // cached by the first, not a re-resolve. + buffer[FileOffset(2, 0)] = 0x03; + buffer[FileOffset(2, 1)] = 0x02; + buffer[FileOffset(2, 2)] = 2; + buffer[FileOffset(2, 4)] = 0xAA; + buffer[FileOffset(2, 5)] = 0xBB; + buffer[FileOffset(2, 6)] = 0x05; + buffer[FileOffset(2, 7)] = 0x02; + buffer[FileOffset(2, 8)] = 1; + buffer[FileOffset(2, 10)] = 0xCC; + + using WorkbookStream wb = WorkbookStream.Chained(buffer, chain, chainLength: 2, SectorSize, length: 2 * SectorSize); + using BiffCursor cursor = wb.OpenCursor(); + + Assert.True(cursor.TryReadRecord(out int first, out ReadOnlySpan firstData)); + Assert.Equal(0x0203, first); + Assert.Equal([0xAA, 0xBB], firstData.ToArray()); + + Assert.True(cursor.TryReadRecord(out int second, out ReadOnlySpan secondData)); + Assert.Equal(0x0205, second); + Assert.Equal([0xCC], secondData.ToArray()); + } + + // A crafted chain entry pointing past the buffer must surface as InvalidDataException, the same + // type the streamed path raises for a corrupt container — not an IndexOutOfRange from slicing, + // and never a read of whatever happens to sit past the workbook in the caller's buffer. + [Fact] + public void ThrowsInvalidDataWhenAChainEntryPointsPastTheBuffer() + { + (byte[] buffer, int[] chain) = BuildFragmented(firstSector: 99); + + using WorkbookStream wb = WorkbookStream.Chained(buffer, chain, chainLength: 2, SectorSize, length: 2 * SectorSize); + using BiffCursor cursor = wb.OpenCursor(); + + Assert.Throws(() => cursor.TryReadRecord(out _, out _)); + } + } +} diff --git a/tests/ExcelReader.Tests/ConcurrencyContractTests.cs b/tests/ExcelReader.Tests/ConcurrencyContractTests.cs new file mode 100644 index 00000000..7a815285 --- /dev/null +++ b/tests/ExcelReader.Tests/ConcurrencyContractTests.cs @@ -0,0 +1,59 @@ +using ExcelReader.Core.Reader; +using ExcelReader.Core.Writer; + +namespace ExcelReader.Tests +{ + // Covers the thread-safety contract documented on IExcelRowReader/IWorkbookWriter: instances are + // not thread-safe, but independent instances used one-per-thread must not interfere with each + // other. These tests would catch a regression where a reader/writer accidentally leaned on shared + // mutable state (a static cache, a shared buffer pool misuse, etc.) instead of per-instance state. + public class ConcurrencyContractTests + { + [Fact] + public Task IndependentXlsxReadersOnSeparateThreadsDoNotInterfere() + { + const int readerCount = 16; + IEnumerable tasks = Enumerable.Range(0, readerCount).Select(i => Task.Run(() => + { + string expected = $"value-{i}"; + using MemoryStream ms = WorkbookBuilder.Build($"""{expected}"""); + using XlsxReader reader = Excel.From(ms); + using XlsxReader.Enumerator e = reader.GetEnumerator(); + Assert.True(e.MoveNext()); + Assert.Equal(expected, e.Current[0].GetString()); + })); + + return Task.WhenAll(tasks); + } + + [Fact] + public Task IndependentXlsxWorkbookWritersOnSeparateThreadsDoNotInterfere() + { + const int writerCount = 16; + IEnumerable tasks = Enumerable.Range(0, writerCount).Select(i => Task.Run(async () => + { + string expected = $"value-{i}"; + using MemoryStream ms = new(); + await using (XlsxWorkbookWriter wb = await XlsxWorkbookWriter.CreateAsync(ms, leaveOpen: true, ct: TestContext.Current.CancellationToken)) + { + await wb.StartAsync(TestContext.Current.CancellationToken); + XlsxSheetWriter sheet = wb.AddSheet("Sheet1"); + await sheet.StartAsync(TestContext.Current.CancellationToken); + await using (XlsxRowWriter row = await sheet.StartRowAsync(TestContext.Current.CancellationToken)) + { + row.Write(expected); + } + await sheet.EndAsync(TestContext.Current.CancellationToken); + await wb.EndAsync(TestContext.Current.CancellationToken); + } + ms.Position = 0; + using XlsxReader reader = Excel.From(ms); + using XlsxReader.Enumerator e = reader.GetEnumerator(); + Assert.True(e.MoveNext()); + Assert.Equal(expected, e.Current[0].GetString()); + })); + + return Task.WhenAll(tasks); + } + } +} diff --git a/tests/ExcelReader.Tests/DialectRobustnessTests.cs b/tests/ExcelReader.Tests/DialectRobustnessTests.cs index e5eda5fd..73f15966 100644 --- a/tests/ExcelReader.Tests/DialectRobustnessTests.cs +++ b/tests/ExcelReader.Tests/DialectRobustnessTests.cs @@ -42,6 +42,31 @@ public void AttrReturnsEmptyForMissingOrUnquotedAttribute() Assert.True(XlsxXml.Attr(" name=bad"u8, " name="u8).IsEmpty); // unquoted value } + // ---- S5: overlong numeric XML entities must not wrap into an unrelated valid codepoint ---- + + [Fact] + public void OverlongDecimalEntityRoundTripsAsLiteralText() + { + // 20 digits; unchecked accumulation would wrap the int and could decode to a valid, + // unrelated codepoint instead of being rejected as malformed. + string result = XlsxXml.DecodeToString("�"u8); + Assert.Equal("�", result); + } + + [Fact] + public void OverlongHexEntityRoundTripsAsLiteralText() + { + string result = XlsxXml.DecodeToString("�"u8); + Assert.Equal("�", result); + } + + [Fact] + public void ValidNumericEntityStillDecodesNormally() + { + Assert.Equal("A", XlsxXml.DecodeToString("A"u8)); + Assert.Equal("A", XlsxXml.DecodeToString("A"u8)); + } + [Fact] public void SingleQuotedWorkbookAttributesAreRead() { diff --git a/tests/ExcelReader.Tests/ExcelOpenAndOleErrorTests.cs b/tests/ExcelReader.Tests/ExcelOpenAndOleErrorTests.cs index 0609c310..f39970a6 100644 --- a/tests/ExcelReader.Tests/ExcelOpenAndOleErrorTests.cs +++ b/tests/ExcelReader.Tests/ExcelOpenAndOleErrorTests.cs @@ -331,6 +331,64 @@ public async Task NonSeekableXlsSourceIsBufferedAndReadAsync() Assert.Equal("Async", e.Current[0].GetString()); } + // --- Excel.FromXlsx* aliases (API1: format-named factory matching FromXls/FromXlsb) --- + + [Fact] + public void FromXlsxFileOpensLikeFromFile() + { + string path = WriteTemp(".xlsx", WorkbookBuilder.Build("""1""")); + try + { + using XlsxReader reader = Excel.FromXlsxFile(path); + using XlsxReader.Enumerator e = reader.GetEnumerator(); + Assert.True(e.MoveNext()); + Assert.Equal("1", e.Current[0].GetString()); + } + finally + { + File.Delete(path); + } + } + + [Fact] + public void FromXlsxStreamAndMemoryOpenLikeFrom() + { + using MemoryStream ms = WorkbookBuilder.Build("""7"""); + using XlsxReader streamReader = Excel.FromXlsx(ms, leaveOpen: true); + using XlsxReader.Enumerator se = streamReader.GetEnumerator(); + Assert.True(se.MoveNext()); + Assert.Equal("7", se.Current[0].GetString()); + + using XlsxReader memoryReader = Excel.FromXlsx(ms.ToArray().AsMemory()); + using XlsxReader.Enumerator me = memoryReader.GetEnumerator(); + Assert.True(me.MoveNext()); + Assert.Equal("7", me.Current[0].GetString()); + } + + [Fact] + public async Task FromXlsxFileAsyncAndFromXlsxAsyncOpenLikeTheirCanonicalCounterparts() + { + CancellationToken ct = TestContext.Current.CancellationToken; + string path = WriteTemp(".xlsx", WorkbookBuilder.Build("""9""")); + try + { + await using XlsxReader fileReader = await Excel.FromXlsxFileAsync(path, ct: ct); + await using XlsxReader.Enumerator fe = fileReader.GetEnumerator(); + Assert.True(fe.MoveNext()); + Assert.Equal("9", fe.Current[0].GetString()); + } + finally + { + File.Delete(path); + } + + await using MemoryStream ms = WorkbookBuilder.Build("""3"""); + await using XlsxReader streamReader = await Excel.FromXlsxAsync(ms, ct: ct); + await using XlsxReader.Enumerator se = streamReader.GetEnumerator(); + Assert.True(se.MoveNext()); + Assert.Equal("3", se.Current[0].GetString()); + } + private static string WriteTemp(string extension, MemoryStream content) { string path = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString("N") + extension); diff --git a/tests/ExcelReader.Tests/FuzzTests.cs b/tests/ExcelReader.Tests/FuzzTests.cs index 5552ad7e..003f651a 100644 --- a/tests/ExcelReader.Tests/FuzzTests.cs +++ b/tests/ExcelReader.Tests/FuzzTests.cs @@ -5,8 +5,8 @@ namespace ExcelReader.Tests { - // Seeded-random-mutator fuzz harness (F14 in docs/road-to-a.md). No binary-format parser in this - // codebase (OLE/CFB, BIFF8, BIFF12, ZIP) had ever been exercised against randomized corruption — + // Seeded-random-mutator fuzz harness. No binary-format parser in this codebase (OLE/CFB, BIFF8, + // BIFF12, ZIP) had ever been exercised against randomized corruption — // only hand-crafted malformed inputs. This flips random bytes in otherwise-valid seed files and // requires every resulting failure to surface as one of this library's own graceful rejections // (ExcelLimitExceededException, or a well-known BCL parsing exception like InvalidDataException), diff --git a/tests/ExcelReader.Tests/MemorySourceParityTests.cs b/tests/ExcelReader.Tests/MemorySourceParityTests.cs new file mode 100644 index 00000000..e302001e --- /dev/null +++ b/tests/ExcelReader.Tests/MemorySourceParityTests.cs @@ -0,0 +1,123 @@ +using System.Buffers; +using System.Text; +using ExcelReader.Core.Reader; +using ExcelReader.Core.ValueObjects; + +namespace ExcelReader.Tests +{ + public class MemorySourceParityTests + { + [Fact] + public void CsvMemorySourceMatchesStreamSource() + { + byte[] bytes = Encoding.UTF8.GetBytes("name,age\nAda,37\n"); + + using CsvReader streamReader = Excel.FromCsv(new MemoryStream(bytes, writable: false)); + using CsvReader memoryReader = Excel.FromCsv(bytes.AsMemory()); + + AssertRowsEqual(streamReader, memoryReader); + } + + [Fact] + public void CsvMemorySourceMatchesStreamSource_ForSlicedAndNonArrayBackedBuffers() + { + byte[] bytes = Encoding.UTF8.GetBytes("name,age\nAda,37\n"); + ReadOnlyMemory sliced = bytes.AsMemory(1, bytes.Length - 2); + var manager = new NonArrayMemoryManager(sliced.ToArray()); + + using CsvReader streamReader = Excel.FromCsv(new MemoryStream(sliced.ToArray(), writable: false)); + using CsvReader memoryReader = Excel.FromCsv(sliced); + using CsvReader managerReader = Excel.FromCsv(manager.Memory); + + AssertRowsEqual(streamReader, memoryReader); + AssertRowsEqual(streamReader, managerReader); + } + + [Fact] + public void XlsMemorySourceMatchesStreamSource() + { + byte[] bytes = XlsWorkbookBuilder.Build(sheets: [("S1", [["Name", 1, true]])]).ToArray(); + + using XlsReader streamReader = Excel.FromXls(new MemoryStream(bytes, writable: false)); + using XlsReader memoryReader = Excel.FromXls(bytes.AsMemory()); + + AssertRowsEqual(streamReader, memoryReader); + } + + [Fact] + public void XlsMemorySourceMatchesStreamSource_ForSlicedAndNonArrayBackedBuffers() + { + byte[] bytes = XlsWorkbookBuilder.Build(sheets: [("S1", [["Name", 1, true]])]).ToArray(); + byte[] prefixed = [0, 0, 0, .. bytes]; + ReadOnlyMemory sliced = prefixed.AsMemory(3, bytes.Length); + var manager = new NonArrayMemoryManager(bytes); + + using XlsReader streamReader = Excel.FromXls(new MemoryStream(sliced.ToArray(), writable: false)); + using XlsReader memoryReader = Excel.FromXls(sliced); + using XlsReader managerReader = Excel.FromXls(manager.Memory); + + AssertRowsEqual(streamReader, memoryReader); + AssertRowsEqual(streamReader, managerReader); + } + + [Fact] + public void XlsMemorySourceThrowsForMalformedOleBuffers() + { + byte[] bytes = Encoding.UTF8.GetBytes("not an OLE document"); + + Assert.Throws(() => Excel.FromXls(bytes.AsMemory())); + } + + private static void AssertRowsEqual(IExcelRowReader expected, IExcelRowReader actual) + { + string[] expectedValues = ReadRows(expected); + string[] actualValues = ReadRows(actual); + + Assert.Equal(expectedValues, actualValues); + } + + private static string[] ReadRows(IExcelRowReader reader) + { + List values = []; + foreach (Row row in reader) + { + StringBuilder sb = new(); + foreach (var cell in row.Cells) + { + sb.Append(cell.Value.GetString()); + sb.Append('|'); + } + values.Add(sb.ToString()); + } + return [.. values]; + } + + private sealed class NonArrayMemoryManager(byte[] data) : MemoryManager + { + private readonly byte[] _data = data; + + public override Memory Memory => _data; + + public override Span GetSpan() + { + throw new NotSupportedException(); + } + + public override MemoryHandle Pin(int elementIndex = 0) + { + throw new NotSupportedException(); + } + + public override void Unpin() + { + throw new NotSupportedException(); + } + +#pragma warning disable IDISP010 // Call base.Dispose(disposing) + protected override void Dispose(bool disposing) +#pragma warning restore IDISP010 // Call base.Dispose(disposing) + { + } + } + } +} diff --git a/tests/ExcelReader.Tests/MemoryZipParityTests.cs b/tests/ExcelReader.Tests/MemoryZipParityTests.cs index 50f26808..a178e6d8 100644 --- a/tests/ExcelReader.Tests/MemoryZipParityTests.cs +++ b/tests/ExcelReader.Tests/MemoryZipParityTests.cs @@ -10,7 +10,7 @@ namespace ExcelReader.Tests { - // Z4/Z5 in docs/in-memory-zip.md: Excel.From/FromXlsb/Open(ReadOnlyMemory) must be + // Excel.From/FromXlsb/Open(ReadOnlyMemory) (the in-memory ZIP path) must be // observationally identical to the streamed path — same cells, same exceptions, same exception // types on malformed input. Every fixture here is a real ZipArchive-built file, so any divergence // is a bug in the memory path, not the fixture. @@ -67,8 +67,8 @@ public async Task MemoryAsyncEnumeratorNeverSuspends(MemoryFixture fixture) await using IExcelRowEnumerator e = await task; } - // ---- 2. PrefetchDecompression overlaps inflate with parsing here too (docs/in-memory-zip.md - // Phase 1 gate), same as the streamed path — output must stay identical either way. ---- + // ---- 2. PrefetchDecompression overlaps inflate with parsing here too, same as the + // streamed path — output must stay identical either way. ---- [Theory] [MemberData(nameof(Fixtures))] diff --git a/tests/ExcelReader.Tests/MidStreamCancellationTests.cs b/tests/ExcelReader.Tests/MidStreamCancellationTests.cs new file mode 100644 index 00000000..6509992d --- /dev/null +++ b/tests/ExcelReader.Tests/MidStreamCancellationTests.cs @@ -0,0 +1,148 @@ +using System.Text; +using ExcelReader.Core.Reader; +using ExcelReader.Core.Writer; + +namespace ExcelReader.Tests +{ + // Cancelling partway through a large sheet, not just at open time (PrefetchDecompressionTests + // already covers the already-cancelled/prefetch cases). XLS and XLSB check the token on every + // MoveNextAsync call, so cancellation is observed on the very next call regardless of buffering. + // XLSX and CSV only observe it when the pooled buffer actually needs a refill (via the + // underlying Stream.ReadAsync honoring the token), so those two need a workbook big enough to + // force a real refill after cancellation to prove the token is wired through at all. + public class MidStreamCancellationTests + { + [Fact] + public async Task XlsxCancellationMidLargeSheetThrows() + { + CancellationToken outer = TestContext.Current.CancellationToken; + byte[] bytes = BuildLargeXlsx(); + using CancellationTokenSource cts = new(); + + await using MemoryStream ms = new(bytes, writable: false); + await using XlsxReader reader = await Excel.FromAsync(ms, ct: outer); + await using XlsxReader.Enumerator e = await reader.GetAsyncEnumeratorAsync(cts.Token); + + for (int i = 0; i < 5; i++) + { + Assert.True(await e.MoveNextAsync()); + } + cts.Cancel(); + + await Assert.ThrowsAnyAsync(async () => + { + while (await e.MoveNextAsync()) + { + Assert.True(e.Current.ColumnCount >= 0); + } + }); + } + + [Fact] + public async Task CsvCancellationMidLargeSheetThrows() + { + CancellationToken outer = TestContext.Current.CancellationToken; + byte[] bytes = BuildLargeCsv(); + using CancellationTokenSource cts = new(); + + await using MemoryStream ms = new(bytes, writable: false); + await using CsvReader reader = await Excel.FromCsvAsync(ms, ct: outer); + await using CsvReader.Enumerator e = await reader.GetAsyncEnumeratorAsync(cts.Token); + + for (int i = 0; i < 5; i++) + { + Assert.True(await e.MoveNextAsync()); + } + cts.Cancel(); + + await Assert.ThrowsAnyAsync(async () => + { + while (await e.MoveNextAsync()) + { + Assert.True(e.Current.ColumnCount >= 0); + } + }); + } + + [Fact] + public async Task XlsCancellationAfterCancelThrowsOnNextMoveNext() + { + // XlsReader.Enumerator checks the token unconditionally on every call, so a small + // workbook already proves the contract — no need to force a buffer refill. + CancellationToken outer = TestContext.Current.CancellationToken; + using MemoryStream ms = XlsWorkbookBuilder.Build(sheets: [("S1", [["Row1"], ["Row2"], ["Row3"]])]); + using CancellationTokenSource cts = new(); + + await using XlsReader reader = await Excel.FromXlsAsync(ms, ct: outer); + await using XlsReader.Enumerator e = reader.GetAsyncEnumerator(cts.Token); + + Assert.True(await e.MoveNextAsync()); + cts.Cancel(); + + await Assert.ThrowsAnyAsync(async () => await e.MoveNextAsync()); + } + + [Fact] + public async Task XlsbCancellationAfterCancelThrowsOnNextMoveNext() + { + // XlsbReader.Enumerator checks the token unconditionally on every call too. + CancellationToken outer = TestContext.Current.CancellationToken; + byte[] bytes = await BuildSmallXlsbAsync(outer); + using CancellationTokenSource cts = new(); + + await using MemoryStream ms = new(bytes, writable: false); + await using XlsbReader reader = await Excel.FromXlsbAsync(ms, ct: outer); + await using XlsbReader.Enumerator e = await reader.GetAsyncEnumeratorAsync(cts.Token); + + Assert.True(await e.MoveNextAsync()); + cts.Cancel(); + + await Assert.ThrowsAnyAsync(async () => await e.MoveNextAsync()); + } + + // Many mixed-type rows so the sheet spans several 64 KiB buffer refills, not just the + // initial fill — the point where XLSX/CSV's implicit (stream-driven) cancellation fires. + private static byte[] BuildLargeXlsx() + { + StringBuilder sb = new(512 * 1024); + for (int r = 1; r <= 6000; r++) + { + sb.Append("") + .Append("").Append(r).Append("") + .Append("row ").Append(r).Append(" text") + .Append(""); + } + using MemoryStream ms = WorkbookBuilder.Build(sb.ToString()); + return ms.ToArray(); + } + + private static byte[] BuildLargeCsv() + { + StringBuilder sb = new(512 * 1024); + for (int r = 1; r <= 20_000; r++) + { + sb.Append(r).Append(",row ").Append(r).Append(" text,").Append(r * 1.5).Append('\n'); + } + return Encoding.UTF8.GetBytes(sb.ToString()); + } + + private static async Task BuildSmallXlsbAsync(CancellationToken ct) + { + MemoryStream ms = new(); + await using (XlsbWorkbookWriter wb = await XlsbWorkbookWriter.CreateAsync(ms, leaveOpen: true, ct: ct)) + { + await wb.StartAsync(ct); + XlsbSheetWriter sheet = wb.AddSheet("S1"); + await sheet.StartAsync(ct); + for (int r = 0; r < 3; r++) + { + await using XlsbRowWriter row = await sheet.StartRowAsync(ct); + row.Write($"row {r}"); + } + await sheet.EndAsync(ct); + await wb.EndAsync(ct); + } + return ms.ToArray(); + } + } +} diff --git a/tests/ExcelReader.Tests/ReaderLimitTests.cs b/tests/ExcelReader.Tests/ReaderLimitTests.cs index d0ed5f95..ce3e56be 100644 --- a/tests/ExcelReader.Tests/ReaderLimitTests.cs +++ b/tests/ExcelReader.Tests/ReaderLimitTests.cs @@ -45,7 +45,7 @@ public void CellAccumulatorAcceptsColumnIndexAtExcelLimitBoundary() } // Patches the uncompressed-size field of a central-directory record in place, so the entry's // declared size (what ZipArchiveEntry.Length reports) lies far above its real, tiny compressed - // content — the exact shape of a zip-bomb-style amplification attack (see docs/road-to-a.md, F1). + // content — the exact shape of a zip-bomb-style amplification attack. private static void ForgeCentralDirectoryUncompressedSize(byte[] zipBytes, string entryName, uint forgedSize) { byte[] nameBytes = Encoding.UTF8.GetBytes(entryName); @@ -257,6 +257,60 @@ public void LimitedReadStreamCountsSpanReadsAndEntryLimit() Assert.Equal(4, ex.Actual); } + // --- XlsCompoundFile (.xls / OLE-CFB) container-phase guard rails --- + + [Fact] + public void ForgedWorkbookSizeNearUInt32MaxThrowsInvalidDataException() + { + using MemoryStream ms = XlsWorkbookBuilder.BuildPatched( + XlsWorkbookBuilder.WorkbookSizeOffset, XlsWorkbookBuilder.LE64(0xFFFFFFFFL)); + Assert.Throws(() => Excel.FromXls(ms)); + } + + [Fact] + public void ForgedWorkbookSizeNearLongMaxThrowsInvalidDataException() + { + using MemoryStream ms = XlsWorkbookBuilder.BuildPatched( + XlsWorkbookBuilder.WorkbookSizeOffset, XlsWorkbookBuilder.LE64(long.MaxValue - 1)); + Assert.Throws(() => Excel.FromXls(ms)); + } + + [Fact] + public void ForgedRootEntrySizeAboveIntMaxValueThrowsInvalidDataException() + { + // The root entry's mini-stream length used to be cast straight from a long to an int; a + // value above the signed 32-bit range truncated to a negative limit, which the chain + // reader interpreted as "read everything" instead of "read N bytes". The default builder's + // workbook size always sits above the mini-stream cutoff, so the workbook size is shrunk + // here too, forcing the one branch that reads the root entry's length at all. + byte[] bytes = XlsWorkbookBuilder.Build(sheets: [("S1", [["A"]])]).ToArray(); + XlsWorkbookBuilder.LE64(100).CopyTo(bytes, XlsWorkbookBuilder.WorkbookSizeOffset); + XlsWorkbookBuilder.LE64(int.MaxValue + 1L).CopyTo(bytes, XlsWorkbookBuilder.RootEntrySizeOffset); + using var ms = new MemoryStream(bytes); + Assert.Throws(() => Excel.FromXls(ms)); + } + + [Fact] + public void ForgedMiniCutoffThrowsInvalidDataException() + { + using MemoryStream ms = XlsWorkbookBuilder.BuildPatched( + XlsWorkbookBuilder.MiniCutoffOffset, XlsWorkbookBuilder.LE32(int.MaxValue - 1)); + Assert.Throws(() => Excel.FromXls(ms)); + } + + [Fact] + public void ForgedOversizedWorkbookSizeTripsTotalDecompressedLimitBeforeAllocating() + { + using MemoryStream ms = XlsWorkbookBuilder.BuildPatched( + XlsWorkbookBuilder.WorkbookSizeOffset, XlsWorkbookBuilder.LE64(5000)); + + var options = new ExcelReaderOptions { MaxTotalDecompressedBytes = 4096 }; + + ExcelLimitExceededException ex = Assert.Throws(() => Excel.FromXls(ms, options: options)); + Assert.Equal(nameof(ExcelReaderOptions.MaxTotalDecompressedBytes), ex.LimitName); + Assert.Equal(5000, ex.Actual); + } + [Fact] public void ExcelLimitExceededExceptionConstructorsAreCovered() { diff --git a/tests/ExcelReader.Tests/RealWorldXlsxCorpusTests.cs b/tests/ExcelReader.Tests/RealWorldXlsxCorpusTests.cs index d411c19d..a9d4c321 100644 --- a/tests/ExcelReader.Tests/RealWorldXlsxCorpusTests.cs +++ b/tests/ExcelReader.Tests/RealWorldXlsxCorpusTests.cs @@ -1,194 +1,60 @@ using ExcelReader.Core.Enums; using ExcelReader.Core.Reader; -using ExcelReader.Core.ValueObjects; namespace ExcelReader.Tests { + // Fixtures in this class are genuine binaries actually exported by the named producer — unlike + // XlsxDialectShapeTests/XlsxProducerDialectShapeTests, which hand-author XML mimicking known + // quirks. Keep fixtures tiny (a handful of rows) to bound repo size. public class RealWorldXlsxCorpusTests { - public static IEnumerable Fixtures + // Generated via the SheetJS "xlsx" npm package (v0.18.5): XLSX.utils.aoa_to_sheet + + // XLSX.writeFile({ cellDates: true }). Notably exercises two dialect quirks XlsxReader + // supports specifically because non-Excel producers emit them: string cells typed t="str" + // (normally a cached formula-result type) instead of t="s"/"inlineStr", and date cells typed + // t="d" holding literal ISO-8601 text instead of a numeric serial. + [Fact] + public void ReadsSheetJsGeneratedWorkbook() { - get - { - yield return - [ - new CorpusFixture( - "LibreOffice-style whitespace, spans, and single quotes", - """ - - leading trailing - - 12.5 - 1 - - """, - null, - [ - new ExpectedCell(0, 0, " leading trailing ", CellType.ExcelString), - new ExpectedCell(0, 2, "12.5", CellType.Number), - new ExpectedCell(0, 3, "1", CellType.Boolean), - ]) - ]; - - yield return - [ - new CorpusFixture( - "openpyxl-style sparse inline and shared strings", - """ - - inline & escaped - 0 - - - -3 - - - """, - "rich text", - [ - new ExpectedCell(0, 1, "inline & escaped", CellType.ExcelString), - new ExpectedCell(0, 4, "rich text", CellType.ExcelString), - new ExpectedCell(1, 0, "-3", CellType.Number), - new ExpectedCell(1, 3, "", CellType.ExcelString), - ]) - ]; - - yield return - [ - new CorpusFixture( - "Apache POI-style formulas and explicit cell types", - """ - - CONCAT("a","b")ab - #DIV/0! - 0 - - """, - null, - [ - new ExpectedCell(0, 0, "ab", CellType.Formula), - new ExpectedCell(0, 1, "#DIV/0!", CellType.Error), - new ExpectedCell(0, 2, "0", CellType.Boolean), - ]) - ]; - - yield return - [ - new CorpusFixture( - "Excelize-style single-quoted attributes and CDATA text", - """ - - 0 - ]]> - - """, - "]]>", - [ - new ExpectedCell(0, 0, "shared & ", CellType.ExcelString), - new ExpectedCell(0, 1, "raw & ", CellType.ExcelString), - ]) - ]; - - yield return - [ - new CorpusFixture( - "Google Sheets-style extra row markup between cells", - """ - - 0 - - - - - next - - """, - "sheet title", - [ - new ExpectedCell(0, 0, "sheet title", CellType.ExcelString), - new ExpectedCell(1, 0, "next", CellType.ExcelString), - ]) - ]; - } - } - - [Theory] - [MemberData(nameof(Fixtures))] - public void SyncReaderHandlesCorpusFixture(CorpusFixture fixture) - { - using MemoryStream ms = WorkbookBuilder.Build(fixture.Rows, fixture.SharedStrings); - using XlsxReader reader = Excel.From(ms); - - Assert.NotEmpty(fixture.Expected); - AssertExpected(reader.GetEnumerator(), fixture.Expected); - } - - [Theory] - [MemberData(nameof(Fixtures))] - public async Task AsyncReaderHandlesCorpusFixture(CorpusFixture fixture) - { - CancellationToken ct = TestContext.Current.CancellationToken; - await using MemoryStream ms = WorkbookBuilder.Build(fixture.Rows, fixture.SharedStrings); - await using XlsxReader reader = await Excel.FromAsync(ms, ct: ct); - await using XlsxReader.Enumerator rows = await reader.GetAsyncEnumeratorAsync(ct); - - Assert.NotEmpty(fixture.Expected); - await AssertExpectedAsync(rows, fixture.Expected); - } - - private static void AssertExpected(XlsxReader.Enumerator rows, ExpectedCell[] expected) - { - int next = 0; - int rowIndex = 0; - while (rows.MoveNext()) - { - next = AssertRow(rows.Current, expected, next, rowIndex); - rowIndex++; - } - - Assert.Equal(expected.Length, next); - } - - private static async Task AssertExpectedAsync( - XlsxReader.Enumerator rows, - ExpectedCell[] expected) - { - int next = 0; - int rowIndex = 0; - while (await rows.MoveNextAsync()) - { - next = AssertRow(rows.Current, expected, next, rowIndex); - rowIndex++; - } - - Assert.Equal(expected.Length, next); - } - - private static int AssertRow(Row row, ExpectedCell[] expected, int next, int rowIndex) - { - while (next < expected.Length && expected[next].Row == rowIndex) - { - ExpectedCell cell = expected[next++]; - Assert.True(row.ColumnCount > cell.Column); - Assert.Equal(cell.Type, row[cell.Column].Type); - Assert.Equal(cell.Value, row[cell.Column].GetString()); - } - - return next; + string path = Path.Combine(AppContext.BaseDirectory, "data", "sheetjs-sample.xlsx"); + using XlsxReader reader = Excel.FromFile(path); + using XlsxReader.Enumerator e = reader.GetEnumerator(); + + Assert.True(e.MoveNext()); + Assert.Equal("name", e.Current[0].GetString()); + Assert.Equal("quantity", e.Current[1].GetString()); + Assert.Equal("price", e.Current[2].GetString()); + Assert.Equal("in_stock", e.Current[3].GetString()); + Assert.Equal("restock_date", e.Current[4].GetString()); + // SheetJS writes plain strings as t="str" (normally a cached-formula-result type) rather + // than t="s"/"inlineStr" — this is CellType.Formula here, not CellType.ExcelString. + Assert.Equal(CellType.Formula, e.Current[0].Type); + + Assert.True(e.MoveNext()); + Assert.Equal("Widget", e.Current[0].GetString()); + Assert.True(e.Current[1].TryParse(null, out int quantity)); + Assert.Equal(12, quantity); + Assert.True(e.Current[2].TryParse(null, out double price)); + Assert.Equal(4.5, price); + Assert.Equal(CellType.Boolean, e.Current[3].Type); + Assert.Equal("1", e.Current[3].GetString()); + Assert.Equal(CellType.Date, e.Current[4].Type); + Assert.True(e.Current[4].TryGetDateTime(out DateTime restock)); + Assert.Equal(new DateTime(2024, 1, 14, 21, 0, 0, DateTimeKind.Unspecified), restock); + + Assert.True(e.MoveNext()); + Assert.Equal("Gadget", e.Current[0].GetString()); + Assert.True(e.Current[1].TryParse(null, out int gadgetQty)); + Assert.Equal(0, gadgetQty); + Assert.True(e.Current[4].TryGetDateTime(out DateTime gadgetDate)); + Assert.Equal(new DateTime(2024, 2, 29, 21, 0, 0, DateTimeKind.Unspecified), gadgetDate); + + Assert.True(e.MoveNext()); + Assert.Equal("Gizmo", e.Current[0].GetString()); + Assert.True(e.Current[4].TryGetDateTime(out DateTime gizmoDate)); + Assert.Equal(new DateTime(2024, 6, 29, 21, 0, 0, DateTimeKind.Unspecified), gizmoDate); + + Assert.False(e.MoveNext()); } - - public sealed record CorpusFixture( - string Name, - string Rows, - string? SharedStrings, - ExpectedCell[] Expected) - { - public override string ToString() - { - return Name; - } - } - - public readonly record struct ExpectedCell(int Row, int Column, string Value, CellType Type); } } diff --git a/tests/ExcelReader.Tests/TestUtils.cs b/tests/ExcelReader.Tests/TestUtils.cs index 6a1fa5f4..83c7c397 100644 --- a/tests/ExcelReader.Tests/TestUtils.cs +++ b/tests/ExcelReader.Tests/TestUtils.cs @@ -28,7 +28,8 @@ internal NonSeekableStream(byte[] bytes) [ExcludeFromCodeCoverage] public override long Position - { get => _inner.Position; set => throw new NotSupportedException(); + { + get => _inner.Position; set => throw new NotSupportedException(); } public override int Read(byte[] buffer, int offset, int count) diff --git a/tests/ExcelReader.Tests/WorkbookWriterTests.cs b/tests/ExcelReader.Tests/WorkbookWriterTests.cs index db40e26c..1a1f8e9e 100644 --- a/tests/ExcelReader.Tests/WorkbookWriterTests.cs +++ b/tests/ExcelReader.Tests/WorkbookWriterTests.cs @@ -122,7 +122,7 @@ private sealed class MoneyConverter : IExcelCellConverter, IExcelCellWrit { public bool TryConvert(in Cell cell, bool isDate1904, IFormatProvider provider, out Money value) { - if (cell.TryParse(provider, out decimal amount)) + if (cell.TryParse(provider, out decimal amount)) { value = new Money(amount); return true; diff --git a/tests/ExcelReader.Tests/XlsWorkbookBuilder.cs b/tests/ExcelReader.Tests/XlsWorkbookBuilder.cs index 39c9cdb3..9991db74 100644 --- a/tests/ExcelReader.Tests/XlsWorkbookBuilder.cs +++ b/tests/ExcelReader.Tests/XlsWorkbookBuilder.cs @@ -111,10 +111,15 @@ internal static MemoryStream BuildRawSheet(bool includeEof, params (int Id, byte // tests to corrupt one field of an otherwise-valid container. internal const int SectorShiftOffset = 0x1E; // log2(sector size); valid is 9 -> 512 internal const int FatSectorCountOffset = 0x2C; // header DIFAT lists this many FAT sectors + internal const int MiniCutoffOffset = 0x38; // header's mini stream cutoff field (Int32) internal const int SignatureOffset = 0x00; // Directory is sector 1: header (512) + FAT sector (512) = byte 1024. The Workbook entry // is the second 128-byte directory entry, so its UTF-16 name starts at 1024 + 128. internal const int WorkbookEntryNameOffset = 1024 + 128; + // The Workbook entry's Int64 Size field (see WriteDirectoryEntry: offset 120 within the entry). + internal const int WorkbookSizeOffset = 1024 + 128 + 120; + // The Root Entry's Int64 Size field — the first 128-byte directory entry, so no +128 offset. + internal const int RootEntrySizeOffset = 1024 + 120; // A valid single-sheet workbook with `replacement` overwritten at `offset`. internal static MemoryStream BuildPatched(int offset, params byte[] replacement) @@ -134,6 +139,13 @@ internal static byte[] LE16(int value) return U16(value); } + internal static byte[] LE64(long value) + { + byte[] bytes = new byte[8]; + BinaryPrimitives.WriteInt64LittleEndian(bytes, value); + return bytes; + } + internal static byte[] RawLabel(int row, int col, string value) { return [.. U16(row), .. U16(col), .. U16(0), .. BiffString(value)]; diff --git a/tests/ExcelReader.Tests/XlsxDialectShapeTests.cs b/tests/ExcelReader.Tests/XlsxDialectShapeTests.cs new file mode 100644 index 00000000..0922f3b2 --- /dev/null +++ b/tests/ExcelReader.Tests/XlsxDialectShapeTests.cs @@ -0,0 +1,197 @@ +using ExcelReader.Core.Enums; +using ExcelReader.Core.Reader; +using ExcelReader.Core.ValueObjects; + +namespace ExcelReader.Tests +{ + // Hand-authored XML fragments mimicking known quirks of specific producers (LibreOffice, openpyxl, + // Apache POI, etc.) — not files actually exported by them. See RealWorldXlsxCorpusTests for tests + // against genuine producer-exported binaries. + public class XlsxDialectShapeTests + { + public static IEnumerable Fixtures + { + get + { + yield return + [ + new CorpusFixture( + "LibreOffice-style whitespace, spans, and single quotes", + """ + + leading trailing + + 12.5 + 1 + + """, + null, + [ + new ExpectedCell(0, 0, " leading trailing ", CellType.ExcelString), + new ExpectedCell(0, 2, "12.5", CellType.Number), + new ExpectedCell(0, 3, "1", CellType.Boolean), + ]) + ]; + + yield return + [ + new CorpusFixture( + "openpyxl-style sparse inline and shared strings", + """ + + inline & escaped + 0 + + + -3 + + + """, + "rich text", + [ + new ExpectedCell(0, 1, "inline & escaped", CellType.ExcelString), + new ExpectedCell(0, 4, "rich text", CellType.ExcelString), + new ExpectedCell(1, 0, "-3", CellType.Number), + new ExpectedCell(1, 3, "", CellType.ExcelString), + ]) + ]; + + yield return + [ + new CorpusFixture( + "Apache POI-style formulas and explicit cell types", + """ + + CONCAT("a","b")ab + #DIV/0! + 0 + + """, + null, + [ + new ExpectedCell(0, 0, "ab", CellType.Formula), + new ExpectedCell(0, 1, "#DIV/0!", CellType.Error), + new ExpectedCell(0, 2, "0", CellType.Boolean), + ]) + ]; + + yield return + [ + new CorpusFixture( + "Excelize-style single-quoted attributes and CDATA text", + """ + + 0 + ]]> + + """, + "]]>", + [ + new ExpectedCell(0, 0, "shared & ", CellType.ExcelString), + new ExpectedCell(0, 1, "raw & ", CellType.ExcelString), + ]) + ]; + + yield return + [ + new CorpusFixture( + "Google Sheets-style extra row markup between cells", + """ + + 0 + + + + + next + + """, + "sheet title", + [ + new ExpectedCell(0, 0, "sheet title", CellType.ExcelString), + new ExpectedCell(1, 0, "next", CellType.ExcelString), + ]) + ]; + } + } + + [Theory] + [MemberData(nameof(Fixtures))] + public void SyncReaderHandlesCorpusFixture(CorpusFixture fixture) + { + using MemoryStream ms = WorkbookBuilder.Build(fixture.Rows, fixture.SharedStrings); + using XlsxReader reader = Excel.From(ms); + + Assert.NotEmpty(fixture.Expected); + AssertExpected(reader.GetEnumerator(), fixture.Expected); + } + + [Theory] + [MemberData(nameof(Fixtures))] + public async Task AsyncReaderHandlesCorpusFixture(CorpusFixture fixture) + { + CancellationToken ct = TestContext.Current.CancellationToken; + await using MemoryStream ms = WorkbookBuilder.Build(fixture.Rows, fixture.SharedStrings); + await using XlsxReader reader = await Excel.FromAsync(ms, ct: ct); + await using XlsxReader.Enumerator rows = await reader.GetAsyncEnumeratorAsync(ct); + + Assert.NotEmpty(fixture.Expected); + await AssertExpectedAsync(rows, fixture.Expected); + } + + private static void AssertExpected(XlsxReader.Enumerator rows, ExpectedCell[] expected) + { + int next = 0; + int rowIndex = 0; + while (rows.MoveNext()) + { + next = AssertRow(rows.Current, expected, next, rowIndex); + rowIndex++; + } + + Assert.Equal(expected.Length, next); + } + + private static async Task AssertExpectedAsync( + XlsxReader.Enumerator rows, + ExpectedCell[] expected) + { + int next = 0; + int rowIndex = 0; + while (await rows.MoveNextAsync()) + { + next = AssertRow(rows.Current, expected, next, rowIndex); + rowIndex++; + } + + Assert.Equal(expected.Length, next); + } + + private static int AssertRow(Row row, ExpectedCell[] expected, int next, int rowIndex) + { + while (next < expected.Length && expected[next].Row == rowIndex) + { + ExpectedCell cell = expected[next++]; + Assert.True(row.ColumnCount > cell.Column); + Assert.Equal(cell.Type, row[cell.Column].Type); + Assert.Equal(cell.Value, row[cell.Column].GetString()); + } + + return next; + } + + public sealed record CorpusFixture( + string Name, + string Rows, + string? SharedStrings, + ExpectedCell[] Expected) + { + public override string ToString() + { + return Name; + } + } + + public readonly record struct ExpectedCell(int Row, int Column, string Value, CellType Type); + } +} diff --git a/tests/ExcelReader.Tests/RealWorldInteropTests.cs b/tests/ExcelReader.Tests/XlsxProducerDialectShapeTests.cs similarity index 98% rename from tests/ExcelReader.Tests/RealWorldInteropTests.cs rename to tests/ExcelReader.Tests/XlsxProducerDialectShapeTests.cs index 4db0e02b..b51b8a56 100644 --- a/tests/ExcelReader.Tests/RealWorldInteropTests.cs +++ b/tests/ExcelReader.Tests/XlsxProducerDialectShapeTests.cs @@ -7,7 +7,9 @@ namespace ExcelReader.Tests { - public class RealWorldInteropTests + // Hand-authored XML/ZIP fragments mimicking known producer quirks — not files actually exported by + // those producers. See RealWorldXlsxCorpusTests for tests against genuine producer-exported binaries. + public class XlsxProducerDialectShapeTests { private const string SpreadsheetNs = "http://schemas.openxmlformats.org/spreadsheetml/2006/main"; private const string RelationshipsNs = "http://schemas.openxmlformats.org/officeDocument/2006/relationships"; diff --git a/tests/ExcelReader.Tests/XlsxReaderTests.cs b/tests/ExcelReader.Tests/XlsxReaderTests.cs index 14f85a41..4266bff4 100644 --- a/tests/ExcelReader.Tests/XlsxReaderTests.cs +++ b/tests/ExcelReader.Tests/XlsxReaderTests.cs @@ -6,7 +6,7 @@ namespace ExcelReader.Tests { // Focused reader suite for XlsxReader itself — the flagship format's coverage was previously // scattered across dialect/corpus/interop test files with no single place asserting the reader's - // own structural behaviors (F13 in docs/road-to-a.md). + // own structural behaviors. public class XlsxReaderTests { [Fact] diff --git a/tests/ExcelReader.Tests/ZipMemoryIndexTests.cs b/tests/ExcelReader.Tests/ZipMemoryIndexTests.cs index 2e991d88..831870ea 100644 --- a/tests/ExcelReader.Tests/ZipMemoryIndexTests.cs +++ b/tests/ExcelReader.Tests/ZipMemoryIndexTests.cs @@ -8,8 +8,8 @@ namespace ExcelReader.Tests { - // Z1 in docs/in-memory-zip.md: the in-memory ZIP central-directory reader, exercised directly - // (Excel.From(ReadOnlyMemory) — Z4 — doesn't exist yet). Every fixture here is a real + // The in-memory ZIP central-directory reader, exercised directly here (rather than only through + // Excel.From(ReadOnlyMemory)). Every fixture here is a real // ZipArchive-built file, so any divergence from the streamed path (parsed via // ZipEntryBytes/ZipArchive in the same test) is a bug in the new reader, not the fixture. // No CRC-32 check: ZipArchive doesn't validate it on read either (see ZipMemoryIndex.OpenPart), diff --git a/tests/ExcelReader.Tests/data/sheetjs-sample.xlsx b/tests/ExcelReader.Tests/data/sheetjs-sample.xlsx new file mode 100644 index 00000000..24f60c90 Binary files /dev/null and b/tests/ExcelReader.Tests/data/sheetjs-sample.xlsx differ