From c5c76996156f2e8b117c2dcf8cb7a75a2fa3d141 Mon Sep 17 00:00:00 2001 From: Calixte Denizet Date: Thu, 23 May 2024 10:14:31 +0200 Subject: [PATCH 1/3] Auto-close the marked content sections left open in a text object (bug 1898053) Some PDFs open a marked content section inside a text object and never close it. Each section adds a nesting level in the text layer, so the DOM becomes deep enough to crash Chromium with a stack overflow. When extracting the text, the sections opened after the matching BT are now closed on ET. The marked content level is tracked per stream in both getTextContent and getOperatorList: the sections a stream left open are closed when it ends, and an unbalanced EMC is ignored. --- src/core/evaluator.js | 89 ++++++++++++++++++--------- test/integration/text_layer_spec.mjs | 26 ++++++++ test/pdfs/.gitignore | 1 + test/pdfs/bug1898053_minimal.pdf | Bin 0 -> 741 bytes test/unit/api_spec.js | 53 ++++++++++++++++ test/unit/evaluator_spec.js | 64 +++++++++++++++++++ 6 files changed, 205 insertions(+), 28 deletions(-) create mode 100644 test/pdfs/bug1898053_minimal.pdf diff --git a/src/core/evaluator.js b/src/core/evaluator.js index 1cf9be975f439..96e2d5d81a570 100644 --- a/src/core/evaluator.js +++ b/src/core/evaluator.js @@ -1720,6 +1720,7 @@ class PartialEvaluator { const stateManager = new StateManager(initialState); const preprocessor = new EvaluatorPreprocessor(stream, xref, stateManager); const timeSlotManager = new TimeSlotManager(); + let markedContentLevel = 0; function closePendingRestoreOPS(argument) { for (let i = 0, ii = preprocessor.savedStatesDepth; i < ii; i++) { @@ -1727,6 +1728,12 @@ class PartialEvaluator { } } + function closePendingMarkedContentOPS() { + for (; markedContentLevel > 0; markedContentLevel--) { + operatorList.addOp(OPS.endMarkedContent, []); + } + } + return new Promise(function promiseBody(resolve, reject) { const next = function (promise) { Promise.all([promise, operatorList.ready]).then(function () { @@ -1741,7 +1748,7 @@ class PartialEvaluator { timeSlotManager.reset(); const operation = {}; - let stop, i, ii, cs, name, isValidName; + let stop, cs, name, isValidName; while (!(stop = timeSlotManager.check())) { // The arguments parsed by read() are used beyond this loop, so we // cannot reuse the same array on each iteration. Therefore we pass @@ -2299,6 +2306,7 @@ class PartialEvaluator { // but doing so is meaningless without knowing the semantics. continue; case OPS.beginMarkedContentProps: + markedContentLevel++; if (!(args[0] instanceof Name)) { warn(`Expected name for beginMarkedContentProps arg0=${args[0]}`); operatorList.addOp(OPS.beginMarkedContentProps, ["OC", null]); @@ -2333,7 +2341,7 @@ class PartialEvaluator { ); return; } - // Other marked content types aren't supported yet. + // Preserve only the MCID from non-OC property dictionaries. args = [ args[0].name, args[1] instanceof Dict ? args[1].get("MCID") : null, @@ -2341,21 +2349,27 @@ class PartialEvaluator { break; case OPS.beginMarkedContent: + if (args?.some(arg => arg instanceof Dict)) { + warn(`getOperatorList - ignoring operator: ${fn}`); + continue; + } + markedContentLevel++; + break; case OPS.endMarkedContent: + if (args?.some(arg => arg instanceof Dict)) { + warn(`getOperatorList - ignoring operator: ${fn}`); + continue; + } + if (markedContentLevel === 0) { + continue; + } + markedContentLevel--; + break; default: - // Note: Ignore the operator if it has `Dict` arguments, since - // those are non-serializable, otherwise postMessage will throw - // "An object could not be cloned.". - if (args !== null) { - for (i = 0, ii = args.length; i < ii; i++) { - if (args[i] instanceof Dict) { - break; - } - } - if (i < ii) { - warn("getOperatorList - ignoring operator: " + fn); - continue; - } + // Avoid postMessage errors from `Dict` arguments. + if (args?.some(arg => arg instanceof Dict)) { + warn(`getOperatorList - ignoring operator: ${fn}`); + continue; } } operatorList.addOp(fn, args); @@ -2364,8 +2378,8 @@ class PartialEvaluator { next(deferred); return; } - // Some PDFs don't close all restores inside object/form. - // Closing those for them. + // Close marked content and graphics states left open by this stream. + closePendingMarkedContentOPS(); closePendingRestoreOPS(); resolve(); }).catch(reason => { @@ -2378,6 +2392,7 @@ class PartialEvaluator { `task: "${reason}".` ); + closePendingMarkedContentOPS(); closePendingRestoreOPS(); return; } @@ -2395,7 +2410,6 @@ class PartialEvaluator { seenStyles = new Set(), viewBox, lang = null, - markedContentData = null, disableNormalization = false, keepWhiteSpace = false, prevRefs = null, @@ -2425,9 +2439,8 @@ class PartialEvaluator { resources ||= Dict.empty; stateManager ||= new StateManager(new TextState()); - if (includeMarkedContent) { - markedContentData ||= { level: 0 }; - } + let markedContentLevel = 0; + let textMarkedContentLevel = null; const textContent = { items: [], @@ -3173,6 +3186,19 @@ class PartialEvaluator { textContentItem.str.length = 0; } + function closePendingMarkedContentItems(level = 0) { + if (!includeMarkedContent || markedContentLevel <= level) { + return; + } + flushTextContentItem(); + + for (; markedContentLevel > level; markedContentLevel--) { + textContent.items.push({ + type: "endMarkedContent", + }); + } + } + function enqueueChunk(batch = false) { const length = textContent.items.length; if (length === 0) { @@ -3291,6 +3317,13 @@ class PartialEvaluator { case OPS.beginText: textState.textMatrix = IDENTITY_MATRIX.slice(); textState.textLineMatrix = IDENTITY_MATRIX.slice(); + textMarkedContentLevel = markedContentLevel; + break; + case OPS.endText: + if (textMarkedContentLevel !== null) { + closePendingMarkedContentItems(textMarkedContentLevel); + textMarkedContentLevel = null; + } break; case OPS.showSpacedText: if (!stateManager.state.font) { @@ -3451,7 +3484,6 @@ class PartialEvaluator { seenStyles, viewBox, lang, - markedContentData, disableNormalization, keepWhiteSpace, prevRefs: seenRefs, @@ -3535,7 +3567,7 @@ class PartialEvaluator { case OPS.beginMarkedContent: flushTextContentItem(); if (includeMarkedContent) { - markedContentData.level++; + markedContentLevel++; textContent.items.push({ type: "beginMarkedContent", @@ -3546,7 +3578,7 @@ class PartialEvaluator { case OPS.beginMarkedContentProps: flushTextContentItem(); if (includeMarkedContent) { - markedContentData.level++; + markedContentLevel++; const mcid = args[1] instanceof Dict ? args[1].get("MCID") : null; textContent.items.push({ @@ -3561,12 +3593,11 @@ class PartialEvaluator { case OPS.endMarkedContent: flushTextContentItem(); if (includeMarkedContent) { - if (markedContentData.level === 0) { - // Handle unbalanced beginMarkedContent/endMarkedContent - // operators (fixes issue15629.pdf). + if (markedContentLevel === 0) { + // Ignore unmatched EMC operators (issue 15629). break; } - markedContentData.level--; + markedContentLevel--; textContent.items.push({ type: "endMarkedContent", @@ -3585,6 +3616,7 @@ class PartialEvaluator { return; } flushTextContentItem(); + closePendingMarkedContentItems(); enqueueChunk(); resolve(); }).catch(reason => { @@ -3599,6 +3631,7 @@ class PartialEvaluator { ); flushTextContentItem(); + closePendingMarkedContentItems(); enqueueChunk(); return; } diff --git a/test/integration/text_layer_spec.mjs b/test/integration/text_layer_spec.mjs index c9d2869ecd224..021019b9a3560 100644 --- a/test/integration/text_layer_spec.mjs +++ b/test/integration/text_layer_spec.mjs @@ -1534,4 +1534,30 @@ describe("Text layer", () => { expect(getPercentDiff(rect.height, 12)).toBeLessThan(0.03); }); }); + + describe("marked-content nesting (bug 1898053)", () => { + let pages; + + beforeAll(async () => { + pages = await loadAndWait( + "bug1898053_minimal.pdf", + ".textLayer .endOfContent" + ); + }); + afterAll(async () => { + await closePages(pages); + }); + + it("must keep auto-closed sections at the text-layer root", async () => { + await Promise.all( + pages.map(async ([browserName, page]) => { + const count = await page.evaluate( + () => + document.querySelectorAll(".textLayer > .markedContent").length + ); + expect(count).toBe(6); + }) + ); + }); + }); }); diff --git a/test/pdfs/.gitignore b/test/pdfs/.gitignore index 8fc80157baa3a..8148a5d3202a4 100644 --- a/test/pdfs/.gitignore +++ b/test/pdfs/.gitignore @@ -951,3 +951,4 @@ !signed_verified.pdf !function_based_shading_cmyk.pdf !large_jpeg_downscale.pdf +!bug1898053_minimal.pdf diff --git a/test/pdfs/bug1898053_minimal.pdf b/test/pdfs/bug1898053_minimal.pdf new file mode 100644 index 0000000000000000000000000000000000000000..5c0586aa0477d7a1e2e3183d1d817d8e5aa33694 GIT binary patch literal 741 zcmah{J#W)M7-r}YPD1>Jcd-OQtBKK6;s+pgs2U`Mh>(~Ae}IW@h&#J!qB3xnb@uaqJ@50rM%ZevTN`y`e7t)12U&oD z_gkfSOK;|A~54uFBzCyRMH_IKoB67r`l1Q7Xe#*oJ+94+|JSh_P{Aa_rcs@ zX+}5sL}6GxHm+V}snS-JmPvKsGHEVZE(>tWY>JgozYAIsEBII>il=7W=E_ojSU^hm zAW&Ra%dAk>2^UqKvV0()gO62`Y$%yf_mE1~VhK-~4l1Nzbc80S1)AIS5Rl@;CTeSL`MK5#CI!ugN>V+U zh;zLnt2t32%a+}S LgAC*Gvo`t*y++of literal 0 HcmV?d00001 diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js index 4a976b987a880..b4b2e93e8f711 100644 --- a/test/unit/api_spec.js +++ b/test/unit/api_spec.js @@ -4480,6 +4480,59 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`) await loadingTask.destroy(); }); + it("auto-closes marked content opened in a text object (bug 1898053)", async function () { + const loadingTask = getDocument( + buildGetDocumentParams("bug1898053_minimal.pdf") + ); + const pdfDoc = await loadingTask.promise; + const pdfPage = await pdfDoc.getPage(1); + const { items } = await pdfPage.getTextContent({ + includeMarkedContent: true, + }); + + expect(items.map(item => item.id ?? item.type ?? item.str)).toEqual([ + "Hello, world!", + "p3R_mc1", + "endMarkedContent", + "p3R_mc2", + "endMarkedContent", + "p3R_mc3", + "endMarkedContent", + "p3R_mc4", + "endMarkedContent", + "p3R_mc5", + "endMarkedContent", + "p3R_mc6", + "endMarkedContent", + ]); + + await loadingTask.destroy(); + }); + + it("preserves marked content spanning text objects (bug 1823296)", async function () { + const loadingTask = getDocument(buildGetDocumentParams("bug1823296.pdf")); + const pdfDoc = await loadingTask.promise; + const pdfPage = await pdfDoc.getPage(1); + const { items } = await pdfPage.getTextContent({ + includeMarkedContent: true, + disableNormalization: true, + }); + + const start = items.findIndex(item => item.id === "p3R_mc8"); + const end = items.findIndex( + (item, index) => index > start && item.type === "endMarkedContent" + ); + + expect(items.slice(start + 1, end).map(item => item.str)).toEqual([ + "", + "PDF/UA is not a separate file-format but simply a way to use the " + + "familiar PDF format invented by Adobe", + "Systems and now standardized as ISO 32000.[5]", + ]); + + await loadingTask.destroy(); + }); + it("gets text content with multi-byte entries, using predefined CMaps (issue 16176)", async function () { const loadingTask = getDocument( buildGetDocumentParams("issue16176.pdf", { diff --git a/test/unit/evaluator_spec.js b/test/unit/evaluator_spec.js index cb8b5ff15087f..fe8c795f40263 100644 --- a/test/unit/evaluator_spec.js +++ b/test/unit/evaluator_spec.js @@ -50,6 +50,26 @@ describe("evaluator", function () { return operatorList; } + async function runTextContentCheck(evaluator, stream) { + const items = []; + const sink = { + desiredSize: 100, + ready: Promise.resolve(), + enqueue(chunk) { + items.push(...chunk.items); + }, + }; + const task = new WorkerTask("TextContentCheck"); + await evaluator.getTextContent({ + stream, + task, + resources: new ResourcesMock(), + includeMarkedContent: true, + sink, + }); + return items; + } + let partialEvaluator; beforeAll(function () { @@ -444,6 +464,50 @@ describe("evaluator", function () { }); }); + describe("text content", function () { + it("should close marked content opened in a text object", async function () { + const stream = new StringStream( + "/Outer BMC BT /Inner BMC ET /Sibling BMC EMC EMC" + ); + const items = await runTextContentCheck(partialEvaluator, stream); + + expect(items).toEqual([ + { type: "beginMarkedContent", tag: "Outer" }, + { type: "beginMarkedContent", tag: "Inner" }, + { type: "endMarkedContent" }, + { type: "beginMarkedContent", tag: "Sibling" }, + { type: "endMarkedContent" }, + { type: "endMarkedContent" }, + ]); + }); + + it("should preserve marked content opened before a text object", async function () { + const stream = new StringStream( + "/Outer BMC BT ET BT ET /Inner BMC EMC EMC" + ); + const items = await runTextContentCheck(partialEvaluator, stream); + + expect(items).toEqual([ + { type: "beginMarkedContent", tag: "Outer" }, + { type: "beginMarkedContent", tag: "Inner" }, + { type: "endMarkedContent" }, + { type: "endMarkedContent" }, + ]); + }); + + it("should not close marked content on an unmatched endText", async function () { + const stream = new StringStream("/Outer BMC ET /Inner BMC EMC EMC"); + const items = await runTextContentCheck(partialEvaluator, stream); + + expect(items).toEqual([ + { type: "beginMarkedContent", tag: "Outer" }, + { type: "beginMarkedContent", tag: "Inner" }, + { type: "endMarkedContent" }, + { type: "endMarkedContent" }, + ]); + }); + }); + describe("operator list", function () { class StreamSinkMock { enqueue() {} From 47423d1c290dae92725e7824e8ff85bbc35226c4 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 19 Aug 2026 10:36:33 +0200 Subject: [PATCH 2/3] Improve `JpegImage.canUseImageDecoder` return value consistency For JPEG images that can be decoded natively, always return an Object with a consistent shape regardless of the marker data. Also, tweak a comment in `JpegImage.getTransferableImage` to avoid ambiguity. Finally, replace `TextEncoder` usage with our `stringToBytes` helper in a unit-test. --- src/core/jpeg_stream.js | 2 +- src/core/jpg.js | 20 ++++++++++++-------- test/unit/jpeg_stream_spec.js | 11 ++++++----- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/core/jpeg_stream.js b/src/core/jpeg_stream.js index 0ef04ae2ccbec..f1df733dcece5 100644 --- a/src/core/jpeg_stream.js +++ b/src/core/jpeg_stream.js @@ -179,7 +179,7 @@ class JpegStream extends DecodeStream { // the height is only known from a DNL marker or because the scan simply // ends early (issue15492.pdf). `ImageDecoder` reports and scales the // frame according to the SOF, so let our own decoder, which honours the - // dictionary, handle the image instead. + // actual JPEG image data, handle the image instead. return null; } if (useImageDecoder.exifStart) { diff --git a/src/core/jpg.js b/src/core/jpg.js index 3c51ea30d080a..9c624b6e2db84 100644 --- a/src/core/jpg.js +++ b/src/core/jpg.js @@ -811,11 +811,14 @@ class JpegImage { static canUseImageDecoder(data, colorTransform = -1) { const view = new DataView(data.buffer, data.byteOffset, data.byteLength); - let exifOffsets = null; + const info = { + width: 0, + height: 0, + exifStart: 0, + exifEnd: 0, + }; let offset = 0; let numComponents = null; - let scanLines = 0, - samplesPerLine = 0; let fileMarker = view.getUint16(offset); offset += 2; if (fileMarker !== /* SOI (Start of Image) = */ 0xffd8) { @@ -845,12 +848,13 @@ class JpegImage { appData[4] === 0 && appData[5] === 0 ) { - if (exifOffsets) { + if (info.exifStart) { throw new JpegError("Duplicate EXIF-blocks found."); } // Don't do the EXIF-block replacement here, see `JpegStream`, // since that can modify the original PDF document. - exifOffsets = { exifStart: oldOffset + 6, exifEnd: newOffset }; + info.exifStart = oldOffset + 6; + info.exifEnd = newOffset; } fileMarker = view.getUint16(offset); offset += 2; @@ -860,8 +864,8 @@ class JpegImage { case 0xffc2: // SOF2 (Start of Frame, Progressive DCT) // Skip marker length. // Skip precision. - scanLines = view.getUint16(offset + (2 + 1)); - samplesPerLine = view.getUint16(offset + (2 + 1 + 2)); + info.height = view.getUint16(offset + (2 + 1)); // scanLines + info.width = view.getUint16(offset + (2 + 1 + 2)); // samplesPerLine numComponents = data[offset + (2 + 1 + 2 + 2)]; break markerLoop; case 0xffff: // Fill bytes @@ -882,7 +886,7 @@ class JpegImage { return null; } // A zero SOF height means that a later DNL marker defines it. - return { width: samplesPerLine, height: scanLines, ...exifOffsets }; + return info; } parse(data, { dnlScanLines = null } = {}) { diff --git a/test/unit/jpeg_stream_spec.js b/test/unit/jpeg_stream_spec.js index 39d6e2ae7d66e..a5b2aa40fb59d 100644 --- a/test/unit/jpeg_stream_spec.js +++ b/test/unit/jpeg_stream_spec.js @@ -18,6 +18,7 @@ import { ImageResizer } from "../../src/core/image_resizer.js"; import { JpegImage } from "../../src/core/jpg.js"; import { JpegStream } from "../../src/core/jpeg_stream.js"; import { Stream } from "../../src/core/stream.js"; +import { stringToBytes } from "../../src/shared/util.js"; // Only a JPEG header is needed: `canUseImageDecoder` stops at the SOF marker. function createJpeg({ @@ -57,13 +58,13 @@ describe("jpeg_stream", function () { it("should report the frame dimensions", function () { expect( JpegImage.canUseImageDecoder(createJpeg({ width: 40000, height: 4000 })) - ).toEqual({ width: 40000, height: 4000 }); + ).toEqual({ width: 40000, height: 4000, exifStart: 0, exifEnd: 0 }); expect( JpegImage.canUseImageDecoder( createJpeg({ width: 123, height: 45, numComponents: 1 }) ) - ).toEqual({ width: 123, height: 45 }); + ).toEqual({ width: 123, height: 45, exifStart: 0, exifEnd: 0 }); }); it("should report dimensions for each supported SOF marker", function () { @@ -78,19 +79,19 @@ describe("jpeg_stream", function () { ) ) .withContext(sofMarker.toString(16)) - .toEqual({ width: 40000, height: 4000 }); + .toEqual({ width: 40000, height: 4000, exifStart: 0, exifEnd: 0 }); } }); it("should report a zero SOF height", function () { expect( JpegImage.canUseImageDecoder(createJpeg({ width: 40000, height: 0 })) - ).toEqual({ width: 40000, height: 0 }); + ).toEqual({ width: 40000, height: 0, exifStart: 0, exifEnd: 0 }); }); it("should report the frame dimensions together with the EXIF-offsets", function () { const payload = [1, 2, 3, 4]; - const appData = [...new TextEncoder().encode("Exif\x00\x00"), ...payload]; + const appData = [...stringToBytes("Exif\x00\x00"), ...payload]; // SOI (2) + APP1-marker (2) + length (2) + "Exif\x00\x00" (6) = 12. expect( From cdba9505866a21ab25f6d1834a782dc8a38747b1 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 19 Aug 2026 13:14:06 +0200 Subject: [PATCH 3/3] Cache the `TextDecoder` instances used by the `stringToPDFString` helper This potentially avoids creating a lot of "duplicate" `TextDecoder` instances when parsing/rendering longer PDF documents. For example, when rendering all 1310 pages of the `pdf.pdf` document (from the test-suite) this patch reduces the number of `TextDecoder` instances created by `stringToPDFString` from *over* `33 000` to just a single one. --- src/core/string_utils.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/core/string_utils.js b/src/core/string_utils.js index 0711c166ed5a0..cb8bf2cd11b5c 100644 --- a/src/core/string_utils.js +++ b/src/core/string_utils.js @@ -62,6 +62,7 @@ const PDFStringTranslateTable = [ 0x2019, 0x201a, 0x2122, 0xfb01, 0xfb02, 0x141, 0x152, 0x160, 0x178, 0x17d, 0x131, 0x142, 0x153, 0x161, 0x17e, 0, 0x20ac, ]; +const PDFStringTextDecoders = Object.create(null); function stringToPDFString(str, keepEscapeSequence = false) { // See section 7.9.2.2 Text String Type. @@ -85,7 +86,10 @@ function stringToPDFString(str, keepEscapeSequence = false) { if (encoding) { try { - const decoder = new TextDecoder(encoding, { fatal: true }); + const decoder = (PDFStringTextDecoders[encoding] ??= new TextDecoder( + encoding, + { fatal: true } + )); const buffer = stringToBytes(str); const decoded = decoder.decode(buffer); if (keepEscapeSequence || !decoded.includes("\x1b")) {