Copilot commented on code in PR #2727:
URL: https://github.com/apache/tika/pull/2727#discussion_r3015364218
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-zip-commons/src/test/java/org/apache/tika/detect/zip/ZipDetectionTest.java:
##########
@@ -79,7 +80,8 @@ public void testStreaming() throws Exception {
detector = new DefaultZipContainerDetector();
//try on a file that isn't a TikaInputStream
- try (InputStream is = new
BufferedInputStream(Files.newInputStream(TikaInputStream.get(getStream("testJAR.jar")).getPath())))
{
+ try (TemporaryResources tmp = new TemporaryResources();
+ InputStream is = new
BufferedInputStream(Files.newInputStream(TikaInputStream.get(getStream("testJAR.jar"),
tmp, new Metadata()).getPath()))) {
Review Comment:
`TikaInputStream.get(stream, tmp, metadata)` is explicitly documented to
*not* close the original `stream` when `TemporaryResources.close()` is called.
In this test, the `InputStream` returned by `getStream("testJAR.jar")` (and the
internal wrapper that would close it) is never closed because the
`TikaInputStream` instance is only used transiently for `getPath()` and is not
closed. This can still leak the underlying stream/file handle.
Consider creating a named `TikaInputStream` and including it in the
try-with-resources (alongside `tmp` and the file-based `is`), or otherwise
explicitly closing the `TikaInputStream`/original stream after `getPath()` is
obtained.
```suggestion
TikaInputStream tis =
TikaInputStream.get(getStream("testJAR.jar"), tmp, new Metadata());
InputStream is = new
BufferedInputStream(Files.newInputStream(tis.getPath()))) {
```
--
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]
