Copilot commented on code in PR #3209:
URL: https://github.com/apache/tika/pull/3209#discussion_r4053079332
##########
tika-core/src/main/java/org/apache/tika/io/ReopenableSource.java:
##########
@@ -363,16 +368,36 @@ public void close() throws IOException {
}
}
+ /**
+ * A mark is kept in the open stream's buffer, so a reset within {@code
readlimit} costs
+ * nothing. Re-opening on every reset is what a reader that marks and
resets per record (POI
+ * reading a metafile's bitmaps) turned into inflating a zip entry
thousands of times.
+ */
@Override
public synchronized void mark(int readlimit) {
markPosition = position;
+ markInStream = false;
+ if (currentStream != null && retainedBuffer == null) {
+ // the buffer grows to honour a mark; past the cap a reset
re-opens instead
+ currentStream.mark(Math.min(readlimit, MAX_BUFFERED_MARK));
+ markInStream = true;
+ }
}
@Override
public synchronized void reset() throws IOException {
if (markPosition < 0) {
throw new IOException("Mark not set");
}
+ if (markInStream && currentStream != null) {
+ try {
+ currentStream.reset();
+ position = markPosition;
Review Comment:
`markInStream` can be stale here: `tryRetainInMemory()` closes and replaces
`currentStream` with a `ByteArrayInputStream` without clearing this flag. If a
caller marks, retains/opens a channel, and then resets, this reset succeeds at
the replacement stream's own initial mark (the current position), while
`position` is set to the old `markPosition`, so subsequent reads start at the
wrong offset. Invalidate the flag whenever the stream is replaced, or only take
this fast path for the originally marked stream.
--
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]
