From 74b5ee46a2397c113498ab159b48de552508b58d Mon Sep 17 00:00:00 2001 From: Timothy van der Ham Date: Sat, 5 Sep 2026 00:37:59 +0200 Subject: [PATCH 1/5] fix(conformance): bound the message a preflight finding retains PdfPreflight.Validate keeps every PreflightAssertion for the result's lifetime, and several rules interpolated a producer-controlled name or string into the finding message whole. A 900,000-byte /Filter name shared by 400 streams retained 705.7 MiB from a 990 KB file (#403); ISO 32000-2 Annex C.1 sets no bound on a name's length (Table C.1's 127 bytes is informative only). Two layers: Layer A (the sink). PreflightContext.Report now cuts any message over MaxMessageChars (1024) to "... ( chars)", with a surrogate-pair check so the cut never leaves a lone high surrogate in the retained string (messages built from UTF-16BE text can put a pair astride the boundary). This alone bounds every message, including the "Rule evaluation failed: {ex.Message}" wrapper in PdfPreflight.cs, which can quote a whole oversized token thrown by the reader. Layer B (the name sites). The nine PdfName.Value interpolations that name a producer-controlled value in a report message (StreamRule's forbidden-filter message, both ActionRule messages, AnnotationRule's /AP-extra-key message, FontStructureRule's Type0 CMap message, LogicalStructureRule's /RoleMap message, PermissionsRule's /Perms-key message, BlendModeRule's /BM message, UaCMapRule's UA-1 CMap message) now quote through DiagnosticExcerpt.Quote, keeping the sentence shape ("...the /AAAA...AAA... (1048576 bytes) filter...") instead of a mid-sentence cut from the sink. AnnotationRule additionally quotes the /Subtype text once where it builds `label`, since that string is reused by the nine messages that follow it; a site whose value arrives already typed `string` (op, resourceName, and the other string-typed sites) is left to the sink, since it is written once per occurrence, not reused. Bound, before -> after: message length at a Layer B site for an N-byte name, about N+60 -> at most 54 (the 32-character excerpt plus the "... (N bytes)" suffix) + the fixed sentence for any int N; message length at every other site, unbounded -> at most 1046 for any int length; the 400-page issue shape, 705.7 MiB retained -> longest message under 1046 chars, sum of all message lengths under 128 KiB. Verdicts, rule ids, clauses, and assertion counts are unchanged; a message for a name of 32 characters or fewer is byte-identical to before. New tests/VellumPdf.Conformance.Tests/PreflightMessageBoundTests.cs, kept independent of PdfPreflightTests.cs (12,000+ lines already) with its own copy of AssemblePdf. Tests 1-3 and 5 (all but the boundary Theory, which cannot compile before MaxMessageChars exists) were run against 52c403e before this fix and observed failing: all 12 relevant cases failed, 11 with "Strings differ" (the unbounded message doesn't match the fixed-excerpt expectation) and FourHundredPagesSharingOneOversizedFilterName_retainOnlyBoundedMessages with "404 out of 407 items in the collection did not pass" its length bound. After the fix, all 15 cases in the new class pass. Self-review: git diff --stat touches only the ten rule/sink files, the CHANGELOG, and the new test file (no PublicAPI.*.txt, nothing under VellumPdf.Reader, VellumPdf.Cli, or TestSupport). grep for '.Value}' outside DiagnosticExcerpt.Quote in src/VellumPdf.Conformance finds only numeric or box.Value interpolations, already bounded by type. grep for "new PreflightAssertion(" finds exactly the one call site in PreflightContext.Report. dotnet build: 0 warnings, 0 errors. dotnet format --verify-no-changes: clean. eng/clean-room-check.ps1: passed. Full test runs with QPDF_HOME, POPPLER_HOME, VERAPDF_HOME, REQUIRE_ORACLES=1, REQUIRE_VERAPDF=1 set: Conformance 1296 total (1281 baseline + 15 new), 0 failed, 0 skipped; Reader 1349 total, 0 failed, 12 skipped; Cli 908 total, 0 failed, 0 skipped, matching the pre-change baselines plus the new class. No added em dash or double hyphen, no added hollow intensifier, no review-history narration left in shipped comments. --- CHANGELOG.md | 6 + src/VellumPdf.Conformance/PdfPreflight.cs | 4 + .../Rules/Actions/ActionRule.cs | 8 +- .../Rules/Annotations/AnnotationRule.cs | 11 +- .../Rules/Fonts/FontStructureRule.cs | 6 +- .../Rules/PreflightContext.cs | 32 +- .../Rules/Structure/LogicalStructureRule.cs | 4 +- .../Rules/Structure/PermissionsRule.cs | 4 +- .../Rules/Structure/StreamRule.cs | 6 +- .../Rules/Transparency/BlendModeRule.cs | 4 +- .../Rules/Ua/UaCMapRule.cs | 5 +- .../PreflightMessageBoundTests.cs | 528 ++++++++++++++++++ 12 files changed, 603 insertions(+), 15 deletions(-) create mode 100644 tests/VellumPdf.Conformance.Tests/PreflightMessageBoundTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index a15f365b..6f1f17e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -131,6 +131,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). writes `-4` instead of `-516`, the same value as `All`; at `/R` 6 the `/Perms` seal (Algorithm 10) changes with it, since it seals the same `/P` value. A document written without `Extract` therefore reads back with `Extract` included in `PdfEncryptionInfo.Permissions`. (#397) +- **A preflight finding could retain a producer-sized string for the result's lifetime.** A rule + that names the offending value in its message interpolated it whole, so one oversized `/Filter` + name shared by 400 streams kept 705.7 MiB of message text alive from a 990 KB file. Every + `PreflightAssertion.Message` is now cut at 1024 characters with its length appended, and the eight + rules that quote a name keep the first 32 characters plus the byte count. Verdicts, rule ids and + counts are unchanged; a message for a value of 32 characters or fewer is byte-identical. (#403) ## [2.3.0] - 2026-09-01 diff --git a/src/VellumPdf.Conformance/PdfPreflight.cs b/src/VellumPdf.Conformance/PdfPreflight.cs index 302d8370..cd3a603a 100644 --- a/src/VellumPdf.Conformance/PdfPreflight.cs +++ b/src/VellumPdf.Conformance/PdfPreflight.cs @@ -342,6 +342,10 @@ internal static PreflightResult Validate(PdfDocumentReader reader, PdfConformanc // starts), but should a future lazily-decoded reader feature let a rule raise either, // "cannot evaluate" and "wrong password" are both a distinct signal that should // propagate to the caller rather than be reported as a conformance violation. + // + // ex.Message can itself quote a whole oversized token (a filter name, a raw + // parser token); PreflightContext.Report bounds the retained message at + // MaxMessageChars, so that whole is not retained here either (#403). context.Report(rule.RuleId, rule.Clause, PreflightSeverity.Error, $"Rule evaluation failed: {ex.Message}"); } diff --git a/src/VellumPdf.Conformance/Rules/Actions/ActionRule.cs b/src/VellumPdf.Conformance/Rules/Actions/ActionRule.cs index e489bb68..4045ce3c 100644 --- a/src/VellumPdf.Conformance/Rules/Actions/ActionRule.cs +++ b/src/VellumPdf.Conformance/Rules/Actions/ActionRule.cs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 using VellumPdf.Core; +using VellumPdf.Reader; namespace VellumPdf.Conformance.Rules.Actions; @@ -137,7 +138,8 @@ private void CheckAction(PreflightContext context, PdfObject? actionObj, HashSet s is null ? "An action dictionary has no /S action-type key (only GoTo, GoToR, GoToE, Thread, " + "URI, Named, and SubmitForm are permitted in PDF/A)." - : $"The action type /{s.Value} is not permitted in PDF/A."); + : $"The action type /{DiagnosticExcerpt.Quote(s.Value)} is not permitted " + + "in PDF/A."); } else if (s.Value == "Named" && context.Resolve(action.Get(_n)) is PdfName named @@ -148,8 +150,8 @@ s is null "ISO19005-2:6.5.1-named-action", "ISO 19005-2:2011, 6.5.1", PreflightSeverity.Error, - $"The named action /{named.Value} is not permitted in PDF/A " - + "(only NextPage, PrevPage, FirstPage, and LastPage are allowed)."); + $"The named action /{DiagnosticExcerpt.Quote(named.Value)} is not permitted " + + "in PDF/A (only NextPage, PrevPage, FirstPage, and LastPage are allowed)."); } // /Next is either a single action or an array of actions executed afterwards. diff --git a/src/VellumPdf.Conformance/Rules/Annotations/AnnotationRule.cs b/src/VellumPdf.Conformance/Rules/Annotations/AnnotationRule.cs index 17525740..d857f99f 100644 --- a/src/VellumPdf.Conformance/Rules/Annotations/AnnotationRule.cs +++ b/src/VellumPdf.Conformance/Rules/Annotations/AnnotationRule.cs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 using VellumPdf.Core; +using VellumPdf.Reader; namespace VellumPdf.Conformance.Rules.Annotations; @@ -63,7 +64,12 @@ public void Evaluate(PreflightContext context) continue; } - var label = subtype is null ? "An annotation" : $"A /{subtype} annotation"; + // Quoted here, once, because label is reused by the nine messages below; an unquoted + // oversized /Subtype would otherwise be cut by the sink in each of them instead of + // excerpted once (#403). + var label = subtype is null + ? "An annotation" + : $"A /{DiagnosticExcerpt.Quote(subtype)} annotation"; // §6.3.2: a /Popup annotation is driven by its parent and is exempt from the annotation // flag requirements (it need not even carry an /F). Every other annotation's flags are @@ -95,7 +101,8 @@ public void Evaluate(PreflightContext context) { apHasOnlyN = false; context.Report(RuleId, Clause, PreflightSeverity.Error, - $"{label}'s appearance dictionary (/AP) shall contain only the /N entry (found /{entry.Key.Value})."); + $"{label}'s appearance dictionary (/AP) shall contain only the /N " + + $"entry (found /{DiagnosticExcerpt.Quote(entry.Key.Value)})."); break; } diff --git a/src/VellumPdf.Conformance/Rules/Fonts/FontStructureRule.cs b/src/VellumPdf.Conformance/Rules/Fonts/FontStructureRule.cs index 511d70d8..3730fd65 100644 --- a/src/VellumPdf.Conformance/Rules/Fonts/FontStructureRule.cs +++ b/src/VellumPdf.Conformance/Rules/Fonts/FontStructureRule.cs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 using VellumPdf.Core; +using VellumPdf.Reader; namespace VellumPdf.Conformance.Rules.Fonts; @@ -399,8 +400,9 @@ private void CheckCMapEncoding(PreflightContext context, PdfDictionary font) return; if (context.Resolve(font.Get(_encoding)) is PdfName name && !_predefinedCMaps.Contains(name.Value)) Report(context, "6.2.11.3.3-cmap-name", "ISO 19005-2:2011, 6.2.11.3.3", - $"A composite font's /Encoding names the CMap /{name.Value}, which is neither one of the " - + "predefined CMaps nor an embedded CMap stream."); + $"A composite font's /Encoding names the CMap " + + $"/{DiagnosticExcerpt.Quote(name.Value)}, which is neither one of the predefined " + + "CMaps nor an embedded CMap stream."); } // §6.2.11.3.1-1: the CIDSystemInfo of a composite font's descendant CIDFont and its CMap must be diff --git a/src/VellumPdf.Conformance/Rules/PreflightContext.cs b/src/VellumPdf.Conformance/Rules/PreflightContext.cs index 070802ee..837e8202 100644 --- a/src/VellumPdf.Conformance/Rules/PreflightContext.cs +++ b/src/VellumPdf.Conformance/Rules/PreflightContext.cs @@ -515,11 +515,25 @@ public IEnumerable EnumerateStreams() /// public ReadOnlyMemory DecryptedRawBody(ParsedStream stream) => Reader.DecryptedStreamView(stream).RawBody; + /// + /// The longest message a retains. A message identifies a + /// finding; it does not carry the producer's value. Many rules interpolate a name, a string or + /// a keyword the document controls, and ISO 32000-2 Annex C.1 sets no bound on any of those + /// ("In general, this PDF standard does not restrict the size or quantity of things described + /// in the PDF file format"), so without this cut a 900,000-byte /Filter name shared by 400 + /// streams retained 705.7 MiB from a 990 KB file (#403). 1024 characters is roughly twice the + /// longest sentence any rule composes on its own (522 characters, A2aContentItemTaggingRule) + /// and short enough that a result list of thousands of findings stays a few megabytes. The + /// token-level counterpart in the Reader is . + /// + internal const int MaxMessageChars = 1024; + /// Records a finding for the current validation pass. /// Stable rule identifier (typically the rule's ). /// Specification clause citation. /// The finding's severity. - /// Human-readable description. + /// Human-readable description. Text past + /// characters is replaced by ... (N chars), N being the full length. /// Optional "N 0 R" object location. public void Report( string ruleId, @@ -527,5 +541,19 @@ public void Report( PreflightSeverity severity, string message, string? objectRef = null) - => _assertions.Add(new PreflightAssertion(ruleId, clause, severity, message, objectRef)); + { + if (message.Length > MaxMessageChars) + { + // A message can end mid-surrogate-pair when the producer value came from UTF-16BE + // text (A2aLangSyntaxRule, UaLangSyntaxRule, XmpPacket): cutting inside the pair would + // leave a lone high surrogate in the retained string, which the CLI's SARIF writer + // (Formatter.WriteSarif) would then have to serialize (#403). + var cut = MaxMessageChars; + if (char.IsHighSurrogate(message[cut - 1])) + cut--; + message = $"{message[..cut]}... ({message.Length} chars)"; + } + + _assertions.Add(new PreflightAssertion(ruleId, clause, severity, message, objectRef)); + } } diff --git a/src/VellumPdf.Conformance/Rules/Structure/LogicalStructureRule.cs b/src/VellumPdf.Conformance/Rules/Structure/LogicalStructureRule.cs index 78930086..73c03e62 100644 --- a/src/VellumPdf.Conformance/Rules/Structure/LogicalStructureRule.cs +++ b/src/VellumPdf.Conformance/Rules/Structure/LogicalStructureRule.cs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 using VellumPdf.Core; +using VellumPdf.Reader; namespace VellumPdf.Conformance.Rules.Structure; @@ -69,7 +70,8 @@ private void CheckRoleMapAcyclic(PreflightContext context, PdfDictionary roleMap RuleId, Clause, PreflightSeverity.Error, - $"The structure tree /RoleMap entry /{entry.Key.Value} shall map to a name."); + $"The structure tree /RoleMap entry " + + $"/{DiagnosticExcerpt.Quote(entry.Key.Value)} shall map to a name."); } } diff --git a/src/VellumPdf.Conformance/Rules/Structure/PermissionsRule.cs b/src/VellumPdf.Conformance/Rules/Structure/PermissionsRule.cs index 25a730d1..31a1a1f0 100644 --- a/src/VellumPdf.Conformance/Rules/Structure/PermissionsRule.cs +++ b/src/VellumPdf.Conformance/Rules/Structure/PermissionsRule.cs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 using VellumPdf.Core; +using VellumPdf.Reader; namespace VellumPdf.Conformance.Rules.Structure; @@ -39,7 +40,8 @@ public void Evaluate(PreflightContext context) continue; context.Report( RuleId, Clause, PreflightSeverity.Error, - $"The permissions dictionary contains the key /{entry.Key.Value}; only /UR3 and /DocMDP " + $"The permissions dictionary contains the key " + + $"/{DiagnosticExcerpt.Quote(entry.Key.Value)}; only /UR3 and /DocMDP " + "are permitted in PDF/A-2."); return; // One report suffices; the verdict is unaffected by the count. } diff --git a/src/VellumPdf.Conformance/Rules/Structure/StreamRule.cs b/src/VellumPdf.Conformance/Rules/Structure/StreamRule.cs index 3d455fc2..484250e8 100644 --- a/src/VellumPdf.Conformance/Rules/Structure/StreamRule.cs +++ b/src/VellumPdf.Conformance/Rules/Structure/StreamRule.cs @@ -150,11 +150,15 @@ private static bool CheckOneFilter(PreflightContext context, PdfName filterName, } // Any other filter name (including LZWDecode) is forbidden. + // The name is producer-controlled and unbounded (Annex C.1); the excerpt is what + // the retained message keeps of it (#403). NumericLimitsRule reports the length + // violation itself. context.Report( "ISO19005-2:6.1.7.2-1-filter", "ISO 19005-2:2011, 6.1.7.2", PreflightSeverity.Error, - $"A stream uses the /{filterName.Value} filter, which is not permitted in PDF/A-2."); + $"A stream uses the /{DiagnosticExcerpt.Quote(filterName.Value)} filter, " + + "which is not permitted in PDF/A-2."); return true; } diff --git a/src/VellumPdf.Conformance/Rules/Transparency/BlendModeRule.cs b/src/VellumPdf.Conformance/Rules/Transparency/BlendModeRule.cs index 603df571..46c3bb34 100644 --- a/src/VellumPdf.Conformance/Rules/Transparency/BlendModeRule.cs +++ b/src/VellumPdf.Conformance/Rules/Transparency/BlendModeRule.cs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 using VellumPdf.Core; +using VellumPdf.Reader; namespace VellumPdf.Conformance.Rules.Transparency; @@ -98,7 +99,8 @@ private void ReportIfInvalid(PreflightContext context, PdfName blendMode) RuleId, Clause, PreflightSeverity.Error, - $"The blend mode /{blendMode.Value} is not one of the standard blend modes permitted in PDF/A-2."); + $"The blend mode /{DiagnosticExcerpt.Quote(blendMode.Value)} is not one of the " + + "standard blend modes permitted in PDF/A-2."); } } } diff --git a/src/VellumPdf.Conformance/Rules/Ua/UaCMapRule.cs b/src/VellumPdf.Conformance/Rules/Ua/UaCMapRule.cs index acd5407f..23cc6b08 100644 --- a/src/VellumPdf.Conformance/Rules/Ua/UaCMapRule.cs +++ b/src/VellumPdf.Conformance/Rules/Ua/UaCMapRule.cs @@ -81,8 +81,9 @@ public void Evaluate(PreflightContext context) RuleId1, Clause, PreflightSeverity.Error, - $"A composite font's /Encoding names the CMap /{nameVal.Value}, which is neither " - + "one of the predefined CMaps nor an embedded CMap stream (§7.21.3.3)."); + $"A composite font's /Encoding names the CMap " + + $"/{DiagnosticExcerpt.Quote(nameVal.Value)}, which is neither one of the " + + "predefined CMaps nor an embedded CMap stream (§7.21.3.3)."); } // Predefined name (including Identity-H/V): §7.21.3.3-2/-3 have no embedded program // to check — they do not apply to predefined-name encodings. diff --git a/tests/VellumPdf.Conformance.Tests/PreflightMessageBoundTests.cs b/tests/VellumPdf.Conformance.Tests/PreflightMessageBoundTests.cs new file mode 100644 index 00000000..7aa01f7d --- /dev/null +++ b/tests/VellumPdf.Conformance.Tests/PreflightMessageBoundTests.cs @@ -0,0 +1,528 @@ +// Copyright © Timothy van der Ham (@Tim81) +// SPDX-License-Identifier: Apache-2.0 + +using System.Linq; +using System.Text; +using VellumPdf.Canvas; +using VellumPdf.Conformance.Rules; +using VellumPdf.Core; +using VellumPdf.Document; +using VellumPdf.Fonts; +using VellumPdf.Reader; + +namespace VellumPdf.Conformance.Tests; + +/// +/// Value-level tests for the #403 message bound: cuts a +/// message at , and the nine sites that quote a +/// producer whole excerpt it through +/// first, so the sentence shape survives instead of +/// being cut mid-word by the sink. +/// +public sealed class PreflightMessageBoundTests +{ + // ── Fixture helpers (copied from PdfPreflightTests.AssemblePdf and kept independent; that ──── + // file is over 12,000 lines already and does not need a #403-only dependency added to it) ──── + + /// + /// A single indirect object for . For a non-stream object, + /// is the complete object text. For a stream object, is + /// the dictionary's inner entries only; the assembler wraps it and appends the /Length. + /// + private sealed record PdfObj(string Dict, byte[]? Stream = null); + + private static readonly PdfObj _pagesObj = new("<< /Type /Pages /Kids [3 0 R] /Count 1 >>"); + private static readonly PdfObj _pageObj = new("<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] >>"); + + private static byte[] AssemblePdf( + IReadOnlyList objects, + string xmpConformance = "B") + { + var all = new List(objects); + + var metaObjNum = all.Count + 1; + var xmp = XmpBytes("2", xmpConformance); + all.Add(new PdfObj("/Type /Metadata /Subtype /XML", xmp)); + all[0] = all[0] with { Dict = InjectIntoDict(all[0].Dict, $"/Metadata {metaObjNum} 0 R") }; + + var ms = new MemoryStream(); + void W(string s) => ms.Write(Encoding.ASCII.GetBytes(s)); + + W("%PDF-1.7\n"); + ms.Write([(byte)'%', 0xE2, 0xE3, 0xCF, 0xD3, (byte)'\n']); + + var offsets = new int[all.Count + 1]; + for (var i = 0; i < all.Count; i++) + { + offsets[i + 1] = (int)ms.Position; + var n = i + 1; + if (all[i].Stream is { } body) + { + W($"{n} 0 obj\n<< {all[i].Dict} /Length {body.Length} >>\nstream\n"); + ms.Write(body); + W("\nendstream\nendobj\n"); + } + else + { + W($"{n} 0 obj\n{all[i].Dict}\nendobj\n"); + } + } + + var xrefOffset = (int)ms.Position; + var size = all.Count + 1; + W($"xref\n0 {size}\n"); + W($"{0:D10} 65535 f \n"); + for (var i = 1; i <= all.Count; i++) + W($"{offsets[i]:D10} 00000 n \n"); + W($"trailer\n<< /Size {size} /Root 1 0 R " + + "/ID [<00112233445566778899AABBCCDDEEFF> <00112233445566778899AABBCCDDEEFF>] >>\n"); + W($"startxref\n{xrefOffset}\n%%EOF\n"); + + return ms.ToArray(); + } + + private static string InjectIntoDict(string dict, string entry) + { + var i = dict.LastIndexOf(">>", StringComparison.Ordinal); + return i < 0 ? dict : string.Concat(dict[..i], entry, " ", dict[i..]); + } + + private static byte[] XmpBytes(string part, string conformance) + { + var xmp = + "" + + "" + + "" + + $"{part}" + + $"{conformance}" + + ""; + return Encoding.UTF8.GetBytes(xmp); + } + + /// + /// Builds a doc whose page's /Contents stream carries the given /Filter name. + /// + private static byte[] BuildOversizedFilterPdf(string filterName) + => AssemblePdf( + [ + new("<< /Type /Catalog /Pages 2 0 R >>"), + _pagesObj, + new("<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Contents 4 0 R >>"), + new($"/Filter /{filterName}", []), + ]); + + /// + /// Same shape as PdfPreflightTests.BuildFontPdf: a Type0 font selected via Tf. + /// + private static byte[] BuildFontPdf(params PdfObj[] fontObjects) + { + var contentObjNum = 6 + fontObjects.Length; + var objects = new List + { + new("<< /Type /Catalog /Pages 2 0 R >>"), + _pagesObj, + new($"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Resources 4 0 R " + + $"/Contents {contentObjNum} 0 R >>"), + new("<< /Font 5 0 R >>"), + new("<< /F0 6 0 R >>"), + }; + objects.AddRange(fontObjects); + objects.Add(new PdfObj(string.Empty, Encoding.ASCII.GetBytes("BT /F0 12 Tf ET"))); + return AssemblePdf(objects); + } + + /// + /// Builds a UA-1 tagged document with one embedded (Type0) font selected via Tf, mirroring + /// OracleCorpus.WriterPdfTagged. Kept independent because that method builds a + /// registered oracle fixture (#403 must not change what any oracle test validates). + /// + private static byte[] BuildUa1TaggedPdf() + { + using var doc = new PdfDocument + { + Conformance = VellumPdf.Document.PdfConformance.PdfUA1, + Language = "en-US", + }; + doc.Info.Title = "PreflightMessageBoundTests fixture"; + var page = doc.AddPage(PageSize.A4); + var handle = doc.EmbedStandard14Font(Standard14.Helvetica); + doc.RegisterEmbeddedFontUsage(page, handle); + + var canvas = new PdfCanvas(page); + var mcid = canvas.BeginMarkedContent("P"); + canvas.BeginText().SetFontByName(handle.ResourceName, 12).SetTextMatrix(1, 0, 0, 1, 72, 720); + var gids = new ushort[7]; + var count = handle.GetGlyphIds("Tagged.", gids); + canvas.ShowGlyphs(gids.AsSpan(0, count)); + canvas.EndText(); + canvas.EndMarkedContent(); + canvas.Finish(); + + var p = new PdfStructElem("P") { Page = page, Mcid = mcid }; + var root = new PdfStructElem("Document"); + root.AddChild(p); + doc.RegisterStructElem(root); + + using var ms = new MemoryStream(); + doc.Save(ms); + return ms.ToArray(); + } + + /// + /// Rewrites the fixture's Type0 font's /Encoding to an oversized name via an incremental + /// update, mirroring OracleCorpus.Ua1BadCMapName's clone-and-AppendRevision step (that + /// method itself is a registered oracle fixture and is not reused directly). + /// + private static byte[] BuildUa1OversizedCMapNamePdf(int nameBytes) + { + var baseline = BuildUa1TaggedPdf(); + using var reader = PdfReader.Open(baseline); + var pagesRef = (PdfIndirectReference)reader.Catalog.Get(PdfName.Pages)!; + var pages = (PdfDictionary)reader.Resolve(pagesRef.ObjectNumber)!; + var kidsObj = pages.Get(new PdfName("Kids")); + var kids = kidsObj is PdfIndirectReference kr + ? (PdfArray)reader.Resolve(kr.ObjectNumber)! + : (PdfArray)kidsObj!; + var pageRef = (PdfIndirectReference)kids[0]; + var page = (PdfDictionary)reader.Resolve(pageRef.ObjectNumber)!; + var resources = (PdfDictionary)reader.ResolveValue(page.Get(new PdfName("Resources"))!)!; + var fontDict = (PdfDictionary)reader.ResolveValue(resources.Get(PdfName.Font)!)!; + var type0Ref = (PdfIndirectReference)fontDict.Entries.First().Value; + var type0 = (PdfDictionary)reader.Resolve(type0Ref.ObjectNumber)!; + + var clone = new PdfDictionary(); + foreach (var kv in type0.Entries) + clone.Set(kv.Key, kv.Value); + clone.Set(new PdfName("Encoding"), new PdfName(new string('A', nameBytes))); + + return reader.AppendRevision([(type0Ref.ObjectNumber, 0, clone)]); + } + + // ── Site 1 / site 2: the shared oversized-/Filter fixture ─────────────────────────────────── + + [Fact] + public void StreamFilter_withAnOversizedName_reportsAFixedExcerpt() + { + var bytes = BuildOversizedFilterPdf(new string('A', 1_048_576)); + + var result = PdfPreflight.Validate(bytes, PdfConformance.PdfA2B); + + var assertion = Assert.Single( + result.Assertions, a => a.RuleId == "ISO19005-2:6.1.7.2-1-filter"); + Assert.Equal( + "A stream uses the /" + new string('A', 32) + "... (1048576 bytes) filter, " + + "which is not permitted in PDF/A-2.", + assertion.Message); + } + + [Fact] + public void RuleEvaluationFailure_withAnOversizedToken_isBoundedByTheSink() + { + var bytes = BuildOversizedFilterPdf(new string('A', 1_048_576)); + + var result = PdfPreflight.Validate(bytes, PdfConformance.PdfA2B); + + var full = "Rule evaluation failed: Unknown PDF filter: /" + new string('A', 1_048_576); + var expected = full[..1024] + $"... ({full.Length} chars)"; + + // Several rules try to decode the same oversized-filter /Contents stream and each wraps + // the same InvalidDataException as its own "Rule evaluation failed" finding (#403); assert + // every one of them, not just the first, since they all quote the identical thrown message. + var matching = result.Assertions + .Where(a => a.Message.StartsWith( + "Rule evaluation failed: Unknown PDF filter: /", StringComparison.Ordinal)) + .ToList(); + Assert.NotEmpty(matching); + Assert.All(matching, a => Assert.Equal(expected, a.Message)); + } + + // ── Sites 3-10: one test per Layer B site ──────────────────────────────────────────────────── + + [Fact] + public void ActionType_withAnOversizedName_reportsAFixedExcerpt() + { + var name = new string('A', 1_048_576); + var bytes = AssemblePdf( + [ + new($"<< /Type /Catalog /Pages 2 0 R /OpenAction << /S /{name} >> >>"), + _pagesObj, + _pageObj, + ]); + + var result = PdfPreflight.Validate(bytes, PdfConformance.PdfA2B); + + var assertion = Assert.Single( + result.Assertions, a => a.RuleId == "ISO19005-2:6.5.1-action"); + Assert.Equal( + "The action type /" + new string('A', 32) + "... (1048576 bytes) is not permitted in PDF/A.", + assertion.Message); + } + + [Fact] + public void NamedAction_withAnOversizedName_reportsAFixedExcerpt() + { + var name = new string('A', 1_048_576); + var bytes = AssemblePdf( + [ + new("<< /Type /Catalog /Pages 2 0 R /OpenAction 4 0 R >>"), + _pagesObj, + _pageObj, + new($"<< /S /Named /N /{name} >>"), + ]); + + var result = PdfPreflight.Validate(bytes, PdfConformance.PdfA2B); + + var assertion = Assert.Single( + result.Assertions, a => a.RuleId == "ISO19005-2:6.5.1-named-action"); + Assert.Equal( + "The named action /" + new string('A', 32) + "... (1048576 bytes) is not permitted " + + "in PDF/A (only NextPage, PrevPage, FirstPage, and LastPage are allowed).", + assertion.Message); + } + + [Fact] + public void AnnotationAppearanceExtraKey_withAnOversizedName_reportsAFixedExcerpt() + { + var key = new string('A', 1_048_576); + var bytes = AssemblePdf( + [ + new("<< /Type /Catalog /Pages 2 0 R >>"), + _pagesObj, + new("<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Annots [4 0 R] >>"), + new("<< /Type /Annot /Subtype /Text /Rect [10 10 50 50] /F 4 /Contents (n) " + + $"/AP << /N 5 0 R /{key} 5 0 R >> >>"), + new("/Type /XObject /Subtype /Form /BBox [0 0 1 1]", Stream: []), + ]); + + var result = PdfPreflight.Validate(bytes, PdfConformance.PdfA2B); + + var assertion = Assert.Single( + result.Assertions, + a => a.RuleId == "ISO19005-2:6.3-annotation" && a.Message.Contains("(/AP)", StringComparison.Ordinal)); + Assert.Equal( + "A /Text annotation's appearance dictionary (/AP) shall contain only the /N entry (found /" + + new string('A', 32) + "... (1048576 bytes)).", + assertion.Message); + } + + [Fact] + public void AnnotationLabel_withAnOversizedSubtype_reportsAFixedExcerptEverywhereItIsReused() + { + var subtype = new string('A', 1_048_576); + var bytes = AssemblePdf( + [ + new("<< /Type /Catalog /Pages 2 0 R >>"), + _pagesObj, + new("<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Annots [4 0 R] >>"), + new($"<< /Type /Annot /Subtype /{subtype} /Rect [10 10 50 50] >>"), + ]); + + var result = PdfPreflight.Validate(bytes, PdfConformance.PdfA2B); + + var label = "A /" + new string('A', 32) + "... (1048576 bytes) annotation"; + var annotationFindings = result.Assertions + .Where(a => a.RuleId == "ISO19005-2:6.3-annotation") + .ToList(); + // No /F and no /AP: the Print-flag message and the missing-appearance message both fire, + // and both must carry the excerpt rather than the whole /Subtype. + Assert.Equal(2, annotationFindings.Count); + Assert.All( + annotationFindings, + a => Assert.StartsWith(label, a.Message, StringComparison.Ordinal)); + Assert.Contains( + annotationFindings, + a => a.Message == label + " shall have the Print flag set."); + Assert.Contains( + annotationFindings, + a => a.Message == label + " shall have a normal appearance (/AP /N)."); + } + + [Fact] + public void Type0CMapName_withAnOversizedName_reportsAFixedExcerpt() + { + var cmapName = new string('A', 1_048_576); + var bytes = BuildFontPdf( + new PdfObj($"<< /Type /Font /Subtype /Type0 /BaseFont /X /Encoding /{cmapName} " + + "/DescendantFonts [7 0 R] >>"), + new PdfObj("<< /Type /Font /Subtype /CIDFontType2 /BaseFont /X /FontDescriptor 8 0 R " + + "/CIDToGIDMap /Identity >>"), + new PdfObj("<< /Type /FontDescriptor /FontName /X /FontFile2 9 0 R >>"), + new PdfObj("/Length1 4", [1, 2, 3, 4])); + + var result = PdfPreflight.Validate(bytes, PdfConformance.PdfA2B); + + var assertion = Assert.Single( + result.Assertions, a => a.RuleId == "ISO19005-2:6.2.11.3.3-cmap-name"); + Assert.Equal( + "A composite font's /Encoding names the CMap /" + new string('A', 32) + + "... (1048576 bytes), which is neither one of the predefined CMaps nor an embedded " + + "CMap stream.", + assertion.Message); + } + + [Fact] + public void RoleMapEntry_withAnOversizedName_reportsAFixedExcerpt() + { + // LogicalStructureRule's "shall map to a name" branch needs a /RoleMap value that is not a + // name (a name key alone cannot reach it); 42 is an arbitrary non-name. + var roleKey = new string('A', 1_048_576); + var bytes = AssemblePdf( + [ + new("<< /Type /Catalog /Pages 2 0 R /MarkInfo << /Marked true >> /StructTreeRoot 4 0 R >>"), + _pagesObj, + _pageObj, + new($"<< /Type /StructTreeRoot /RoleMap << /{roleKey} 42 >> >>"), + ], + xmpConformance: "A"); + + var result = PdfPreflight.Validate(bytes, PdfConformance.PdfA2A); + + var assertion = Assert.Single( + result.Assertions, a => a.RuleId == "ISO19005-2:6.8-logical-structure"); + Assert.Equal( + "The structure tree /RoleMap entry /" + new string('A', 32) + + "... (1048576 bytes) shall map to a name.", + assertion.Message); + } + + [Fact] + public void PermissionsKey_withAnOversizedName_reportsAFixedExcerpt() + { + var key = new string('A', 1_048_576); + var bytes = AssemblePdf( + [ + new($"<< /Type /Catalog /Pages 2 0 R /Perms << /{key} << /Type /Sig >> >> >>"), + _pagesObj, + _pageObj, + ]); + + var result = PdfPreflight.Validate(bytes, PdfConformance.PdfA2B); + + var assertion = Assert.Single( + result.Assertions, a => a.RuleId == "ISO19005-2:6.1.12-1-permissions"); + Assert.Equal( + "The permissions dictionary contains the key /" + new string('A', 32) + + "... (1048576 bytes); only /UR3 and /DocMDP are permitted in PDF/A-2.", + assertion.Message); + } + + [Fact] + public void BlendMode_withAnOversizedName_reportsAFixedExcerpt() + { + var bm = new string('A', 1_048_576); + var bytes = AssemblePdf( + [ + new("<< /Type /Catalog /Pages 2 0 R >>"), + _pagesObj, + new("<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Resources 4 0 R /Contents 7 0 R >>"), + new("<< /ExtGState 5 0 R >>"), + new("<< /GS0 6 0 R >>"), + new($"<< /Type /ExtGState /BM /{bm} >>"), + new(string.Empty, Encoding.ASCII.GetBytes("q /GS0 gs Q")), + ]); + + var result = PdfPreflight.Validate(bytes, PdfConformance.PdfA2B); + + var assertion = Assert.Single( + result.Assertions, a => a.RuleId == "ISO19005-2:6.2.10-blend-mode"); + Assert.Equal( + "The blend mode /" + new string('A', 32) + "... (1048576 bytes) is not one of the " + + "standard blend modes permitted in PDF/A-2.", + assertion.Message); + } + + [Fact] + public void UaType0CMapName_withAnOversizedName_reportsAFixedExcerpt() + { + var bytes = BuildUa1OversizedCMapNamePdf(1_048_576); + + var result = PdfPreflight.Validate(bytes, PdfConformance.PdfUA1); + + var assertion = Assert.Single( + result.Assertions, a => a.RuleId == "ISO14289-1:7.21.3.3-1"); + Assert.Equal( + "A composite font's /Encoding names the CMap /" + new string('A', 32) + + "... (1048576 bytes), which is neither one of the predefined CMaps nor an embedded " + + "CMap stream (§7.21.3.3).", + assertion.Message); + } + + // ── Layer A: the sink's own cut, at the boundary ───────────────────────────────────────────── + + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(2)] + public void Report_atTheBoundary_keepsExactlyMaxMessageChars(int scenario) + { + var bytes = AssemblePdf([new("<< /Type /Catalog /Pages 2 0 R >>"), _pagesObj, _pageObj]); + using var reader = PdfReader.Open(bytes); + var assertions = new List(); + var context = new PreflightContext(reader, PdfConformance.PdfA2B, assertions); + + var message = scenario switch + { + 0 => new string('x', PreflightContext.MaxMessageChars), + 1 => new string('x', PreflightContext.MaxMessageChars + 1), + _ => new string('x', 1023) + "\U0001F600" + "trailing text past the boundary", + }; + + context.Report("VellumTest:boundary", "n/a", PreflightSeverity.Error, message); + + var retained = Assert.Single(assertions).Message; + switch (scenario) + { + case 0: + Assert.Equal(message, retained); + break; + case 1: + Assert.Equal(message[..1024] + $"... ({message.Length} chars)", retained); + break; + default: + Assert.Equal(message[..1023] + $"... ({message.Length} chars)", retained); + break; + } + + Assert.False(retained.Length > 0 && char.IsHighSurrogate(retained[^1])); + } + + // ── The issue's own shape: 400 pages sharing one oversized /Filter name ───────────────────── + + [Fact] + public void FourHundredPagesSharingOneOversizedFilterName_retainOnlyBoundedMessages() + { + const int pageCount = 400; + const int nameBytes = 900_000; + var firstContentObj = 3 + pageCount; + var sharedFilterObj = 3 + pageCount * 2; + + var objects = new List + { + new("<< /Type /Catalog /Pages 2 0 R >>"), + new("<< /Type /Pages /Kids [" + + string.Join(" ", Enumerable.Range(3, pageCount).Select(n => $"{n} 0 R")) + + $"] /Count {pageCount} >>"), + }; + for (var i = 0; i < pageCount; i++) + objects.Add(new PdfObj( + $"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Contents {firstContentObj + i} 0 R >>")); + for (var i = 0; i < pageCount; i++) + objects.Add(new PdfObj($"/Filter {sharedFilterObj} 0 R", [])); + objects.Add(new PdfObj("/" + new string('A', nameBytes))); + + var bytes = AssemblePdf(objects, xmpConformance: "A"); + + var result = PdfPreflight.Validate(bytes, PdfConformance.PdfA2A); + + Assert.All(result.Assertions, a => Assert.True(a.Message.Length <= PreflightContext.MaxMessageChars + 22)); + + var totalLength = result.Assertions.Sum(a => a.Message.Length); + Assert.True(totalLength < 131_072, $"Total retained message length was {totalLength} chars."); + + Assert.Contains( + result.Assertions, + a => a.Message == "A stream uses the /" + new string('A', 32) + "... (900000 bytes) filter, " + + "which is not permitted in PDF/A-2."); + } +} From d28654bb1ca7084e990687e5c768ff238fc730ab Mon Sep 17 00:00:00 2001 From: Timothy van der Ham Date: Sat, 5 Sep 2026 03:01:15 +0200 Subject: [PATCH 2/5] fix(conformance): state the excerpt scope and pair a Changed entry Round 1 of #404 found the prose presenting the ten excerpt sites as the complete set of producer-controlled interpolations. They are not: about thirty more sites rely on the 1024-character sink cut alone, which cuts mid-value and loses the sentence's tail for an oversized value. Sweeping them is a separate change (#405); this commit makes the split explicit. - MaxMessageChars doc: names the ten sites and what they quote, says the sink cut is the only bound every other interpolation has, and records why the two bounds differ (a PdfName value is Latin-1, so Quote counts bytes; the sink sees UTF-16BE-decoded text and steps around a surrogate pair). - Report: the param doc states the step-back and the code-unit count; the guard is a property pattern so a null message still passes through to the constructor as before. - CHANGELOG: a Changed bullet for the message-text change, since Conformance is Shipped and the CLI's text, JSON and SARIF output carries the new text; the Fixed bullet says ten sites, 400 pages, and that the numbers were measured in #403. The transient allocation from the Reader's own unknown-filter throw is unchanged and tracked in #406. - Tests: the class doc counts ten sites and points at #405; the section headers drop their numbering; the surrogate assertion is Assert.DoesNotContain(retained, char.IsSurrogate), which fails for scenario 2 with the step-back removed (verified); the 400-page test names its 22-character suffix allowance and reports the rule id on a failure; BuildUa1TaggedPdf's doc gives the reason that holds (WriterPdfTagged is private to OracleCorpus); the redundant System.Linq using goes (ImplicitUsings is on). The commit message of 74b5ee4 described the Layer B criterion as "arrives already typed string, written once"; the criterion that holds is "interpolates a PdfName.Value whole". Pushed history, recorded here and in the PR body. --- CHANGELOG.md | 19 ++++++++--- .../Rules/PreflightContext.cs | 34 +++++++++++++------ .../PreflightMessageBoundTests.cs | 30 ++++++++++------ 3 files changed, 59 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f1f17e8..4e27c6e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -119,6 +119,15 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). document reports on re-opening now include `Extract`. Documents written with `Extract` (the default, since `All` includes it) carry the same `/P` value as before. See Fixed, below, for why. (#397) +- **A preflight finding's message text changes for a named value longer than 32 characters, and for + any message longer than 1024 characters.** Ten rule sites that quote a producer-supplied name (a + stream `/Filter`, an action `/S` or named action `/N`, an annotation `/Subtype` or an extra `/AP` + key, a composite font's `/Encoding` CMap name in two rules, a `/RoleMap` key, a `/Perms` key, a + blend mode) now keep the first 32 characters followed by `... (N bytes)`, and every + `PreflightAssertion.Message` past 1024 characters ends in `... (N chars)`. `vellum-preflight`'s + text, JSON and SARIF output carries the same text. Verdicts, rule ids, clauses and assertion + counts are unaffected, and a message whose named value is 32 characters or shorter is + byte-identical to before. See Fixed, below, for why. (#403) ### Fixed @@ -133,10 +142,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). `Extract` therefore reads back with `Extract` included in `PdfEncryptionInfo.Permissions`. (#397) - **A preflight finding could retain a producer-sized string for the result's lifetime.** A rule that names the offending value in its message interpolated it whole, so one oversized `/Filter` - name shared by 400 streams kept 705.7 MiB of message text alive from a 990 KB file. Every - `PreflightAssertion.Message` is now cut at 1024 characters with its length appended, and the eight - rules that quote a name keep the first 32 characters plus the byte count. Verdicts, rule ids and - counts are unchanged; a message for a value of 32 characters or fewer is byte-identical. (#403) + name shared by 400 pages kept 705.7 MiB of message text alive from a 990 KB file (measured in + #403). Every `PreflightAssertion.Message` is now cut at 1024 characters with its length appended, + and the ten sites that quoted a name whole keep the first 32 characters plus the byte count; the + remaining sites that interpolate a producer value rely on the 1024-character cut and are listed + in #405. What this bounds is the retained result; the transient allocation the Reader makes while + building its own exception message for an unknown filter is unchanged and tracked in #406. (#403) ## [2.3.0] - 2026-09-01 diff --git a/src/VellumPdf.Conformance/Rules/PreflightContext.cs b/src/VellumPdf.Conformance/Rules/PreflightContext.cs index 837e8202..f21db1bd 100644 --- a/src/VellumPdf.Conformance/Rules/PreflightContext.cs +++ b/src/VellumPdf.Conformance/Rules/PreflightContext.cs @@ -517,14 +517,26 @@ public IEnumerable EnumerateStreams() /// /// The longest message a retains. A message identifies a - /// finding; it does not carry the producer's value. Many rules interpolate a name, a string or - /// a keyword the document controls, and ISO 32000-2 Annex C.1 sets no bound on any of those - /// ("In general, this PDF standard does not restrict the size or quantity of things described - /// in the PDF file format"), so without this cut a 900,000-byte /Filter name shared by 400 - /// streams retained 705.7 MiB from a 990 KB file (#403). 1024 characters is roughly twice the - /// longest sentence any rule composes on its own (522 characters, A2aContentItemTaggingRule) - /// and short enough that a result list of thousands of findings stays a few megabytes. The - /// token-level counterpart in the Reader is . + /// finding; it carries at most an excerpt of the producer's value, never the whole of it. Many + /// rules interpolate a name, a string or a keyword the document controls, and ISO 32000-2 Annex + /// C.1 sets no bound on any of those ("In general, this PDF standard does not restrict the size + /// or quantity of things described in the PDF file format"), so without this cut one + /// 900,000-byte /Filter name shared by 400 pages retained 705.7 MiB of message text from a 990 + /// KB file (measured in #403). 1024 characters is roughly twice the longest sentence any rule + /// composes on its own (522 characters, A2aContentItemTaggingRule) and short enough that a + /// result list of thousands of findings stays a few megabytes. + /// + /// This cut is the only bound most messages have. Ten sites whose message names a producer + /// value (a /Filter, an action type, a named action, an annotation /Subtype or /AP key, a + /// composite font's /Encoding CMap name in two rules, a /RoleMap or /Perms key, a blend mode) + /// additionally excerpt it through the Reader's before + /// interpolating, so the sentence keeps its shape; every other producer-controlled + /// interpolation is cut mid-value here when the value is oversized (#405 lists them). The two + /// differ in what they can assume: a value is Latin-1 (one character per + /// byte, never a surrogate pair), so slices + /// freely and counts bytes, while this cut sees text decoded from UTF-16BE too and has to step + /// around a surrogate pair. + /// /// internal const int MaxMessageChars = 1024; @@ -533,7 +545,9 @@ public IEnumerable EnumerateStreams() /// Specification clause citation. /// The finding's severity. /// Human-readable description. Text past - /// characters is replaced by ... (N chars), N being the full length. + /// characters is replaced by ... (N chars), N being the full length in characters + /// (UTF-16 code units). One character less is kept when the cut would split a surrogate + /// pair. /// Optional "N 0 R" object location. public void Report( string ruleId, @@ -542,7 +556,7 @@ public void Report( string message, string? objectRef = null) { - if (message.Length > MaxMessageChars) + if (message is { Length: > MaxMessageChars }) { // A message can end mid-surrogate-pair when the producer value came from UTF-16BE // text (A2aLangSyntaxRule, UaLangSyntaxRule, XmpPacket): cutting inside the pair would diff --git a/tests/VellumPdf.Conformance.Tests/PreflightMessageBoundTests.cs b/tests/VellumPdf.Conformance.Tests/PreflightMessageBoundTests.cs index 7aa01f7d..02ab8590 100644 --- a/tests/VellumPdf.Conformance.Tests/PreflightMessageBoundTests.cs +++ b/tests/VellumPdf.Conformance.Tests/PreflightMessageBoundTests.cs @@ -1,7 +1,6 @@ // Copyright © Timothy van der Ham (@Tim81) // SPDX-License-Identifier: Apache-2.0 -using System.Linq; using System.Text; using VellumPdf.Canvas; using VellumPdf.Conformance.Rules; @@ -14,10 +13,12 @@ namespace VellumPdf.Conformance.Tests; /// /// Value-level tests for the #403 message bound: cuts a -/// message at , and the nine sites that quote a -/// producer whole excerpt it through +/// message at , and the ten sites that quoted a +/// producer whole (nine .Value interpolations and the annotation +/// label AnnotationRule builds from one) excerpt it through /// first, so the sentence shape survives instead of -/// being cut mid-word by the sink. +/// being cut mid-word by the sink. Every other producer-controlled interpolation relies on the +/// sink cut alone; #405 lists them. /// public sealed class PreflightMessageBoundTests { @@ -134,8 +135,8 @@ private static byte[] BuildFontPdf(params PdfObj[] fontObjects) /// /// Builds a UA-1 tagged document with one embedded (Type0) font selected via Tf, mirroring - /// OracleCorpus.WriterPdfTagged. Kept independent because that method builds a - /// registered oracle fixture (#403 must not change what any oracle test validates). + /// OracleCorpus.WriterPdfTagged. Kept independent because that method is private to + /// OracleCorpus. /// private static byte[] BuildUa1TaggedPdf() { @@ -199,7 +200,7 @@ private static byte[] BuildUa1OversizedCMapNamePdf(int nameBytes) return reader.AppendRevision([(type0Ref.ObjectNumber, 0, clone)]); } - // ── Site 1 / site 2: the shared oversized-/Filter fixture ─────────────────────────────────── + // ── The shared oversized-/Filter fixture: the StreamRule site, and the sink ───────────────── [Fact] public void StreamFilter_withAnOversizedName_reportsAFixedExcerpt() @@ -237,7 +238,7 @@ public void RuleEvaluationFailure_withAnOversizedToken_isBoundedByTheSink() Assert.All(matching, a => Assert.Equal(expected, a.Message)); } - // ── Sites 3-10: one test per Layer B site ──────────────────────────────────────────────────── + // ── One test per remaining site that quotes a producer name ────────────────────────────────── [Fact] public void ActionType_withAnOversizedName_reportsAFixedExcerpt() @@ -484,7 +485,9 @@ public void Report_atTheBoundary_keepsExactlyMaxMessageChars(int scenario) break; } - Assert.False(retained.Length > 0 && char.IsHighSurrogate(retained[^1])); + // Scenario 2 fails here when the surrogate step-back in Report is removed: the cut would + // then land between the two halves of U+1F600 and leave a lone high surrogate. + Assert.DoesNotContain(retained, char.IsSurrogate); } // ── The issue's own shape: 400 pages sharing one oversized /Filter name ───────────────────── @@ -515,8 +518,15 @@ public void FourHundredPagesSharingOneOversizedFilterName_retainOnlyBoundedMessa var result = PdfPreflight.Validate(bytes, PdfConformance.PdfA2A); - Assert.All(result.Assertions, a => Assert.True(a.Message.Length <= PreflightContext.MaxMessageChars + 22)); + // "... (" + up to ten digits of length + " chars)" is the longest suffix Report appends. + const int maxSuffixChars = 5 + 10 + 7; + Assert.All( + result.Assertions, + a => Assert.True( + a.Message.Length <= PreflightContext.MaxMessageChars + maxSuffixChars, + $"A retained message of rule {a.RuleId} was {a.Message.Length} chars.")); + // 128 Ki characters for 407 findings; the pre-fix total was 705.7 MiB of message text. var totalLength = result.Assertions.Sum(a => a.Message.Length); Assert.True(totalLength < 131_072, $"Total retained message length was {totalLength} chars."); From 7b86a404e139db1e52e685dd3a9678586001cad7 Mon Sep 17 00:00:00 2001 From: Timothy van der Ham Date: Sat, 5 Sep 2026 05:09:57 +0200 Subject: [PATCH 3/5] fix(conformance): tighten the message-bound docs and boundary test Round-2 review of #404. No behaviour changes. - PreflightAssertion.Message now says what the text is and is not: no compatibility contract (switch on RuleId), cut at 1024 characters with the length appended, producer names excerpted at 32 characters. - The boundary Theory's comment named DoesNotContain as the assertion that catches a removed surrogate step-back; Assert.Equal fails first (re-verified with the step-back disabled). The comment says so, and the closing check is a lone-surrogate walk, since IsSurrogate would also reject a prefix that ends in a complete pair. - The 400-page test comment quoted #403's 705.7 MiB as this fixture's pre-fix total; the fixture's own figure is 693.6 MiB. - BuildUa1TaggedPdf's doc gives the reason Ua1BadCMapName cannot be reused: it hardcodes a ten-character name. - MaxMessageChars doc and CHANGELOG: 705.7 MiB is a GC delta; the Latin-1 claim is scoped to a name parsed from a document; the byte-identity claim is scoped to the ten excerpt sites. - The cut builds its string with string.Concat over a span instead of an intermediate substring. --- CHANGELOG.md | 17 ++++++------ .../PreflightAssertion.cs | 8 +++++- .../Rules/PreflightContext.cs | 19 +++++++------- .../PreflightMessageBoundTests.cs | 26 +++++++++++++++---- 4 files changed, 47 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e27c6e4..d48f9b0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -126,8 +126,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). blend mode) now keep the first 32 characters followed by `... (N bytes)`, and every `PreflightAssertion.Message` past 1024 characters ends in `... (N chars)`. `vellum-preflight`'s text, JSON and SARIF output carries the same text. Verdicts, rule ids, clauses and assertion - counts are unaffected, and a message whose named value is 32 characters or shorter is - byte-identical to before. See Fixed, below, for why. (#403) + counts are unaffected, and at these ten sites a message whose named value is 32 characters or + shorter is byte-identical to before. See Fixed, below, for why. (#403) ### Fixed @@ -142,12 +142,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). `Extract` therefore reads back with `Extract` included in `PdfEncryptionInfo.Permissions`. (#397) - **A preflight finding could retain a producer-sized string for the result's lifetime.** A rule that names the offending value in its message interpolated it whole, so one oversized `/Filter` - name shared by 400 pages kept 705.7 MiB of message text alive from a 990 KB file (measured in - #403). Every `PreflightAssertion.Message` is now cut at 1024 characters with its length appended, - and the ten sites that quoted a name whole keep the first 32 characters plus the byte count; the - remaining sites that interpolate a producer value rely on the 1024-character cut and are listed - in #405. What this bounds is the retained result; the transient allocation the Reader makes while - building its own exception message for an unknown filter is unchanged and tracked in #406. (#403) + name shared by 400 pages kept 705.7 MiB (GC delta) of message text alive from a 990 KB file + (measured in #403). Every `PreflightAssertion.Message` is now cut at 1024 characters with its + length appended, and the ten sites that quoted a name whole keep the first 32 characters plus the + byte count; the remaining sites that interpolate a producer value rely on the 1024-character cut + and are listed in #405. What this bounds is the retained result; the transient allocation the + Reader makes while building its own exception message for an unknown filter is unchanged and + tracked in #406. (#403) ## [2.3.0] - 2026-09-01 diff --git a/src/VellumPdf.Conformance/PreflightAssertion.cs b/src/VellumPdf.Conformance/PreflightAssertion.cs index d8254b5b..36932e78 100644 --- a/src/VellumPdf.Conformance/PreflightAssertion.cs +++ b/src/VellumPdf.Conformance/PreflightAssertion.cs @@ -26,7 +26,13 @@ public sealed class PreflightAssertion /// The severity of the finding. public PreflightSeverity Severity { get; } - /// A human-readable description of the finding. + /// + /// A human-readable description of the finding. Not a compatibility contract: the wording may + /// change across releases, so a caller that needs to branch on the condition should switch on + /// instead of matching text here. The text is bounded: past 1024 + /// characters it is replaced by ... (N chars) with N the full length, and a producer + /// name a rule quotes is kept to its first 32 characters followed by ... (N bytes). + /// public string Message { get; } /// diff --git a/src/VellumPdf.Conformance/Rules/PreflightContext.cs b/src/VellumPdf.Conformance/Rules/PreflightContext.cs index f21db1bd..53bd2d6f 100644 --- a/src/VellumPdf.Conformance/Rules/PreflightContext.cs +++ b/src/VellumPdf.Conformance/Rules/PreflightContext.cs @@ -521,10 +521,10 @@ public IEnumerable EnumerateStreams() /// rules interpolate a name, a string or a keyword the document controls, and ISO 32000-2 Annex /// C.1 sets no bound on any of those ("In general, this PDF standard does not restrict the size /// or quantity of things described in the PDF file format"), so without this cut one - /// 900,000-byte /Filter name shared by 400 pages retained 705.7 MiB of message text from a 990 - /// KB file (measured in #403). 1024 characters is roughly twice the longest sentence any rule - /// composes on its own (522 characters, A2aContentItemTaggingRule) and short enough that a - /// result list of thousands of findings stays a few megabytes. + /// 900,000-byte /Filter name shared by 400 pages retained 705.7 MiB (GC delta) of message text + /// from a 990 KB file (measured in #403). 1024 characters is roughly twice the longest sentence + /// any rule composes on its own (522 characters, A2aContentItemTaggingRule) and short enough + /// that a result list of thousands of findings stays a few megabytes. /// /// This cut is the only bound most messages have. Ten sites whose message names a producer /// value (a /Filter, an action type, a named action, an annotation /Subtype or /AP key, a @@ -532,10 +532,10 @@ public IEnumerable EnumerateStreams() /// additionally excerpt it through the Reader's before /// interpolating, so the sentence keeps its shape; every other producer-controlled /// interpolation is cut mid-value here when the value is oversized (#405 lists them). The two - /// differ in what they can assume: a value is Latin-1 (one character per - /// byte, never a surrogate pair), so slices - /// freely and counts bytes, while this cut sees text decoded from UTF-16BE too and has to step - /// around a surrogate pair. + /// differ in what they can assume: a parsed from a document is Latin-1 + /// (one character per byte, never a surrogate pair), so + /// slices freely and counts bytes, while this cut + /// sees text decoded from UTF-16BE too and has to step around a surrogate pair. /// /// internal const int MaxMessageChars = 1024; @@ -565,7 +565,8 @@ public void Report( var cut = MaxMessageChars; if (char.IsHighSurrogate(message[cut - 1])) cut--; - message = $"{message[..cut]}... ({message.Length} chars)"; + message = string.Concat(message.AsSpan(0, cut), "... (", message.Length.ToString(), + " chars)"); } _assertions.Add(new PreflightAssertion(ruleId, clause, severity, message, objectRef)); diff --git a/tests/VellumPdf.Conformance.Tests/PreflightMessageBoundTests.cs b/tests/VellumPdf.Conformance.Tests/PreflightMessageBoundTests.cs index 02ab8590..c6a59fa3 100644 --- a/tests/VellumPdf.Conformance.Tests/PreflightMessageBoundTests.cs +++ b/tests/VellumPdf.Conformance.Tests/PreflightMessageBoundTests.cs @@ -173,7 +173,7 @@ private static byte[] BuildUa1TaggedPdf() /// /// Rewrites the fixture's Type0 font's /Encoding to an oversized name via an incremental /// update, mirroring OracleCorpus.Ua1BadCMapName's clone-and-AppendRevision step (that - /// method itself is a registered oracle fixture and is not reused directly). + /// method hardcodes a ten-character name, so it cannot produce an oversized one). /// private static byte[] BuildUa1OversizedCMapNamePdf(int nameBytes) { @@ -485,9 +485,24 @@ public void Report_atTheBoundary_keepsExactlyMaxMessageChars(int scenario) break; } - // Scenario 2 fails here when the surrogate step-back in Report is removed: the cut would - // then land between the two halves of U+1F600 and leave a lone high surrogate. - Assert.DoesNotContain(retained, char.IsSurrogate); + // The Assert.Equal above is the discriminating check: with the surrogate step-back in + // Report removed, scenario 2 keeps 1024 characters and lands between the two halves of + // U+1F600, so the expected 1023-character prefix no longer matches. The walk below restates + // the invariant that check implies (no lone surrogate anywhere in the retained text) in the + // form the CLI's SARIF writer needs. + for (var i = 0; i < retained.Length; i++) + { + if (char.IsHighSurrogate(retained[i])) + { + Assert.True(i + 1 < retained.Length && char.IsLowSurrogate(retained[i + 1]), + $"Lone high surrogate at index {i}."); + i++; + } + else + { + Assert.False(char.IsLowSurrogate(retained[i]), $"Lone low surrogate at index {i}."); + } + } } // ── The issue's own shape: 400 pages sharing one oversized /Filter name ───────────────────── @@ -526,7 +541,8 @@ public void FourHundredPagesSharingOneOversizedFilterName_retainOnlyBoundedMessa a.Message.Length <= PreflightContext.MaxMessageChars + maxSuffixChars, $"A retained message of rule {a.RuleId} was {a.Message.Length} chars.")); - // 128 Ki characters for 407 findings; the pre-fix total was 705.7 MiB of message text. + // 128 Ki characters for 407 findings; the pre-fix total for this fixture was 693.6 MiB of + // message text (#403's 705.7 MiB figure is from a 990 KB variant of the same shape). var totalLength = result.Assertions.Sum(a => a.Message.Length); Assert.True(totalLength < 131_072, $"Total retained message length was {totalLength} chars."); From be8bb67bceb7654cd9d3aa365acad58528c98812 Mon Sep 17 00:00:00 2001 From: Timothy van der Ham Date: Sat, 5 Sep 2026 05:57:57 +0200 Subject: [PATCH 4/5] docs(conformance): scope the excerpt claim in the Message doc The PreflightAssertion.Message doc said every producer name a rule quotes is kept to 32 characters. Only ten sites excerpt; the rest are bounded by the 1024-character sink cut alone, and the doc now says so. The MaxMessageChars summary said a message never carries the whole of a producer value; a value short enough to fit is carried whole, so the sentence is now about oversized values only. The 400-page test comment labels #403's 705.7 MiB as a GC delta, the way the CHANGELOG and the MaxMessageChars doc already do. No code change. --- src/VellumPdf.Conformance/PreflightAssertion.cs | 6 ++++-- src/VellumPdf.Conformance/Rules/PreflightContext.cs | 3 ++- .../PreflightMessageBoundTests.cs | 3 ++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/VellumPdf.Conformance/PreflightAssertion.cs b/src/VellumPdf.Conformance/PreflightAssertion.cs index 36932e78..3e0413c2 100644 --- a/src/VellumPdf.Conformance/PreflightAssertion.cs +++ b/src/VellumPdf.Conformance/PreflightAssertion.cs @@ -30,8 +30,10 @@ public sealed class PreflightAssertion /// A human-readable description of the finding. Not a compatibility contract: the wording may /// change across releases, so a caller that needs to branch on the condition should switch on /// instead of matching text here. The text is bounded: past 1024 - /// characters it is replaced by ... (N chars) with N the full length, and a producer - /// name a rule quotes is kept to its first 32 characters followed by ... (N bytes). + /// characters it is replaced by ... (N chars) with N the full length. Some rules + /// additionally excerpt the producer name they quote, keeping its first 32 characters + /// followed by ... (N bytes); elsewhere an oversized producer value is bounded by the + /// 1024-character cut alone. /// public string Message { get; } diff --git a/src/VellumPdf.Conformance/Rules/PreflightContext.cs b/src/VellumPdf.Conformance/Rules/PreflightContext.cs index 53bd2d6f..d3df4bd1 100644 --- a/src/VellumPdf.Conformance/Rules/PreflightContext.cs +++ b/src/VellumPdf.Conformance/Rules/PreflightContext.cs @@ -517,7 +517,8 @@ public IEnumerable EnumerateStreams() /// /// The longest message a retains. A message identifies a - /// finding; it carries at most an excerpt of the producer's value, never the whole of it. Many + /// finding; it carries at most an excerpt of an oversized producer value, never the whole of + /// one. Many /// rules interpolate a name, a string or a keyword the document controls, and ISO 32000-2 Annex /// C.1 sets no bound on any of those ("In general, this PDF standard does not restrict the size /// or quantity of things described in the PDF file format"), so without this cut one diff --git a/tests/VellumPdf.Conformance.Tests/PreflightMessageBoundTests.cs b/tests/VellumPdf.Conformance.Tests/PreflightMessageBoundTests.cs index c6a59fa3..ba914fe4 100644 --- a/tests/VellumPdf.Conformance.Tests/PreflightMessageBoundTests.cs +++ b/tests/VellumPdf.Conformance.Tests/PreflightMessageBoundTests.cs @@ -542,7 +542,8 @@ public void FourHundredPagesSharingOneOversizedFilterName_retainOnlyBoundedMessa $"A retained message of rule {a.RuleId} was {a.Message.Length} chars.")); // 128 Ki characters for 407 findings; the pre-fix total for this fixture was 693.6 MiB of - // message text (#403's 705.7 MiB figure is from a 990 KB variant of the same shape). + // message text (#403's 705.7 MiB is a GC delta measured on a 990 KB variant of the same + // shape). var totalLength = result.Assertions.Sum(a => a.Message.Length); Assert.True(totalLength < 131_072, $"Total retained message length was {totalLength} chars."); From 8763761460ac3549db99b92021fd48d224cb82a0 Mon Sep 17 00:00:00 2001 From: Timothy van der Ham Date: Sat, 5 Sep 2026 06:38:34 +0200 Subject: [PATCH 5/5] docs(conformance): count the excerpt sites in the Message doc Round 4 of #404 read the PreflightAssertion.Message doc as saying that any quoted producer name is excerpted, and that the unit is the rule. DiagnosticExcerpt.Quote returns a name of 32 bytes or fewer whole, and FontStructureRule excerpts at one site while interpolating a base font name whole at four others, so the doc now says "ten sites" and "an oversized one", matching the MaxMessageChars doc, the class doc and the CHANGELOG bullet. The MaxMessageChars summary is reflowed; the round-3 edit had left a ten-character line inside the paragraph. No code change. --- src/VellumPdf.Conformance/PreflightAssertion.cs | 8 ++++---- src/VellumPdf.Conformance/Rules/PreflightContext.cs | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/VellumPdf.Conformance/PreflightAssertion.cs b/src/VellumPdf.Conformance/PreflightAssertion.cs index 3e0413c2..5fa2cfed 100644 --- a/src/VellumPdf.Conformance/PreflightAssertion.cs +++ b/src/VellumPdf.Conformance/PreflightAssertion.cs @@ -30,10 +30,10 @@ public sealed class PreflightAssertion /// A human-readable description of the finding. Not a compatibility contract: the wording may /// change across releases, so a caller that needs to branch on the condition should switch on /// instead of matching text here. The text is bounded: past 1024 - /// characters it is replaced by ... (N chars) with N the full length. Some rules - /// additionally excerpt the producer name they quote, keeping its first 32 characters - /// followed by ... (N bytes); elsewhere an oversized producer value is bounded by the - /// 1024-character cut alone. + /// characters it is replaced by ... (N chars) with N the full length. Ten sites + /// additionally excerpt the producer name they quote, keeping an oversized one to its first + /// 32 characters followed by ... (N bytes); every other producer value is bounded by + /// the 1024-character cut alone. /// public string Message { get; } diff --git a/src/VellumPdf.Conformance/Rules/PreflightContext.cs b/src/VellumPdf.Conformance/Rules/PreflightContext.cs index d3df4bd1..e61cfd9e 100644 --- a/src/VellumPdf.Conformance/Rules/PreflightContext.cs +++ b/src/VellumPdf.Conformance/Rules/PreflightContext.cs @@ -518,10 +518,10 @@ public IEnumerable EnumerateStreams() /// /// The longest message a retains. A message identifies a /// finding; it carries at most an excerpt of an oversized producer value, never the whole of - /// one. Many - /// rules interpolate a name, a string or a keyword the document controls, and ISO 32000-2 Annex - /// C.1 sets no bound on any of those ("In general, this PDF standard does not restrict the size - /// or quantity of things described in the PDF file format"), so without this cut one + /// one. Many rules interpolate a name, a string or a keyword the document controls, and + /// ISO 32000-2 Annex C.1 sets no bound on any of those ("In general, this PDF standard does + /// not restrict the size or quantity of things described in the PDF file format"), so + /// without this cut one /// 900,000-byte /Filter name shared by 400 pages retained 705.7 MiB (GC delta) of message text /// from a 990 KB file (measured in #403). 1024 characters is roughly twice the longest sentence /// any rule composes on its own (522 characters, A2aContentItemTaggingRule) and short enough