Control the remaining cap correctly for reuse

Fixes jhy#2448

And refactored mark / fill to simplify control flow

Author: jhy

CHANGES.md

@@ -32,6 +32,8 @@
 * Cloning a `Parser` now preserves any custom `TagSet` applied to the parser. [#2422](https://github.com/jhy/jsoup/issues/2422), [#2423](https://github.com/jhy/jsoup/pull/2423)
 * Custom tags marked as `Tag.Void` now parse and serialize like the built-in void elements: they no longer consume following content, and the XML serializer emits the expected self-closing form. [#2425](https://github.com/jhy/jsoup/issues/2425)
 * The `<br>` element is once again classified as an inline tag (`Tag.isBlock() == false`), matching common developer expectations and its role as phrasing content in HTML, while pretty-printing and text extraction continue to treat it as a line break in the rendered output. [#2387](https://github.com/jhy/jsoup/issues/2387), [#2439](https://github.com/jhy/jsoup/issues/2439)
+* Fixed an intermittent truncation when fetching and parsing remote documents via `Jsoup.connect(url).get()`. On responses without a charset header, the initial charset sniff could sometimes (depending on buffering / `available()` behavior) be mistaken for end-of-stream and a partial parse reused, dropping trailing content. [#2448](https://github.com/jhy/jsoup/issues/2448)
+
 
 ### Internal Changes
 * Deprecated internal helper `org.jsoup.internal.Functions` (for removal in v1.23.1). This was previously used to support older Android API levels without full `java.util.function` coverage; jsoup now requires core library desugaring so this indirection is no longer necessary. [#2412](https://github.com/jhy/jsoup/pull/2412)

src/main/java/org/jsoup/helper/DataUtil.java

@@ -248,6 +248,7 @@ static CharsetDoc detectCharset(ControllableInputStream input, @Nullable String
         if (charsetName == null) { // read ahead and determine from meta. safe first parse as UTF-8
             int origMax = input.max();
             input.max(firstReadBufferSize);
+            input.resetFullyRead(); // clear any pre-read (e.g., BOM) state before capped sniff
             input.mark(firstReadBufferSize);
             input.allowClose(false); // ignores closes during parse, in case we need to rewind
             try (Reader reader = new SimpleStreamReader(input, UTF_8)) { // input is currently capped to firstReadBufferSize

src/main/java/org/jsoup/internal/ControllableInputStream.java

@@ -20,19 +20,19 @@
 // reimplemented from ConstrainableInputStream for JDK21 - extending BufferedInputStream will pin threads during read
 public class ControllableInputStream extends FilterInputStream {
     private final SimpleBufferedInput buff; // super.in, but typed as SimpleBufferedInput
-    private int maxSize;
-    private long startTime;
-    private long timeout = 0; // optional max time of request
-    private int remaining;
-    private int markPos;
-    private boolean interrupted;
-    private boolean allowClose = true; // for cases where we want to re-read the input, can ignore .close() from the parser
+    private int maxSize;                    // logical cap exposed to callers (0 == unlimited)
+    private long startTime;                 // start time for timeout checks, nanos
+    private long timeout = 0;               // optional max time of request
+    private int remaining;                  // how many bytes may still be returned to caller under the current cap
+    private int markPos;                    // logical readPos snapshot for InputStream.mark/reset (not a buffer cursor)
+    private boolean interrupted;            // true if Thread.interrupted() was detected, used to latch interrupted state
+    private boolean allowClose = true;      // for cases where we want to re-read the input, can ignore .close() from the parser
 
     // if we are tracking progress, will have the expected content length, progress callback, connection
     private @Nullable Progress<?> progress;
     private @Nullable Object progressContext;
-    private int contentLength = -1;
-    private int readPos = 0; // amount read; can be reset()
+    private int contentLength = -1;         // expected content length for progress; -1 == unknown
+    private int readPos = 0;                // amount read; can be reset()
 
     private ControllableInputStream(SimpleBufferedInput in, int maxSize) {
         super(in);
@@ -85,6 +85,7 @@ public int read(byte[] b, int off, int len) throws IOException {
 
         if (capped && len > remaining)
             len = remaining; // don't read more than desired, even if available
+        buff.capRemaining(capped ? remaining : Integer.MAX_VALUE);
 
         while (true) { // loop trying to read until we get some data or hit the overall timeout, if we have one
             if (expired())
@@ -95,7 +96,9 @@ public int read(byte[] b, int off, int len) throws IOException {
                 if (read == -1) { // completed
                     contentLength = readPos;
                 } else {
-                    remaining -= read;
+                    if (capped && read > 0) {
+                        remaining -= read; // track bytes returned to the caller
+                    }
                     readPos += read;
                 }
                 emitProgress();
@@ -107,6 +110,11 @@ public int read(byte[] b, int off, int len) throws IOException {
         }
     }
 
+    @Override
+    public boolean markSupported() {
+        return true;
+    }
+
     /**
      * Reads this inputstream to a ByteBuffer. The supplied max may be less than the inputstream's max, to support
      * reading just the first bytes.
@@ -145,15 +153,24 @@ public static ByteBuffer readToByteBuffer(InputStream in, int max) throws IOExce
 
     @SuppressWarnings("NonSynchronizedMethodOverridesSynchronizedMethod") // not synchronized in later JDKs
     @Override public void reset() throws IOException {
-        super.reset();
-        remaining = maxSize - markPos;
+        if (markPos < 0) throw new IOException("Resetting to invalid mark");
+        buff.rewindToMark();
+        buff.clearMark();
+        if (maxSize != 0) {
+            remaining = maxSize - markPos;
+            buff.capRemaining(remaining);
+        } else {
+            remaining = 0;
+            buff.capRemaining(Integer.MAX_VALUE);
+        }
         readPos = markPos; // readPos is used for progress emits
+        markPos = -1;
     }
 
     @SuppressWarnings("NonSynchronizedMethodOverridesSynchronizedMethod") // not synchronized in later JDKs
     @Override public void mark(int readlimit) {
-        super.mark(readlimit);
-        markPos = maxSize - remaining;
+        markPos = readPos;
+        buff.setMark();
     }
 
     /**
@@ -165,6 +182,10 @@ public boolean baseReadFully() {
         return buff.baseReadFully();
     }
 
+    public void resetFullyRead() {
+        buff.resetFullyRead();
+    }
+
     /**
      Get the max size of this stream (how far at most will be read from the underlying stream)
      * @return the max size
@@ -175,7 +196,9 @@ public int max() {
 
     public void max(int newMax) {
         remaining += newMax - maxSize; // update remaining to reflect the difference in the new maxsize
+        if (remaining < 0) remaining = 0;
         maxSize = newMax;
+        buff.capRemaining(newMax == 0 ? Integer.MAX_VALUE : remaining);
     }
 
     public void allowClose(boolean allowClose) {

src/main/java/org/jsoup/internal/SimpleBufferedInput.java

@@ -17,11 +17,12 @@
 class SimpleBufferedInput extends FilterInputStream {
     static final int BufferSize = DefaultBufferSize;
     static final SoftPool<byte[]> BufferPool = new SoftPool<>(() -> new byte[BufferSize]);
+    private int capRemaining = Integer.MAX_VALUE; // how many bytes we are allowed to pull from the underlying stream
 
     private byte @Nullable [] byteBuf; // the byte buffer; recycled via SoftPool. Created in fill if required
     private int bufPos;
     private int bufLength;
-    private int bufMark = -1;
+    private int bufMark = -1; // mark set by ControllableInputStream; -1 when unset
     private boolean inReadFully = false; // true when the underlying inputstream has been read fully
 
     SimpleBufferedInput(@Nullable InputStream in) {
@@ -50,12 +51,6 @@ public int read(byte[] dest, int offset, int desiredLen) throws IOException {
 
         int bufAvail = bufLength - bufPos;
         if (bufAvail <= 0) { // can't serve from the buffer
-            if (!inReadFully && bufMark < 0) {
-                // skip creating / copying into a local buffer; just pass through
-                int read = in.read(dest, offset, desiredLen);
-                closeIfDone(read);
-                return read;
-            }
             fill();
             bufAvail = bufLength - bufPos;
         }
@@ -76,28 +71,22 @@ private void fill() throws IOException {
             byteBuf = BufferPool.borrow();
         }
 
-        if (bufMark < 0) { // no mark, can lose buffer (assumes we've read to bufLen)
-            bufPos = 0;
-        } else if (bufPos >= BufferSize) { // no room left in buffer
-            if (bufMark > 0) { // can throw away early part of the buffer
-                int size = bufPos - bufMark;
-                System.arraycopy(byteBuf, bufMark, byteBuf, 0, size);
-                bufPos = size;
-                bufMark = 0;
-            } else { // invalidate mark
-                bufMark = -1;
-                bufPos = 0;
-            }
-        }
+        compact();
         bufLength = bufPos;
-        int read = in.read(byteBuf, bufPos, byteBuf.length - bufPos);
+        int toRead = Math.min(byteBuf.length - bufPos, capRemaining);
+        if (toRead <= 0) return;
+        int read = in.read(byteBuf, bufPos, toRead);
         if (read > 0) {
             bufLength = read + bufPos;
-            while (byteBuf.length - bufLength > 0) { // read in more if we have space, without blocking
+            capRemaining -= read;
+            while (byteBuf.length - bufLength > 0 && capRemaining > 0) { // read in more if we have space, without blocking
                 if (in.available() < 1) break;
-                read = in.read(byteBuf, bufLength, byteBuf.length - bufLength);
+                toRead = Math.min(byteBuf.length - bufLength, capRemaining);
+                if (toRead <= 0) break;
+                read = in.read(byteBuf, bufLength, toRead);
                 if (read <= 0) break;
                 bufLength += read;
+                capRemaining -= read;
             }
         }
         closeIfDone(read);
@@ -123,30 +112,55 @@ boolean baseReadFully() {
         return inReadFully;
     }
 
-    @Override
-    public int available() throws IOException {
-        if (byteBuf != null && bufLength - bufPos > 0)
-            return bufLength - bufPos; // doesn't include those in.available(), but mostly used as a block test
-        return inReadFully ? 0 : in.available();
+    void resetFullyRead() {
+        if (in != null) // for null-wrapped streams, leave as fully read to avoid fill() on a null input
+            inReadFully = false;
     }
 
-    @SuppressWarnings("NonSynchronizedMethodOverridesSynchronizedMethod") // explicitly not synced
     @Override
-    public void mark(int readlimit) {
-        if (readlimit > BufferSize) {
-            throw new IllegalArgumentException("Read-ahead limit is greater than buffer size");
+    public int available() throws IOException {
+        int buffered = (byteBuf != null) ? (bufLength - bufPos) : 0;
+        if (buffered > 0) {
+            return buffered; // doesn't include those in.available(), but mostly used as a block test
         }
+        int avail = inReadFully ? 0 : in.available();
+        return avail;
+    }
+
+    void capRemaining(int newRemaining) {
+        capRemaining = Math.max(0, newRemaining);
+    }
+
+    void setMark() {
         bufMark = bufPos;
     }
 
-    @SuppressWarnings("NonSynchronizedMethodOverridesSynchronizedMethod") // explicitly not synced
-    @Override
-    public void reset() throws IOException {
+    void rewindToMark() throws IOException {
         if (bufMark < 0)
             throw new IOException("Resetting to invalid mark");
         bufPos = bufMark;
     }
 
+    void clearMark() {
+        bufMark = -1;
+    }
+
+    private void compact() {
+        if (byteBuf == null || bufPos == 0) return;
+        int keepFrom = bufMark >= 0 ? bufMark : bufPos;
+        if (keepFrom <= 0) return;
+
+        int remaining = bufLength - keepFrom;
+        if (remaining > 0) {
+            System.arraycopy(byteBuf, keepFrom, byteBuf, 0, remaining);
+        }
+        bufLength = remaining;
+        bufPos -= keepFrom;
+        if (bufMark >= 0) {
+            bufMark -= keepFrom;
+        }
+    }
+
     @Override
     public void close() throws IOException {
         if (in != null) super.close();

src/test/java/org/jsoup/helper/DataUtilTest.java

@@ -359,4 +359,63 @@ void handlesUnlimitedRead() throws IOException {
         Document doc = Jsoup.parse(ParseTest.getPath("/fuzztests/2353.html.gz"));
         assertTrue(doc.html().contains("Read-Fully!"));
     }
+
+    @Test
+    void charsetSniffingCanReuseTruncatedPreParse() throws IOException {
+        // #2448: when available() reports buffered bytes after the first read, the sniffed pre-parse may be reused while capped, leading to truncation
+
+        StringBuilder sb = new StringBuilder();
+        sb.append("<!doctype html><html><head><title>t</title></head><body><pre>");
+        while (sb.length() < 6200) {
+            sb.append("0123456789 abcdefghijklmnopqrstuvwxyz\n");
+        }
+        sb.append("</pre><main>list</main><hr></body></html>");
+        String html = sb.toString();
+
+
+        byte[] bytes = html.getBytes(StandardCharsets.UTF_8);
+        ControllableInputStream in = ControllableInputStream.wrap(new BufferedOnceAvailableStream(bytes), 0);
+
+        DataUtil.CharsetDoc charsetDoc = DataUtil.detectCharset(in, null, "http://example.com/", Parser.htmlParser());
+        Document doc = DataUtil.parseInputStream(charsetDoc, "http://example.com/", Parser.htmlParser());
+
+        assertNotNull(doc.selectFirst("hr"), "hr should survive the sniff + full parse");
+    }
+
+    // delivers all bytes in the first read, then signals available()>0 once to trigger a second read and baseReadFully=true
+    static final class BufferedOnceAvailableStream extends InputStream {
+        private final byte[] data;
+        private int pos = 0;
+        private boolean extraSignal = true;
+
+        BufferedOnceAvailableStream(byte[] data) {
+            this.data = data;
+        }
+
+        @Override
+        public int read(byte[] b, int off, int len) {
+            if (pos >= data.length) return -1;
+            int take = Math.min(len, data.length - pos);
+            System.arraycopy(data, pos, b, off, take);
+            pos += take;
+            return take;
+        }
+
+        @Override
+        public int read() {
+            return pos < data.length ? (data[pos++] & 0xff) : -1;
+        }
+
+        @Override
+        public int available() {
+            if (pos < data.length)
+                return data.length - pos;
+            if (extraSignal) {
+                extraSignal = false;
+                return 1; // nudge SimpleBufferedInput.fill() to try another read
+            }
+            return 0;
+        }
+    }
+
 }