[
https://issues.apache.org/jira/browse/TIKA-4856?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18113671#comment-18113671
]
ASF GitHub Bot commented on TIKA-4856:
--------------------------------------
Copilot commented on code in PR #3146:
URL: https://github.com/apache/tika/pull/3146#discussion_r3975492231
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-pdf-module/src/test/java/org/apache/tika/renderer/pdf/pdfbox/PDFBoxRendererTest.java:
##########
@@ -83,6 +83,19 @@ public void testImageQualityConfigurable() throws Exception {
"quality 1.0 should be far larger: " + uncompressed + " vs " +
compressed);
}
+ /** A range that runs past the last page ends there; it is "the first N
pages", not an error. */
+ @Test
+ public void testRangePastTheLastPageIsClamped() throws Exception {
+ PDFBoxRenderer renderer = new PDFBoxRenderer();
+ try (InputStream is =
getClass().getResourceAsStream("/test-documents/testPDF.pdf");
+ TikaInputStream tis = TikaInputStream.get(is);
+ PageBasedRenderResults results = (PageBasedRenderResults)
renderer.render(
+ tis, new Metadata(), new ParseContext(), new
PageRangeRequest(1, 9999))) {
+ assertEquals(1, results.getResults().size());
+ assertEquals(RenderResult.STATUS.SUCCESS,
results.getResults().get(0).getStatus());
+ }
+ }
Review Comment:
The test currently asserts `size() == 1`, which only verifies clamping if
`/test-documents/testPDF.pdf` is a single-page PDF; it won’t catch regressions
for multi-page documents. Consider switching to a known multi-page fixture (or
asserting against that fixture’s expected page count) and verifying
`results.getResults().size()` equals the document’s page count when requesting
`(1, veryLargeNumber)`.
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-pdf-module/src/main/java/org/apache/tika/parser/pdf/PDF2XHTML.java:
##########
@@ -167,6 +167,10 @@ private void renderPage(PDPage page) throws IOException {
if (config.getImageStrategy() !=
PDFParserConfig.IMAGE_STRATEGY.RENDER_PAGES_AT_PAGE_END) {
return;
}
+ int maxRenderedPages = config.getMaxRenderedPages();
+ if (maxRenderedPages > 0 && getCurrentPageNo() > maxRenderedPages) {
+ return;
+ }
Review Comment:
This limit relies on the page-numbering convention of `getCurrentPageNo()`
(1-based vs 0-based). Adding a short clarifying comment (or aligning the check
explicitly with the same 1-based semantics used by `new PageRangeRequest(1,
maxRenderedPages)`) would reduce the risk of future off-by-one changes.
> /unpack/thumbnail: return the document thumbnail with its metadata
> ------------------------------------------------------------------
>
> Key: TIKA-4856
> URL: https://issues.apache.org/jira/browse/TIKA-4856
> Project: Tika
> Issue Type: New Feature
> Reporter: Dominik Schmidt
> Priority: Major
>
> With TIKA-4850 through TIKA-4855 every container format that carries a
> thumbnail emits it as a THUMBNAIL embedded document, the PDF parser renders
> pages as RENDERING documents, and the EMF/WMF renderer turns the vector
> thumbnails of Office documents into raster ones. Getting "the thumbnail of
> this file" out of that still takes format knowledge on the client: the
> THUMBNAIL of a Word or Excel file is an EMF/WMF whose usable form is the
> RENDERING underneath it, a PDF has no THUMBNAIL but a page RENDERING, the
> THUMBNAIL of a DOCX inside a ZIP is not the ZIP's, and with rendering enabled
> the picture of an embedded OLE object is a RENDERING too. Plus the request
> config that switches the renderers on.
> Proposal: POST /unpack/thumbnail next to /unpack and /unpack/all, multipart
> like them. It runs the usual forked parse in unpack mode with a fixed parse
> context (PDF page 1 rendered, EMF/WMF rendered) and picks, in this order: the
> raster THUMBNAIL at depth 1; the rendering of that thumbnail; the depth-1
> RENDERING of PDF page 1. The endpoint extracts what the document carries; it
> does not resize, convert or generate previews.
> The response is JSON: the /rmeta metadata object of the selected embedded
> document, and the image as base64. Thumbnails are small, so the encoding
> overhead does not matter, and the caller gets type, dimensions, origin
> (stored thumbnail or rendering, tk:rendering:rendered-by) and path in one
> round trip without unpacking a zip. 204 when the document has no thumbnail.
> {
> "metadata": {
> "Content-Type": "image/png",
> "Content-Length": "8459",
> "tiff:ImageWidth": "800",
> "tiff:ImageLength": "1131",
> "tk:embedded-resource-type": "RENDERING",
> "tk:embedded-resource-path": "/thumbnail.emf/thumbnail.png",
> "tk:embedded-depth": "2",
> "tk:rendering:rendered-by": "poi-metafile-renderer",
> "tk:resource-name": "thumbnail.png"
> },
> "image": "iVBORw0KGgoAAAANSUhEUgAA..."
> }
> To keep the selection rule short, the metafile renderer could give the
> rendering of a THUMBNAIL the THUMBNAIL type as well (its
> tk:rendering:rendered-by tells it apart), so a raster thumbnail is a
> THUMBNAIL regardless of whether the document stored it as PNG or as EMF.
> What do you think?
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
