tballison commented on PR #3540:
URL: https://github.com/apache/tika/pull/3540#issuecomment-6665843817
Bot nits below.
If you have any example files for unit tests great. Even if they're
synthetic, they'd be good to add, but that isn't a blocker for me.
```
1. No real-file fixture (SketchUpParserTest). Every test file comes from
the test's own header(), cdib() and zipped()
helpers. So the tests show the parser matches the author's model of the
format, not SketchUp's. The "107 real files,
byte for byte" claim can't be re-checked from the repo. Ask for one
small, redistributable file per layout (a CDib one
up to 2020, a zip one from 2021), made by the author.
2. The CDib marker hard-codes schema 3 (SketchUpParser.java, the CDIB
constant). The javadoc calls it "a 16-bit schema",
but the search matches only 03 00. A version that writes a different
schema would silently produce no thumbnail rather
than an error. Either match only ff ff ?? ?? 04 00 CDib or state that 3
is the only schema seen.
3. The CDib read sizes itself from the record's length field.
IOUtils.toByteArray(record, (int) length) trusts that field,
up to 16 MB. Depending on the commons-io version, a tiny file could
make it allocate the full 16 MB upfront. The zip
path already uses readNBytes; doing the same here and checking the byte
count would be consistent and safer.
```
--
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]
