fix(extractor): bound Ps/Json/Pdf extractor memory usage#179
Merged
Conversation
Prevent unbounded heap use when extracting large or hostile PostScript, JSON, and PDF documents, following the module's existing truncate-not-reject convention. All new bounds default to unlimited, so normal inputs are unchanged. - PsExtractor: read via AbstractExtractor.readWithLimit with a configurable maxTextLength (default unlimited) instead of slurping the whole stream into a byte[] and String. - JsonExtractor: cap the accumulated output text with a new TextAccumulator (which also bounds object-field output, not just arrays), and pin the ObjectMapper StreamReadConstraints to Jackson's own defaults (nesting 1000, string 20_000_000, number 1000) so the parse-time DoS ceilings are explicit without rejecting any input a default ObjectMapper would accept. - PdfExtractor: bound the extracted text with a BoundedTextWriter and spool the input to a temp file for PDFBox's file-backed loading instead of buffering the whole PDF in memory; the temp file is always cleaned up. Add tests for truncation, normal-input parity, a legitimately deep (500-level) JSON that must not be rejected, and PDF temp-file cleanup.
…ngth Add a shared AbstractExtractor.limitInputStream(in, limit) helper and an opt-in maxContentLength (default 0 = unlimited) to JsonExtractor and PdfExtractor so oversized input is rejected with MaxLengthExceededException before it can exhaust heap or disk: - JsonExtractor: readTree() materializes the whole JsonNode tree, so bounding maxTextLength (output) alone did not cap the dominant allocation. Wrap the parse input so the tree is built from at most maxContentLength bytes. Also correct the class Javadoc: the pinned StreamReadConstraints match Jackson's own defaults and are not the memory guard. - PdfExtractor: the input was spooled to a temp file unbounded, trading a heap-exhaustion vector for a disk-exhaustion one. Bound the spool with the same helper. Defaults are unchanged (maxContentLength <= 0 returns the stream unwrapped), so existing behavior is byte-for-byte identical. PsExtractor already bounds its read and is untouched. Add tests for limitInputStream (exactly-limit accepted, limit+1 rejected, non-positive returns the same instance) and for the new reject/under-cap paths in JsonExtractorTest and PdfExtractorTest.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Bound the heap usage of three extractors that previously read/accumulated their input without any limit, so a large or hostile PostScript, JSON, or PDF document can no longer drive unbounded memory growth. All new bounds follow the module's existing truncate-not-reject convention and default to unlimited, so normal inputs are unchanged.
Changes
new String(InputStreamUtil.getBytes(in), enc)(whole stream →byte[]→String) withAbstractExtractor.readWithLimit(reader, maxTextLength), the same helperTextExtractor/MarkdownExtractoruse. New configurablemaxTextLength(defaultLong.MAX_VALUE).TextAccumulator, which also bounds object-field output (previously only arrays were bounded viamaxArrayElements; a huge flat object could still produce unbounded output).ObjectMapperStreamReadConstraintsexplicitly to Jackson's own defaults (maxNestingDepth=1000,maxStringLength=20_000_000,maxNumberLength=1000). This documents the parse-time DoS ceilings without tightening them below what a defaultObjectMapperalready enforces — so no legitimate JSON that parsed before is rejected.BoundedTextWriter(caps the sum across the page text, annotations, and embedded documents).Loader.loadPDF(File)instead ofRandomAccessReadBuffer(InputStream)(which buffers the whole PDF in heap). The temp file is always deleted in afinally(viaFileUtil.deleteInBackground), after the document is closed. The per-call extraction executor/timeout is unchanged.All truncation is high-surrogate-safe and sets the
truncated/maxTextLengthExtractDatametadata, consistent with the existingreadWithLimitbehavior.Testing
JsonExtractorTest: a legitimately deep (500-level) JSON is not rejected and extracts content — this guards against re-tighteningmaxNestingDepthbelow Jackson's default (it fails if the limit is dropped back to a small value); plus a truly-pathological (2000-level) document is rejected.PdfExtractorTest: aBoundedTextWriterunit test proving the buffer never exceeds the cap, and a temp-file-cleanup test.fess-crawlermodule suite passes (a pre-existing timing-sensitiveFtpClientTestflake under parallel load is unrelated — these changes do not touch FTP — and passes in isolation);mvn formatter:format/license:format/javadoc:jarclean.