You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Fix PDF linear-scan crash and add configurable render background - #34
Crash on linear-scan fallback recovery (PdfDocument.Open): a real-world PDF
(BR_Cavro_Air_Displacement_Pipettor_396359.pdf) crashed with InvalidDataException: "Invalid character in hexadecimal string." while recovering via
the linear-scan fallback (BuildLinearScanFallback/ScanObjectOffsets). Root cause: the
fallback tokenized the entire raw document buffer - including binary/compressed stream
payloads never meant to be parsed as PDF syntax - with the general-purpose PdfTokenizer,
and a stray < byte in binary noise threw uncaught. Worse, even once the throw was
caught, the same general-purpose tokenization badly desynced against binary noise
(misreading it as implausible number/string tokens), causing the scan to miss the vast
majority of real objects in the file. Objects stored only inside compressed /Type /ObjStm
object streams were also never recoverable by this fallback at all (no literal N G obj
marker for them). Fixed by:
Rewriting ScanObjectOffsets/ScanForTrailerDictionary to locate N G obj / trailer
markers via raw byte-pattern scanning instead of general-purpose tokenization, which is
far more resilient to binary stream noise.
Adding RegisterCompressedObjectsFromObjectStreams() to recover objects nested inside
discovered object streams during the fallback.
Verified against the originally-reported file (now opens, 6/6 pages, with only the
already-known/documented JPXDecode limitation affecting 3 pages) and against all 5
external PDF corpora (115 files): zero crashes, zero timeouts, zero regressions.
Missing configurable page background color: PdfDocument.Render always produced a
fully transparent background, unlike essentially every other PDF renderer, which defaults
to opaque white. Added a new PdfRenderOptions type (an extensible options record/class,
intentionally designed so future render-time options can be added without further breaking
changes) with a BackgroundColor property defaulting to opaque white, and a new trailing PdfRenderOptions? options = null parameter on both Render overloads. This is a 0.0.0
pre-release package with no external consumers, so this breaking change was accepted now
deliberately to avoid piecemeal breaking changes later as more render options are added.
Existing test call sites that rely on isolating painted regions against a transparent
background were updated to pass an explicit transparent PdfRenderOptions override,
preserving their original intent.
Type of Change
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update
Code quality improvement
Related Issues
Closes #
Pre-Submission Checklist
Build and Test
Code builds successfully and all tests pass: pwsh ./build.ps1 (7314/7314 tests passed, net8.0/net9.0/net10.0)
Code produces zero warnings
Code Quality
New code has appropriate XML documentation comments
Static analyzer warnings have been addressed
Quality Checks
All linters pass: pwsh ./lint.ps1 (markdownlint-cli2, cspell, yamllint, dotnet format, reqstream, versionmark, reviewmark, sysml2tools)
Testing
Added unit tests for new functionality (two crash-fix regression tests with hand-authored
minimal malformed PDF fixtures, plus default/custom/transparent PdfRenderOptions behavior
tests)
Updated existing tests if behavior changed (93 existing Render(...) call sites audited;
paint-isolation tests updated to pass an explicit transparent background override)
All tests follow the AAA (Arrange, Act, Assert) pattern
Test coverage is maintained or improved
Documentation
Updated README.md (if applicable)
Updated docs/ documentation (if applicable) - reqstream, design, verification, sysml2
model, and user guide updated for the new configurable background color
Added code examples for new features (if applicable)
Updated requirements.yaml (if applicable)
Additional Notes
External PDF corpus harness (5 corpora, 115 files total) re-run against this branch's PDF
project: zero crashes, zero timeouts, consistent with prior baselines.
- Rewrite ScanObjectOffsets/ScanForTrailerDictionary to locate "N G obj" /
"trailer" markers via raw byte-pattern scanning instead of tokenizing raw
stream bytes with PdfTokenizer, fixing a crash (uncaught
InvalidDataException from ReadHexString on a stray less-than byte in
binary stream noise) and a silent desync that caused most real objects in
malformed/non-standard-xref PDFs to be missed.
- Add RegisterCompressedObjectsFromObjectStreams() to resolve indirect
objects stored only inside /Type /ObjStm compressed object streams during
linear-scan fallback, fixing "Indirect object N is not defined" failures.
- Add regression tests covering both fixes with hand-authored malformed PDF
fixtures (linear-scan-stream-noise.pdf, object-stream-linear-scan-fallback.pdf).
- Add PdfRenderOptions (opaque white default BackgroundColor) and thread an
optional PdfRenderOptions parameter through both PdfDocument.Render
overloads, clearing the rendered Surface to the configured background
color before executing the content stream (previously always fully
transparent).
- Update existing test call sites to pass a transparent PdfRenderOptions
where tests assert paint isolation against a transparent background, and
add dedicated tests for the new default/opaque-white and custom
background-color behavior.
- Update companion documentation (reqstream, design, verification, sysml2
model, review sets, user guide) to cover the new configurable background
behavior.
Reworks fallback scanning and compressed-object recovery. Critical (2 votes): encrypted object streams may be decoded before encryption initialization. Critical (4 votes): raw scanning can treat stream-payload markers as object offsets.
…llback
Addresses two critical findings from PR review of the linear-scan fallback
recovery path:
- Initialize encryption before decoding compressed object streams during
fallback recovery, so encrypted ObjStm containers are decrypted correctly
instead of being misread as plaintext.
- Exclude stream payload content from raw object-offset scanning, so an
'N G obj'-shaped byte sequence embedded inside binary/compressed stream
data (or a PDF content-stream comment) is no longer mistaken for a real
indirect object header and does not corrupt the reconstructed offset
table.
Fixing the stream-payload exclusion surfaced two further regressions,
also fixed here:
- The endstream search incorrectly required a non-identifier byte
immediately before the match, which can reject genuine matches since
that byte is arbitrary stream payload data.
- Skipping past a stream payload was being attempted after every raw
'obj' byte match, including the tail of 'endobj', causing spurious
forward scans that could skip over and silently drop small, nearby
objects (such as single-object ObjStm containers).
Verified against the originally reported crashing file and all 115 files
across 5 external PDF corpora: no crashes, same open-failure counts as
baseline.
Also:
- Adds regression tests for both findings, including one proven to fail
without the encryption-ordering fix.
- Adds a test proving the DPI-based Render overload forwards custom
PdfRenderOptions (background color) unchanged.
- Documents the DPI-based Render overload in the design doc (previously
undocumented).
- Fixes a stale test-name reference in reqstream/verification docs left
over from the earlier PdfRenderOptions background-color change.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This raw search still examines stream payloads, so a payload containing trailer <<...>> after the real trailer becomes the last successfully parsed candidate and can replace the actual trailer, including its /Root or /Encrypt entries. Restrict trailer candidates to non-payload regions (or reuse the stream ranges found during object scanning) before assigning last.
var found = IndexOfKeyword(buffer, position, buffer.Length, "trailer"u8);
if (found < 0)
{
break;
}
var after = found + "trailer"u8.Length;
var tokenizer = new PdfTokenizer(buffer) { Position = after };
try
{
var candidate = ParseValue(tokenizer);
if (candidate.Kind == PdfKind.Dictionary)
{
last = candidate;
- SkipPastStreamPayload now tokenizes the dictionary and only treats
'stream' as genuine when it is the literal token immediately after
the dictionary's closing '>>', instead of a raw keyword search that
could be fooled by a dictionary value containing the word 'stream'.
- ScanObjectOffsets now tracks and returns the stream-payload byte
ranges it skips; ScanForTrailerDictionary uses these ranges to
reject 'trailer' keyword matches that fall inside a stream payload,
preventing a later, coincidental 'trailer' sequence inside stream
content from replacing the genuine trailer (and silently dropping
its /Encrypt entry).
- Corrected TryNextToken's stale XML doc comment, which claimed
ScanObjectOffsets/ScanForTrailerDictionary tokenize the entire raw
buffer; both now use raw-byte scanning instead.
- Added regression tests for both fallback-recovery fixes, each
verified to fail against the prior (unfixed) code.
- Linked the two new tests under the LinearScanFallback requirement.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Addressed the "previously missed" finding (ScanForTrailerDictionary, line 860 - excluding stream payloads when scanning for trailer dictionaries) in cd93d67, alongside the two new findings above.
ScanObjectOffsets now also tracks and returns every stream-payload byte range it skips over. ScanForTrailerDictionary takes these ranges and rejects any trailer keyword match whose byte position falls inside one, so a trailer <<...>> sequence occurring inside a stream's own payload bytes (after its declared /Length, but still before the literal endstream) can no longer win over the genuine trailer under the scan's own "later occurrence overrides an earlier one" semantics - preventing a stream payload from silently replacing the real trailer's /Root//Encrypt entries.
Added a regression test, PdfDocument_Open_LinearScanFallback_TrailerKeywordInsideStreamPayload_DoesNotReplaceRealTrailer, which builds an encrypted PDF where a decoy trailer <<...>> (missing /Encrypt) is embedded inside a stream's payload, physically after the genuine trailer. The test asserts the stream's own RC4-encrypted content still decrypts and renders correctly, proving the genuine trailer's /Encrypt entry survived. Verified this test fails against the prior (unfixed) code and passes with the fix.
The fallback trailer search has the same raw-text problem outside stream ranges: a comment or string such as % trailer << /Root ... >> is parsed as a candidate, and because the last successfully parsed candidate wins, it can replace the real trailer and lose the correct /Root or /Encrypt. Restrict trailer recognition to PDF syntax (or validate candidates against the surrounding xref/trailer structure) rather than searching all remaining bytes.
var found = IndexOfKeyword(buffer, position, buffer.Length, "trailer"u8);
if (found < 0)
{
break;
}
var after = found + "trailer"u8.Length;
if (streamPayloadRanges.Exists(range => found >= range.Start && found < range.End))
{
// This "trailer" byte sequence is inside a stream payload, not real PDF syntax -
// skip it without attempting to parse a dictionary here.
position = after;
continue;
}
var tokenizer = new PdfTokenizer(buffer) { Position = after };
try
{
var candidate = ParseValue(tokenizer);
if (candidate.Kind == PdfKind.Dictionary)
{
last = candidate;
Restrict backward object scans to valid PDF headers
This backward scan accepts N G obj text anywhere outside a recognized stream payload, including PDF comments and literal strings. For example, a later (3 0 obj) passes TryParseObjectHeaderBackward and overwrites object 3's real offset; resolving the page then parses the string bytes as an indirect object and fallback recovery fails. The scan needs lexical-state handling or candidate validation so only actual indirect-object headers can update _xref.
Stream data is bounded by the stream dictionary's /Length, but this code ignores that boundary and stops at the first raw endstream byte sequence. A valid payload can contain endstream followed by a non-identifier byte; the scan then ends the protected range early, allowing later payload text to be mistaken for object/trailer syntax and potentially corrupting fallback recovery. Use the declared stream length (with bounds validation) to determine the payload range, and only use endstream as a consistency check.
Rename test to reflect missing blank-surface assertion
This test no longer checks that the result is blank; it only records that rendering does not throw, while the new background-specific tests cover pixel values separately. Keeping RendersBlankSurface in the method name is misleading and can cause future failures to be diagnosed against an assertion the test does not make.
…stream length for payload boundary
Addresses third round of review findings on the PDF linear-scan fallback:
- ScanForTrailerDictionary and TryParseObjectHeaderBackward now require
candidate 'trailer'/'N G obj' keyword matches to begin at the start of a
line, rejecting byte-perfect matches embedded inside PDF comments or
string literals outside any stream payload.
- SkipPastStreamPayload now prefers the stream dictionary's own declared
/Length (validated against a literal 'endstream' keyword immediately
following it) to bound the payload range, falling back to the previous
raw 'endstream' search only when /Length is missing, indirect, or
inconsistent with the buffer. This prevents a coincidental 'endstream'
byte sequence partway through a payload from ending the protected range
too early.
- Renamed PdfDocument_ContentStream_NoContents_RendersBlankSurface to
RendersWithoutThrowing to match what the test actually asserts.
- Added regression tests for each of the above, each verified to fail
without its corresponding fix.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixed in 588ac59 (third round of linear-scan fallback review findings):
Trailer keyword outside stream payloads (ScanForTrailerDictionary): a trailer byte sequence embedded in a comment or string, positioned outside any stream payload, could still win under the "last occurrence wins" rule even after the stream-payload exclusion. Added a line-start requirement: a candidate trailer keyword is now only accepted when it begins at the start of a line (tolerating only leading horizontal whitespace back to a newline or buffer start), matching how genuine PDF trailers are always written. Regression test: PdfDocument_Open_LinearScanFallback_TrailerKeywordInCommentOutsideStream_DoesNotReplaceRealTrailer, verified to fail without this check.
Backward object-header scan outside stream payloads (TryParseObjectHeaderBackward): the same raw-text problem applied to N G obj headers found outside stream payloads, e.g. inside a dictionary string value. Applied the same line-start requirement before accepting a candidate header. Regression test: PdfDocument_Open_LinearScanFallback_ObjMarkerInsideNonStreamDictionaryValue_DoesNotCorruptOffset, verified to fail without this check.
Stream payload boundary relies on raw endstream search (SkipPastStreamPayload): a coincidental endstream byte sequence partway through a payload (followed by a non-identifier byte) would end the protected range too early. SkipPastStreamPayload now prefers the stream dictionary's own declared /Length (validated by confirming the literal endstream keyword immediately follows it, tolerating an optional CRLF/LF) to compute the payload boundary, falling back to the previous raw search only when /Length is missing, an indirect reference, or inconsistent with the buffer. Regression test: PdfDocument_Open_LinearScanFallback_EarlyFalseEndstreamInPayload_DoesNotCorruptOffsets, verified to fail without this check.
Misleading test name: PdfDocument_ContentStream_NoContents_RendersBlankSurface no longer asserted blankness (that's now covered separately by the background-option tests). Renamed to PdfDocument_ContentStream_NoContents_RendersWithoutThrowing.
All three new fixes were verified using the same methodology as prior rounds: each regression test was confirmed to fail when its corresponding fix was temporarily reverted, then confirmed to pass with the fix restored. Full solution test suite (7338 tests), reqstream --enforce, fix.ps1/build.ps1/lint.ps1, the originally reported crashing file, and all 115 files across the 5 external PDF corpora were re-verified with no regressions.
…am-length search
Addresses a fourth-round review finding: when a stream's /Length is
indirect, absent, or inconsistent with the buffer, SkipPastStreamPayload's
fallback endstream search previously accepted the very first raw
'endstream' byte match unconditionally. Coincidental binary payload noise
can spell 'endstream' partway through genuine stream data, so accepting
that match resumed scanning from inside the payload, letting embedded
object/trailer markers there be mistaken for real document syntax.
FindStructurallyValidEndstream now rejects a candidate match unless it is
itself immediately followed (after an optional end-of-line sequence) by
the literal 'endobj' keyword - the structure every genuine stream object
has - continuing the search until one satisfying that check is found.
Added regression test
PdfDocument_Open_LinearScanFallback_EarlyFalseEndstreamWithIndirectLength_DoesNotCorruptOffsets,
verified to fail without this fix. Re-ran the originally reported crashing
file and all 115 files across the 5 external PDF corpora with an A/B
comparison confirming zero behavioral change from this fix on real-world
documents.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…dling
Replaces the line-start heuristic in the linear-scan fallback's object-header
and trailer detection with genuine PDF lexical-context tracking
(AdvancePastNonSyntax/SkipLiteralString/SkipHexString), so keyword matches
inside comments and string literals are structurally unreachable rather than
heuristically rejected, and valid headers not at the start of a line are no
longer wrongly rejected. Broadens the compressed-object-stream recovery catch
clause to also handle UnsupportedImageFeatureException (which shares no
common base with InvalidDataException), so an unrelated object stream with an
unsupported filter no longer aborts recovery of other, decodable object
streams.
Reframes the remaining raw-byte 'endstream' search (used when no trustworthy
/Length is available) as an explicitly accepted, inherent PDF-format
limitation matching documented qpdf/mutool industry practice, rather than a
gap to keep patching - a worst-case false match can only cause partial/no
recovery of an already-malformed document, never a crash.
Empirically verified via an external-corpus harness that 29/116 real-world
PDFs exercise this fallback path, and that the originally-reported crashing
file now opens successfully (PartialOk) instead of crashing.
Adds 3 new regression tests, each individually verified to fail without its
corresponding fix. Adds qpdf/mutool to the spelling dictionary.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…overy
- IsKeywordAt now rejects a match when the preceding byte is '/',
since a PDF name token (e.g. /trailer) begins with '/' directly
adjacent to its characters per the PDF name-object grammar. Without
this exclusion, a decoy name-keyed dictionary such as
'/trailer << /Root 1 0 R >>' could be mistaken for the real
'trailer' keyword and override the genuine trailer, losing
/Encrypt and breaking decryption.
- ScanForTrailerDictionary replaced its per-position List.Find
linear lookup against streamPayloadRanges with a monotonically
advancing range index, exploiting that the ranges are produced by
a single increasing-position scan. This changes the fallback
trailer scan from O(document-bytes * stream-count) to O(document-
bytes), removing a CPU/timeout risk on malformed inputs with many
stream objects.
- Added regression tests for both fixes:
PdfDocument_Open_LinearScanFallback_TrailerKeywordAsNameToken_DoesNotReplaceRealTrailer
PdfDocument_Open_LinearScanFallback_ManyStreamPayloadRangesBeforeTrailer_StillResolvesTrailer
The name-token fix was verified to make its test fail when reverted
and pass when restored.
- Updated requirements traceability and verification documentation
for CanvasNetPdf-PdfDocument-LinearScanFallback to reference both
new tests.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…y bugs in PDF fallback recovery
Addresses the two newest GitHub review findings plus three issues found via
local code-review subagents, all in PdfDocument.Xref.cs's fallback/recovery
scanning and classic cross-reference resolution:
- AdvancePastNonSyntax: bound total scan work to O(n) by permanently disabling
further literal/hex-string-open attempts for the remainder of a single pass
once any one string scan fails to find its close before EOF, closing a
quadratic-rescan CPU/timeout DoS on a crafted file with many unmatched '('
bytes (discussion_r4173305699).
- AdvancePastNonSyntax: exclude both characters of a dictionary's opening '<<'
from hex-string detection (previously only the first '<' was excluded),
preventing the second '<' from being misread as a hex-string start whose
early-closing '>' could expose embedded decoy object/trailer syntax
(discussion_r4173305727).
- FindStartXrefOffset: replace the unguarded LastIndexOf byte search with a
new LastIndexOfKeyword helper applying the same regular-byte boundary rule
used elsewhere in this file, so a byte-exact 'startxref' sequence that is
only the suffix of a larger token (e.g. 'xstartxref') is never mistaken for
a standalone marker - found via local review, not yet reported upstream.
- ParseIndirectObjectAt: validate a cross-reference entry's declared offset
against its original 64-bit value before narrowing to int, preventing an
out-of-range offset from silently wrapping around to an in-bounds value and
bypassing bounds validation entirely - found via local review.
- ParseIndirectObjectAt: cross-check the parsed header's own declared object
number against the number being resolved (mirroring the equivalent check
already performed for compressed objects), rejecting a cross-reference
entry whose offset leads to a different, shape-compatible object's header
instead of silently accepting and relabeling its content - found via local
review.
Each fix is covered by a new regression test and was rigor-verified by
temporarily reverting the fix, confirming the test fails (or, for the
quadratic-scan fix, times out) without it, then restoring the fix and
re-confirming the test passes - in all cases across net8.0/net9.0/net10.0.
Verified locally before push per request: build.ps1 (7380/7380 tests pass),
fix.ps1 (no unexpected diff), lint.ps1 (clean), reqstream --lint (clean), and
the external verify-harness corpus check (0 crashes/timeouts across 115
files plus the known previously-crashing fixture).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…on and comment handling
Addresses three new GitHub review findings on PdfDocument.Xref.cs's
fallback-recovery scanner:
- LastIndexOfKeyword (used by FindStartXrefOffset): reject a preceding '/'
byte, mirroring IsKeywordAt's existing name-token exclusion, so a PDF name
token such as /startxref can never be mistaken for the bare startxref
keyword and redirect cross-reference parsing to the wrong revision.
- IsFollowedByEndObjKeyword (used by FindStructurallyValidEndstream): skip
any PDF whitespace and comments - not merely an optional CRLF sequence -
before checking for the following endobj keyword, matching the PDF
grammar's own token-separation rules.
- TryParseObjectHeaderBackward: honor PDF comments as token separators
between an object header's number, generation, and obj keyword, via a new
bounded backward whitespace-and-comment skip, so a header such as
\1 0 % comment\\nobj\ is recognized the same way a real forward parser
would find it.
Both new comment-skipping helpers (forward and backward) are bounded to a
small fixed window to keep the scan strictly linear and avoid reintroducing
the quadratic-scan risk fixed earlier in this same file.
Added five new regression tests covering all three fixes plus the existing
token-suffix exclusion pattern, each rigor-verified by temporarily reverting
the fix and confirming the new test fails before restoring it. Updated
requirements traceability and verification documentation for the new tests.
Full local verification: build.ps1 (7389/7389 tests passed across
net8.0/net9.0/net10.0), fix.ps1 (clean diff), lint.ps1 (clean), and an
external 115-file PDF corpus check (0 crashes/timeouts).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This checks AdvancePastNonSyntax before checking whether i is inside one of the protected stream-payload ranges. If a binary payload starts with ( or < and its closing delimiter occurs after the payload, SkipLiteralString/SkipHexString can advance past range.End and over the real trailer before the range check runs; an encrypted fallback document can then lose /Encrypt (and object-stream recovery) or otherwise miss the trailer. Test range membership and jump past the payload before applying lexical skipping, or bound the skip to the current range.
…l review
Found by running local code-review and rubber-duck review agents against
the same bug class as the preceding GitHub-reported findings, per request
to catch these locally instead of via paid online review rounds:
- TryGetDeclaredStreamEnd only tolerated a single optional CRLF between a
stream's correctly declared /Length payload end and the endstream
keyword, not arbitrary whitespace and comments (same gap already fixed
in IsFollowedByEndObjKeyword and TryParseObjectHeaderBackward). A
genuinely correct declared length separated by a comment was wrongly
distrusted, falling through to the much weaker raw endstream scan and
risking acceptance of an early, coincidental endstream/endobj pair
inside the payload - ending the protected payload range too soon.
- SkipMarkerWhitespaceAndCommentsForward's bounded 256-byte scan window
could return a position still inside an unterminated comment if the
comment's own end-of-line terminator fell outside that window while the
buffer continued further - letting a caller (IsFollowedByEndObjKeyword)
misread comment-body bytes that happened to spell a keyword as a real
one. Now returns the comment's own start position in that ambiguous
case, which can never match a keyword, so the caller safely treats it
as no boundary found rather than guessing.
Added regression tests for both (reverting each fix independently
confirmed the corresponding new test fails, then passes once restored).
Full build.ps1 (7395/7395 across net8.0/net9.0/net10.0), fix.ps1, and
lint.ps1 all clean; external 115-file corpus check unchanged (0
crashes/timeouts).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The stream-range guard runs after AdvancePastNonSyntax. If a stream payload begins with ( or <, this call scans inside the arbitrary payload before the guard can jump over it; an unterminated delimiter can even advance past the range and skip the real trailer. Check whether i is inside the next payload range before attempting comment/string tracking, then jump directly to its end.
…railer-scan range ordering
- Add TryGetNonNegativeInt helper validating a parsed PdfToken.Number
(double) is representable as a non-negative int before narrowing,
replacing bare (int) casts that could silently truncate a
non-integer value (e.g. 1.5 -> 1) instead of rejecting malformed
input. Applied in RegisterCompressedObjectsFromObjectStreams,
LoadCompressedObject, and ParseIndirectObjectAt.
- Reorder ScanForTrailerDictionary's stream-payload-range membership
check to run before AdvancePastNonSyntax is consulted, mirroring
ScanObjectOffsets' already-safe pattern: a payload beginning with an
unescaped '(' or '<' must never be treated as real PDF string syntax
before the range guard has a chance to skip it.
- Add regression tests for all three truncation fixes (classic-xref
object header, direct xref-stream compressed-object path, and the
linear-scan-fallback compressed-object path), each verified via a
revert/restore cycle to fail without the corresponding fix.
- Update reqstream/verification docs for the new tests, and document
the trailer-scan reordering's reasoning (including why a dedicated
fixture for it does not discriminate buggy vs. fixed behavior given
how streamPayloadRanges are computed).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The fallback trusts /N by narrowing its parsed double with (int), so a malformed object stream declaring /N 1.5 is treated as one entry and its contained objects can be registered. This contradicts the surrounding integer-validation hardening and can make fallback accept a grammatically invalid stream (or wrap an out-of-range value); validate /N with TryGetNonNegativeInt before comparing it with decoded.Length.
This says no separate DPI-specific background-color test is needed, but the PR adds PdfDocument_RenderWithDpi_PropagatesOptionsBackgroundColor and registers it in the requirements. Please list that test here and describe it as the verification of the forwarding behavior so this verification document matches the actual test suite.
…string trade-off
- RegisterCompressedObjectsFromObjectStreams and LoadCompressedObject now
validate their declared /N (and /First, for LoadCompressedObject) as
non-negative integers instead of narrowing via a bare (int) cast, closing
the same truncation bug class already fixed for per-entry object numbers.
LoadCompressedObject is reachable from normal, non-fallback parsing of any
PDF using real xref-stream compressed objects, giving this fix a broader
blast radius than the sibling fallback-only check.
- Tried and reverted a stricter AdvancePastNonSyntax behavior that stopped
the linear-scan fallback entirely on the first unterminated string: a
local code-review pass found this caused any incidental unmatched
delimiter anywhere in a file - ordinary binary noise, not an attack - to
abandon recovery of an otherwise perfectly valid document. Restored the
original quadratic-protection-only behavior and documented the trade-off
explicitly in the XML doc remarks.
- Added PdfDocument_Open_LinearScanFallback_UnrelatedUnterminatedLiteralStringBeforeObjectHeader_StillResolvesDocument
to regression-test that an unrelated unterminated-string run before real
object headers does not break recovery.
- Removed the now-invalid regression test for the reverted strict behavior.
- Updated reqstream and verification docs for the net test changes.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This verification section is only partially updated for the new options parameter: its heading and the DPI/test-call descriptions above still refer to Render(int, float), and the forwarding sentence still names Render(int, int, int). Update the whole section to use the current Render(int, float, PdfRenderOptions?) and Render(int, int, int, PdfRenderOptions?) signatures so the verification document does not describe APIs that no longer exist.
…t one
RegisterCompressedObjectsFromObjectStreams previously refused to overwrite any already-registered xref entry, regardless of which definition physically occurred later in the file. This contradicted ScanObjectOffsets' own 'later occurrence wins' rule for direct N G obj headers, and meant an incrementally updated document that redefines an object via a compressed object stream after an earlier direct definition would silently keep the stale direct definition.
Add GetPhysicalPosition to compare a direct offset against a compressed entry's containing stream offset, and only overwrite an existing entry when the candidate is physically later. Also evict any stale _objectCache entry for the object number being overwritten, since an earlier iteration of the same registration loop may already have resolved and cached the prior value while probing other object numbers for ObjStm candidacy.
Add a regression test proving the later compressed redefinition - not the earlier direct one - is what gets resolved and rendered.
Also fix stale Render(int, float)/Render(int, int, int) signatures in the RenderWithDpi verification doc section to the current PdfRenderOptions?-accepting overloads.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Description
Fixes two real-world PDF issues:
Crash on linear-scan fallback recovery (
PdfDocument.Open): a real-world PDF(
BR_Cavro_Air_Displacement_Pipettor_396359.pdf) crashed withInvalidDataException: "Invalid character in hexadecimal string."while recovering viathe linear-scan fallback (
BuildLinearScanFallback/ScanObjectOffsets). Root cause: thefallback tokenized the entire raw document buffer - including binary/compressed stream
payloads never meant to be parsed as PDF syntax - with the general-purpose
PdfTokenizer,and a stray
<byte in binary noise threw uncaught. Worse, even once the throw wascaught, the same general-purpose tokenization badly desynced against binary noise
(misreading it as implausible number/string tokens), causing the scan to miss the vast
majority of real objects in the file. Objects stored only inside compressed
/Type /ObjStmobject streams were also never recoverable by this fallback at all (no literal
N G objmarker for them). Fixed by:
ScanObjectOffsets/ScanForTrailerDictionaryto locateN G obj/trailermarkers via raw byte-pattern scanning instead of general-purpose tokenization, which is
far more resilient to binary stream noise.
RegisterCompressedObjectsFromObjectStreams()to recover objects nested insidediscovered object streams during the fallback.
already-known/documented
JPXDecodelimitation affecting 3 pages) and against all 5external PDF corpora (115 files): zero crashes, zero timeouts, zero regressions.
Missing configurable page background color:
PdfDocument.Renderalways produced afully transparent background, unlike essentially every other PDF renderer, which defaults
to opaque white. Added a new
PdfRenderOptionstype (an extensible options record/class,intentionally designed so future render-time options can be added without further breaking
changes) with a
BackgroundColorproperty defaulting to opaque white, and a new trailingPdfRenderOptions? options = nullparameter on bothRenderoverloads. This is a 0.0.0pre-release package with no external consumers, so this breaking change was accepted now
deliberately to avoid piecemeal breaking changes later as more render options are added.
Existing test call sites that rely on isolating painted regions against a transparent
background were updated to pass an explicit transparent
PdfRenderOptionsoverride,preserving their original intent.
Type of Change
Related Issues
Closes #
Pre-Submission Checklist
Build and Test
pwsh ./build.ps1(7314/7314 tests passed, net8.0/net9.0/net10.0)Code Quality
Quality Checks
pwsh ./lint.ps1(markdownlint-cli2, cspell, yamllint, dotnet format, reqstream, versionmark, reviewmark, sysml2tools)Testing
minimal malformed PDF fixtures, plus default/custom/transparent
PdfRenderOptionsbehaviortests)
Render(...)call sites audited;paint-isolation tests updated to pass an explicit transparent background override)
Documentation
model, and user guide updated for the new configurable background color
Additional Notes
External PDF corpus harness (5 corpora, 115 files total) re-run against this branch's PDF
project: zero crashes, zero timeouts, consistent with prior baselines.