tballison commented on PR #3095:
URL: https://github.com/apache/tika/pull/3095#issuecomment-5493234683
From my :robot: ... I think most are useful. One or two are puntable. Let me
know what you think. Thank you for iterating.
```
⎿
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/renderer/microsoft/POIMetafileRenderer.java
● 187 [security] Rendered height is unbounded (only width is
capped at 10000): a hostile metafile's aspect ratio drives an arbitrarily large
BufferedImage allocation, and the resulting OutOfMemoryError is an Error that
escapes every
catch(Exception) in the render path — in
both canvas() (line 187) and the scale() bitmap fallback (line 207).
● 152 [correctness] draw(HwmfPicture) catches bare
RuntimeException around picture.getSize() when the intended case is exactly
POI's IllegalStateException("invalid wmf file - window records are
incomplete.") — unrelated runtime failures
(e.g. an NPE from a POI bug) get silently
rerouted to the first-bitmap fallback.
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/MetafileRendering.java
● 61 [correctness] renderWidth (and imageFormatName) is
silently dead in every default configuration: @TikaComponent on
POIMetafileRenderer defaults to spi=true, so DefaultParser's always-injected
SPI CompositeRenderer claims image/emf|wmf
and takes precedence over
defaultRenderer(config) — the PR's own tests request widths 200/300/400, render
at 800, and pass because no test asserts output width.
● 67 [correctness] Injected renderers are handed
TikaInputStream.get(new byte[0]) with the parsed picture only as an open
container — a POIMetafileRenderer-private convention — so any third-party
Renderer that follows the Renderer contract
and reads the stream renders 0 bytes on
every file.
● 90 [correctness] Rewriting the renderer-assigned RENDERING
type to THUMBNAIL hides thumbnail renderings from type-based filters and yields
two THUMBNAIL-typed embedded docs for one file.
● 145 [simplification] renderingName() re-implements basename
extraction that tika-core's FilenameUtils.getName() already provides, missing
its ':' handling and '.'/'..' sanitization.
● 68 [efficiency] The full rasterization (vector draw + PNG
encode + temp-file write) runs before extractor.shouldParseEmbedded is
consulted, so an extractor that filters out RENDERING docs still pays the
entire render cost per image.
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/OfficeParser.java
● 310 [compatibility] OLE2 SummaryInformation thumbnail emission
is default-on with no opt-out: handleThumbnail runs unconditionally
(MetafileParserConfig.renderImage gates only rendering; OfficeParserConfig has
no thumbnail switch), adding an
extra thumbnail.wmf embedded document to
every OLE2 file with a stored thumbnail.
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/MetafileParserConfig.java
● 86 [config-validation] renderOnlyEmbeddedResourceTypes is an
unvalidated case-sensitive Set<String> matched against
EmbeddedResourceType.name(): a config typo silently disables rendering with no
error.
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/WMFParser.java
● 71 [simplification] EMFParser and WMFParser duplicate ~35 lines
of identical wiring (fields, three constructors, getConfig differing only in
the "emf-parser"/"wmf-parser" key, setRenderer plus a caller-less getRenderer),
and
POIMetafileRenderer's two draw() overloads
and canvas()/scale() duplicate the canvas/height-computation logic.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
