Bugfix: xref-streams were not added#1173
Conversation
On a large sample of pdf-files PdfPig failed to read the correct StructTree-object for about 1% of them. The StructTree object was simply missing in the CrossReferenceTable.CrossReferenceTable. It turned out that the constructed CrossReferenceTable could miss Stream-parts if there were multiple Table-parts because a stream will only be added if it's associated with the very first Table-part. The remedy would seem to be to check for and add streams that are associated with any of the Table-parts, not just the first one. On a sample of 72 files where this failed, this changed fixed the StructTree for all of them.
|
@ricflams thanks a lot for sharing the insight, even if outdated now. @EliotJones worked on the refactoring of this part of the code, so ill let him have a look first. If he does not have time, ill try to find the time to take care of the change. In this context, if you could share files that are failing (the 1%), that'd be very helpful. That being said, if you i have time to look into your fix in the context of the new code, that'd be amazing. Thanks again! |
|
Hey and thanks for the quick reply
I included 4 pdfs which should get you (Eliot) going, I think. But let me know if you need more. I ran through about 10K pdfs and 84 failed. This incomplete xref was the reason for 72 of them and the remaining 12 looked legit broken: eg xref-pointers into nowhere etc. And PAC (I didn’t try Acrobat) also complained those 12 were broken. So it really did seem like it was just fixing “all the problems”. When cheking more files, like a sample of 100K pdfs, I’ve generally seen PdfPig doing really well at parsing - as good as PAC and Acrobat, it feels like.
Maybe I do in a week. I took a quick look after I realized the old xref-table-code wasn’t in play anymore and it just looked so completely different with the bruteforce-approach that it would take me a some time to see where it went wrong with the stream-parts. And I figured you guys might know just what to do, once you were made aware of the issue and had some sample files to look at. And, it was late 😉 |
|
@BobLd fyi I took a look this morning and spotted how to fix it in the refactored xref-reading-code. I’ll update this PR with that fix instead later today. |
- If an XrefTable has an associated stream, as indicated via the XrefStm-property, then read and add that XrefStream - Any table can have 0 or 1 such associated streams - A caveat: such an associated stream might also theoretically be part of the Parts-sequence in which case it would be encountered both by looping through all those parts along with all the regular tables and now also by association to any of those tables. It doesn't seem harmful since the offsets are flattened eventually anyway and stored by their offset-key into a mapping-table.
With the fix for including associated streams, this test now finds more text on the first page. I've verified using Aspose.PDF and by viewing the ErcotFacts.pdf file being tested that yes, it was indeed missing part of the text before.
Page two has had four more characters added, which is now delected by this xref-stream fix
Including the stream-xref means that the formerly missing font is no longer missing, so simply run the two test-cases under the (stricter) assumption of SkipMissingFonts=false.
|
Thanks a lot @ricflams ! Great stuff |
Updated [PdfPig](https://github.com/UglyToad/PdfPig) from 0.1.9 to 0.1.13. <details> <summary>Release notes</summary> _Sourced from [PdfPig's releases](https://github.com/UglyToad/PdfPig/releases)._ ## 0.1.13 ## What's Changed * Increment version to 0.1.13 by @BobLd in UglyToad/PdfPig#1207 * Simply order by offset also when not doing brute force to fix #1208 by @BobLd in UglyToad/PdfPig#1210 * Ensure no key end up missing in ResolveInternal and fix #1209 by @BobLd in UglyToad/PdfPig#1211 * update release logic to check out master before commit by @EliotJones in UglyToad/PdfPig#1212 * Return empty glyph in ReadCompositeGlyph when glyphIndex is out of range and fix #1213 by @BobLd in UglyToad/PdfPig#1215 * Handling of optional content group names without proper name by @carlokok in UglyToad/PdfPig#1216 * Minor Type1FontParser optimisations by @BobLd in UglyToad/PdfPig#1221 * Use file header offset when doing brute force find and fix #1223 by @BobLd in UglyToad/PdfPig#1224 * Do not return glyph bbox and path in Type1Font if character name is '.notdef' by @BobLd in UglyToad/PdfPig#1229 ## New Contributors * @carlokok made their first contribution in UglyToad/PdfPig#1216 **Full Changelog**: UglyToad/PdfPig@v0.1.12...unreleased ## 0.1.12 ## What's Changed * add nullability to core project by @EliotJones in UglyToad/PdfPig#1111 * Fix usage of List.Contains by @theolivenbaum in UglyToad/PdfPig#1112 * allow missing catalog type definition for catalog dictionary by @EliotJones in UglyToad/PdfPig#1113 * Performance improvements and .Net 9 support by @chuckbeasley in UglyToad/PdfPig#1116 * Update run_integration_tests.yml by @BobLd in UglyToad/PdfPig#1117 * Add global.json in tools by @BobLd in UglyToad/PdfPig#1118 * Update run_integration_tests.yml by @BobLd in UglyToad/PdfPig#1119 * Update run_integration_tests.yml by @BobLd in UglyToad/PdfPig#1120 * Update run_common_crawl_tests.yml by @BobLd in UglyToad/PdfPig#1121 * Update nightly_release.yml by @BobLd in UglyToad/PdfPig#1123 * Increase FlateFilter multiplier when preventing malicious OOM and fix #1125 by @BobLd in UglyToad/PdfPig#1126 * Update build_and_test_macos.yml by @BobLd in UglyToad/PdfPig#1129 * Update build_and_test_macos.yml by @BobLd in UglyToad/PdfPig#1130 * Prevent StackOverflow in ParseTrailer and fix #1122 by @BobLd in UglyToad/PdfPig#1127 * Lower max search depth in preventing StackOverflow in ParseTrailer by @BobLd in UglyToad/PdfPig#1131 * add container node support for BookmarksProvider.cs by @migeyusu in UglyToad/PdfPig#1133 * move file parsing to single-pass static methods by @EliotJones in UglyToad/PdfPig#1102 * Add early version of IOSSystemFontLister by @BobLd in UglyToad/PdfPig#1143 * File buffering read stream investigation by @EliotJones in UglyToad/PdfPig#1140 * Draft release on master build by @EliotJones in UglyToad/PdfPig#1145 * First create the StreamInputBytes in PdfDocument.Open() to check the stream CanRead and CanSeek by @BobLd in UglyToad/PdfPig#1147 * Fix font matrix issues by @BobLd in UglyToad/PdfPig#1150 * Properly fix #1148 by always parsing optional tables in TrueTypeFontParser and remove Type 0 font hack by @BobLd in UglyToad/PdfPig#1151 * copy other parser behavior by treating end of stream as valid end inline image by @EliotJones in UglyToad/PdfPig#1152 * add test jobs for common crawl 0000 to 0007 by @EliotJones in UglyToad/PdfPig#1153 * handle case where xobjects use same key as fonts by @EliotJones in UglyToad/PdfPig#1154 * read last line of ignore file by @EliotJones in UglyToad/PdfPig#1155 * Use correct font matrix when transforming the width in Type 0 font and fix #1156 by @BobLd in UglyToad/PdfPig#1157 * Add initial support to process CFF fonts contained inside a TrueType font by @BobLd in UglyToad/PdfPig#1159 * Handle non seekable stream by copying it into a memory stream and fix #1146 by @BobLd in UglyToad/PdfPig#1158 * handle case where offsets are out of range by @EliotJones in UglyToad/PdfPig#1160 * Use record struct in FileHeaderOffset by @BobLd in UglyToad/PdfPig#1161 * Expose letter's font via GetFont(), make Font property as obsolete and use FontDetails instead by @BobLd in UglyToad/PdfPig#1166 * Add GetDescent() and GetAscent() to IFont and loose bounding box to letter by @BobLd in UglyToad/PdfPig#1167 * Use pageFactoryCache.Clear() in Pages dispose and fix #1170 by @BobLd in UglyToad/PdfPig#1174 * Bugfix: xref-streams were not added by @ricflams in UglyToad/PdfPig#1173 * Guard against circular references in XRef tables/streams by @ricflams in UglyToad/PdfPig#1175 * Add more tests to NearestNeighbourWordExtractorTests by @BobLd in UglyToad/PdfPig#1180 * Feature/improve group indexes by @BobLd in UglyToad/PdfPig#1181 * Trim excess in long lived font collections by @BobLd in UglyToad/PdfPig#1184 * Set Type 3 font ascent to Top instead of Height, see #1164 by @BobLd in UglyToad/PdfPig#1185 * Only apply RemoveStridePadding() when bytes per pixel is one and fix #1183 by @BobLd in UglyToad/PdfPig#1187 * Use zlib decode to properly use window size and checksum in flate filter by @rhuijben in UglyToad/PdfPig#1186 * Avoid doing a true file seek for simple peeking in the token parser by @rhuijben in UglyToad/PdfPig#1188 * Fix regression introduced in 3592fc8 where slicing the stream to the length breaks decoding by @BobLd in UglyToad/PdfPig#1192 * Update NameToUnicodeConvertAglSpecification to test what was intended by @rhuijben in UglyToad/PdfPig#1191 * Add CMap caching at document level and add MurmurHash3 hashing function by @BobLd in UglyToad/PdfPig#1193 * Avoid reading ahead and then seeking back by @rhuijben in UglyToad/PdfPig#1189 * Do not slice the stream to the length breaks decoding in FlateDecode by @BobLd in UglyToad/PdfPig#1194 ... (truncated) ## 0.1.11 Welcome to version 0.1.11. The changes in this version have mainly focused on stability. There is a breaking API change. We have also started to run tests against a larger corpus of documents from Common Crawl allowing us to find bugs and malformed files proactively. This release is screened against 6000 additional files. - Improvements to content and font parsing detected by fuzzing inputs. - Improvements and resiliency for finding the `startxref` location when parsing a file.. - Adds build and tests for Mac OS as well as retrieving system fonts on iPad (Mac Catalyst). - Support clipping when rendering XObjects. - Prevent malformed files leading to an out-of-memory when decompressing streams. - Make `IGraphicsStateOperationFactory` and `ReflectionGraphicsStateOperationFactory` public. - Softmask support for images. - Performance improvements using `Span` and `ReadOnlyMemory` where available. - Handle corrupt files where the stream contains comment tokens. - Improvements to copying from existing files when using `PdfDocumentBuilder`, fixes some bugs with copying fonts and dictionary tokens referenced indirectly. - Handle corrupt files with double `endstream` definitions. - More tolerant parsing for a number of invalid PDFs, including invalid USC2 input, CMAP formats, CFF fonts, missing font subtypes, invalid `xref` table positions, missing `/FirstChar` entry for font dictionaries and corrupt ASCII 85 encoded data. - Fix an issue where adding content to an existing PDF using `PdfDocumentBuilder` could result in upside-down or wrongly positioned text due to global transforms in the source PDF. - New option to completely skip annotations when building a document. - Prevent infinite loops in certain documents #1096. - Improved performance when tokenizing numbers, this should provide a minor speed improvement. - When adding a page from an existing PDF to a `PdfDocumentBuilder` any external link annotations should be preserved. ### Breaking changes The method on `PdfDocumentBuilder`: ``` public PdfPageBuilder AddPage(PdfDocument document, int pageNumber, Func<PdfAction, PdfAction?>? copyLink) ``` Has been changed to wrap the `copyLink` parameter in an options object to support the `KeepAnnotations` option: ``` public PdfPageBuilder AddPage(PdfDocument document, int pageNumber, AddPageOptions options) ``` You can just set the `CopyLinkFunc` property in the options object if you need to access this functionality. ## Auto generated change log * Bump version to 0.1.11-alpha001 by @BobLd in UglyToad/PdfPig#1009 * Improve Jpeg2000Helper to support J2K codec and add test by @BobLd in UglyToad/PdfPig#1010 * Add SetStrokeDetails() and SetFillDetails() to PdfPath and tidy up ContentStreamProcessor by @BobLd in UglyToad/PdfPig#1014 * Implement clipping in ProcessFormXObject() by @BobLd in UglyToad/PdfPig#1015 * Fix #1017 by @lofcz in UglyToad/PdfPig#1018 * Fix PatternColor Equals() method and fix #1016 by @BobLd in UglyToad/PdfPig#1019 * Feature/image mask by @BobLd in UglyToad/PdfPig#1012 * Update README.md by @BobLd in UglyToad/PdfPig#1020 * Fix bug where FormXObject bbox needs to be normalised by @BobLd in UglyToad/PdfPig#1021 * Add MacOS test pipeline and fix failing tests by @BobLd in UglyToad/PdfPig#1025 ... (truncated) ## 0.1.10 ## What's Changed * Fix GetTextOrientation by cleanly checking if rotation is divisible by 90 and fix #913 by @BobLd in UglyToad/PdfPig#914 * Add early version of BrowserSystemFontLister by @BobLd in UglyToad/PdfPig#920 * Remove list from FileTrailerParser.GetStartXrefPosition() by @BobLd in UglyToad/PdfPig#922 * Reorganise Filters and make them public by @BobLd in UglyToad/PdfPig#925 * Support decrypting V4/R4 files with AESV2 and no Length property by @Greybird in UglyToad/PdfPig#924 * Use pdfScanner in ReadVerticalDisplacements and fix #693 and return 0… by @BobLd in UglyToad/PdfPig#928 * Default page number to 0 in ExplicitDestination when the Dest has no page number and fix #736 by @BobLd in UglyToad/PdfPig#930 * Move Paths, GetAnnotations() and GetOptionalContents() outside of ExperimentalAccess and mark Experimental class and reference as obsolete by @BobLd in UglyToad/PdfPig#931 * Upgrade tests project NuGet packages by @BobLd in UglyToad/PdfPig#932 * Optimize cross reference object offset validation by avoiding nested loop by @madelson in UglyToad/PdfPig#935 * Revive trimming/AOT analysis by @madelson in UglyToad/PdfPig#939 * Stop treating Warnings as Errors by @BobLd in UglyToad/PdfPig#941 * Handle alternate Unicode name representation cXXX and fix #943 by @BobLd in UglyToad/PdfPig#944 * Handle odd ligatures names and fix #945 by @BobLd in UglyToad/PdfPig#946 * Update additional glyph list to latest from PDFBox by @BobLd in UglyToad/PdfPig#948 * New GetText() option: NegativeGapAsWhitespace by @Kizaemon in UglyToad/PdfPig#952 * Fix for IndexOutOfRangeException exception by @GrabzIt in UglyToad/PdfPig#955 * Fix "Nightly Release" pipeline following csproj changes by @BobLd in UglyToad/PdfPig#957 * Do not throw exception when lenient parsing in ON in CrossReferenceParser and fix #959 by @BobLd in UglyToad/PdfPig#961 * Improve UnwrapIndexedColorSpaceBytes by @BobLd in UglyToad/PdfPig#962 * Fix out of range exception in AnnotationProvider by @BobLd in UglyToad/PdfPig#963 * Return a copy of the ArrayPoolBufferWriter buffer in Ascii85, AsciiHex and RunLength filters and fix #964 by @BobLd in UglyToad/PdfPig#965 * Make ColorSpaceDetails.BaseNumberOfColorComponents public to allow for external image factories by @BobLd in UglyToad/PdfPig#966 * Improve GlyphList by @BobLd in UglyToad/PdfPig#967 * Properly handle ZapfDingbats font for TrueTypeSimpleFont and add tests by @BobLd in UglyToad/PdfPig#969 * Execute RemoveStridePadding in place when possible by @BobLd in UglyToad/PdfPig#968 * Add HexToken case in OptionalContent parsing by @simonedd in UglyToad/PdfPig#971 * Update UglyToad.PdfPig.ConsoleRunner target framework to net8 by @BobLd in UglyToad/PdfPig#972 * Do not throw error on Pop when stack size is 1 in lenient mode and fix #973 by @BobLd in UglyToad/PdfPig#974 * Fix warnings about "type 'K' cannot be used as type parameter 'TKey' in the generic type or method 'Dictionary<TKey, TValue>'" by @BobLd in UglyToad/PdfPig#976 * Refactor XObjectFactory by @BobLd in UglyToad/PdfPig#977 * Update UnpackComponents() to account for 1bpc + DeviceGray (hack for Jbig2) by @BobLd in UglyToad/PdfPig#978 * CcittFaxDecodeFilter: do not check for input length, invert bitmap with ref byte and fix #982 by @BobLd in UglyToad/PdfPig#983 * Add JPX bits per component decoding by @BobLd in UglyToad/PdfPig#986 * Issues/987 by @BobLd in UglyToad/PdfPig#990 * Make DecodeParameterResolver class public by @BobLd in UglyToad/PdfPig#993 * Update Microsoft and SkiaSharp NuGet packages by @BobLd in UglyToad/PdfPig#994 * Update Microsoft NuGet packages for UglyToad.PdfPig.Package by @BobLd in UglyToad/PdfPig#996 * Resolve image data (implementation from @kasperdaff) by @BobLd in UglyToad/PdfPig#998 * Pass IFilterProvider to IFilter.Decode() and handle null in PdfExtensions.Resolve() by @BobLd in UglyToad/PdfPig#999 * Improve GetExtendedGraphicsStateDictionary() and StackDictionary.TryGetValue() by @BobLd in UglyToad/PdfPig#1004 * Better handle integer overflow in DocstrumBoundingBoxes by @BobLd in UglyToad/PdfPig#1005 * version 0.1.10 by @BobLd in UglyToad/PdfPig#1006 * Update run_integration_tests.yml by @BobLd in UglyToad/PdfPig#1007 ## New Contributors * @madelson made their first contribution in UglyToad/PdfPig#935 * @Kizaemon made their first contribution in UglyToad/PdfPig#952 * @GrabzIt made their first contribution in UglyToad/PdfPig#955 ... (truncated) Commits viewable in [compare view](UglyToad/PdfPig@v0.1.9...0.1.13). </details> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Laurent Ellerbach <laurelle@microsoft.com>
On 8000 pdf-files PdfPig failed to read the correct StructTree-object for about 1% of them. The StructTree object was simply missing in the CrossReferenceTable.
It turned out that the constructed CrossReferenceTable could miss Stream-parts if there were multiple Table-parts because a stream will only be added if it's associated with the very first Table-part. The remedy would seem to be to check for and add streams that are associated with any of the Table-parts, not just the first one.☝️ That was for the old code in CrossReferenceTableBuilder. I'm still including that just FYI. The fix for the refactored xref code is in FirstPassParser.cs instead: associated streams were just not being added.
On a sample of 72 files where this failed, this change fixed the StructTree for all of them.
I've included four files where this was fixed:
doc-00044a646349ce519d9b7287159795cc.pdf
doc-00072bb64c86990e7391ef5baca69099.pdf
doc-000558f2b1d03d09ea6bb359732a8f09.pdf
doc-000518224506a58efb993b67ef57661b.pdf
Obsolete comment:
I fetched latest master (it had been a couple of months) and realized that the code wasn't called anymore because the parsing of xrefs had been completely refactored: 0afe021. The refactored PdfPig-code still can't deal with that situation (for instance the 4 files I've attached) so I'm back to square one. Even though my change in this PR is for a file than are no longer in play I'm submitting it anyway, since it may give you a hint about what's currently wrong and how to fix it. I really appreciate the effort you're putting into PdfPig. Just rather bad timing that the fix I made today for a problem that had been troubling me for months, was in vain because that part has been refactored three weeks ago 🥲The fix also revealed that one of the github test-case files, ErcotFacts.pdf in Issue874, was not being read correctly before. It actually included more text that was only referred to in the missing stream-xref: here's a diff of the text with/without this PR's fix, how the pdf is rendered in Chrome, and what text Aspose.PDF extracts:
The fix also means that ErcotFacts.pdf is no longer missing a font so that test-case has been changed too.