tballison commented on PR #3536:
URL: https://github.com/apache/tika/pull/3536#issuecomment-6665363568

   Bot nits:
   
   ```
    1. RAR5 timestamps are shifted by the JVM's time zone (RarParser.java, the 
three setLocalTime calls). RAR4 stores times as
        DOS local time. RAR5 stores true UTC instants (Unix or Windows 
FILETIME; see Rar5FileHeaderReader.readTime). The parser
        converts every Date to wall-clock time in the default zone and writes 
it without a zone. So a RAR5 entry's MODIFIED
        value depends on the server's zone and is wrong as UTC. Its comment 
"RAR4 times are DOS local time" is now only
        half-true.
        - Fix: branch on rar.getFormat() == RarFormat.RAR50 and set the instant 
as a UTC date in that case.
        - Test: add an mtime to the synthetic archive and assert the result 
under a non-UTC default zone.
     2. The dictionary size is uncapped (new Archive(tis.getFile())). junrar's 
default maxDictionarySize is 4 GiB. Its RAR5
        window is allocated lazily but grows as bytes are decompressed. A 
small, highly compressible RAR5 that declares a large
        dictionary can therefore grow heap use up to min(4 GiB, output) inside 
one entry. RAR4 dictionaries topped out at 4 MB,
        so this exposure is new with this PR.
        - Under Tika's security model this is DoS and not a vulnerability.
        - Still, it costs one line: new Archive(file, 
ArchiveOptions.builder().maxDictionarySize(...).build()). 256 MB to 1 GB
          covers what WinRAR 5/6 produce. Choosing the cap, or making it 
configurable, is a maintainer decision.
   ```


-- 
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]

Reply via email to