Added IOException throwing readFully and readBody (#2327)

For historical reasons (specifically that a response's body used to be fully buffered during `execute`), `Connection#body()` does not throw a checked IOException.

This change adds a new `Connection#readBody()` which does throw a checked exception, and a partner `Connection#readFully()` method to do the internal buffering.

Clarified relevant documentation.

Author: jhy

CHANGES.md

@@ -12,6 +12,7 @@
 * Custom tags (defined via the `TagSet`) in a foreign namespace (e.g. SVG) can be configured to parse as data tags.
 * Added `NodeVisitor#traverse(Node)` to simplify node traversal calls (vs. importing `NodeTraversor`).
 * The HTML parser now allows the specific text-data type (Data, RcData) to be customized for known tags. (Previously, that was only supported on custom tags.) [#2326](https://github.com/jhy/jsoup/issues/2326).
+* Added `Connection#readFully()` as a replacement for `Connection#bufferUp()` with an explicit IOException. Similarly, added `Connection#readBody()` over `Connection#body()`. Deprecated `Connection#bufferUp()`. [#2327](https://github.com/jhy/jsoup/pull/2327) 
 
 ### Bug Fixes
 * The contents of a  `script` in a `svg` foreign context should be parsed as script data, not text. [#2320](https://github.com/jhy/jsoup/issues/2320)

src/main/java/org/jsoup/Connection.java

@@ -907,42 +907,87 @@ interface Response extends Base<Response> {
         @Nullable String contentType();
 
         /**
-         * Read and parse the body of the response as a Document. If you intend to parse the same response multiple
-         * times, you should {@link #bufferUp()} first.
-         * @return a parsed Document
-         * @throws IOException on error
+         Read and parse the body of the response as a Document. If you intend to parse the same response multiple times,
+         you should {@link #readFully()} first, which will buffer the body into memory.
+
+         @return a parsed Document
+         @throws IOException if an IO exception occurs whilst reading the body.
+         @see #readFully()
          */
         Document parse() throws IOException;
 
         /**
-         * Get the body of the response as a plain string.
-         * @return body
+         Read the response body, and returns it as a plain String.
+
+         @return body
+         @throws IOException if an IO exception occurs whilst reading the body.
+         @since 1.21.1
+         */
+        default String readBody() throws IOException {
+            throw new UnsupportedOperationException();
+        }
+
+        /**
+         Get the body of the response as a plain String.
+
+         <p>Will throw an UncheckedIOException if the body has not been buffered and an error occurs whilst reading the
+         body; use {@link #readFully()} first to buffer the body and catch any exceptions explicitly. Or more simply,
+         {@link #readBody()}.</p>
+
+         @return body
+         @throws UncheckedIOException if an IO exception occurs whilst reading the body.
+         @see #readBody()
+         @see #readFully()
          */
         String body();
 
         /**
-         * Get the body of the response as an array of bytes.
-         * @return body bytes
+         Get the body of the response as an array of bytes.
+
+         <p>Will throw an UncheckedIOException if the body has not been buffered and an error occurs whilst reading the
+         body; use {@link #readFully()} first to buffer the body and catch any exceptions explicitly.</p>
+
+         @return body bytes
+         @throws UncheckedIOException if an IO exception occurs whilst reading the body.
+         @see #readFully()
          */
         byte[] bodyAsBytes();
 
+        /**
+         Read the body of the response into a local buffer, so that {@link #parse()} may be called repeatedly on the same
+         connection response. Otherwise, once the response is read, its InputStream will have been drained and may not be
+         re-read.
+
+         <p>Subsequent calls methods than consume the body, such as {@link #parse()}, {@link #body()},
+         {@link #bodyAsBytes()}, will not need to read the body again, and will not throw exceptions.</p>
+         <p>Calling {@link #readBody()}} has the same effect.</p>
+
+         @return this response, for chaining
+         @throws IOException if an IO exception occurs during buffering.
+         @since 1.21.1
+         */
+        default Response readFully() throws IOException {
+            throw new UnsupportedOperationException();
+        }
+
         /**
          * Read the body of the response into a local buffer, so that {@link #parse()} may be called repeatedly on the
          * same connection response. Otherwise, once the response is read, its InputStream will have been drained and
          * may not be re-read.
          * <p>Calling {@link #body() } or {@link #bodyAsBytes()} has the same effect.</p>
          * @return this response, for chaining
          * @throws UncheckedIOException if an IO exception occurs during buffering.
+         * @deprecated use {@link #readFully()} instead (for the checked exception). Will be removed in a future version.
          */
         Response bufferUp();
 
         /**
          Get the body of the response as a (buffered) InputStream. You should close the input stream when you're done
          with it.
-         <p>Other body methods (like bufferUp, body, parse, etc) will generally not work in conjunction with this method,
+         <p>Other body methods (like readFully, body, parse, etc) will generally not work in conjunction with this method,
          as it consumes the InputStream.</p>
          <p>Any configured max size or maximum read timeout applied to the connection will not be applied to this stream,
-         unless {@link #bufferUp()} is called prior.</p>
+         unless {@link #readFully()} is called prior.</p>
          <p>This method is useful for writing large responses to disk, without buffering them completely into memory
          first.</p>
          @return the response body input stream

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

@@ -1006,24 +1006,45 @@ private ControllableInputStream prepareParse() {
             return streamer;
         }
 
-        private void prepareByteData() {
+        /**
+         Reads the bodyStream into byteData. A no-op if already executed.
+         */
+        @Override
+        public Connection.Response readFully() throws IOException {
             Validate.isTrue(executed, "Request must be executed (with .execute(), .get(), or .post() before getting response body");
             if (bodyStream != null && byteData == null) {
                 Validate.isFalse(inputStreamRead, "Request has already been read (with .parse())");
                 try {
                     byteData = DataUtil.readToByteBuffer(bodyStream, req.maxBodySize());
-                } catch (IOException e) {
-                    throw new UncheckedIOException(e);
                 } finally {
                     inputStreamRead = true;
                     safeClose();
                 }
             }
+            return this;
+        }
+
+        /**
+         Reads the body, but throws an UncheckedIOException if an IOException occurs.
+         @throws UncheckedIOException if an IOException occurs
+         */
+        private void readByteDataUnchecked() {
+            try {
+                readFully();
+            } catch (IOException e) {
+                throw new UncheckedIOException(e);
+            }
+        }
+
+        @Override
+        public String readBody() throws IOException {
+            readFully();
+            return body();
         }
 
         @Override
         public String body() {
-            prepareByteData();
+            readByteDataUnchecked();
             Validate.notNull(byteData);
             // charset gets set from header on execute, and from meta-equiv on parse. parse may not have happened yet
             String body = (charset == null ? UTF_8 : Charset.forName(charset))
@@ -1034,7 +1055,7 @@ public String body() {
 
         @Override
         public byte[] bodyAsBytes() {
-            prepareByteData();
+            readByteDataUnchecked();
             Validate.notNull(byteData);
             Validate.isTrue(byteData.hasArray()); // we made it, so it should
 
@@ -1053,15 +1074,15 @@ public byte[] bodyAsBytes() {
 
         @Override
         public Connection.Response bufferUp() {
-            prepareByteData();
+            readByteDataUnchecked();
             return this;
         }
 
         @Override
         public BufferedInputStream bodyStream() {
             Validate.isTrue(executed, "Request must be executed (with .execute(), .get(), or .post() before getting response body");
 
-            // if we have read to bytes (via buffer up), return those as a stream.
+            // if we have read to bytes (via readFully), return those as a stream.
             if (byteData != null) {
                 return new BufferedInputStream(
                     new ByteArrayInputStream(byteData.array(), 0, byteData.limit()),

src/main/java/org/jsoup/parser/CharacterReader.java

@@ -16,6 +16,8 @@
 
 /**
  CharacterReader consumes tokens off a string. Used internally by jsoup. API subject to changes.
+ <p>If the underlying reader throws an IOException during any operation, the CharacterReader will throw an
+ {@link UncheckedIOException}. That won't happen with String / StringReader inputs.</p>
  */
 public final class CharacterReader implements AutoCloseable {
     static final char EOF = (char) -1;
@@ -81,6 +83,10 @@ private void bufferUp() {
         doBufferUp(); // structured so bufferUp may become an intrinsic candidate
     }
 
+    /**
+     Reads into the buffer. Will throw an UncheckedIOException if the underling reader throws an IOException.
+     @throws UncheckedIOException if the underlying reader throws an IOException
+     */
     private void doBufferUp() {
         /*
         The flow:

src/main/java/org/jsoup/parser/Parser.java

@@ -61,11 +61,26 @@ private Parser(Parser copy) {
         settings = new ParseSettings(copy.settings);
         trackPosition = copy.trackPosition;
     }
-    
+
+    /**
+     Parse the contents of a String.
+
+     @param html HTML to parse
+     @param baseUri base URI of document (i.e. original fetch location), for resolving relative URLs.
+     @return parsed Document
+     */
     public Document parseInput(String html, String baseUri) {
         return parseInput(new StringReader(html), baseUri);
     }
 
+    /**
+     Parse the contents of Reader.
+
+     @param inputHtml HTML to parse
+     @param baseUri base URI of document (i.e. original fetch location), for resolving relative URLs.
+     @return parsed Document
+     @throws java.io.UncheckedIOException if an I/O error occurs in the Reader
+     */
     public Document parseInput(Reader inputHtml, String baseUri) {
         try {
             lock.lock(); // using a lock vs synchronized to support loom threads
@@ -75,10 +90,27 @@ public Document parseInput(Reader inputHtml, String baseUri) {
         }
     }
 
+    /**
+     Parse a fragment of HTML into a list of nodes. The context element, if supplied, supplies parsing context.
+
+     @param fragment the fragment of HTML to parse
+     @param context (optional) the element that this HTML fragment is being parsed for (i.e. for inner HTML).
+     @param baseUri base URI of document (i.e. original fetch location), for resolving relative URLs.
+     @return list of nodes parsed from the input HTML.
+     */
     public List<Node> parseFragmentInput(String fragment, @Nullable Element context, String baseUri) {
         return parseFragmentInput(new StringReader(fragment), context, baseUri);
     }
 
+    /**
+     Parse a fragment of HTML into a list of nodes. The context element, if supplied, supplies parsing context.
+
+     @param fragment the fragment of HTML to parse
+     @param context (optional) the element that this HTML fragment is being parsed for (i.e. for inner HTML).
+     @param baseUri base URI of document (i.e. original fetch location), for resolving relative URLs.
+     @return list of nodes parsed from the input HTML.
+     @throws java.io.UncheckedIOException if an I/O error occurs in the Reader
+     */
     public List<Node> parseFragmentInput(Reader fragment, @Nullable Element context, String baseUri) {
         try {
             lock.lock();

src/main/java/org/jsoup/parser/TokeniserState.java

@@ -1659,7 +1659,7 @@ else if (r.matches('>')) {
     abstract void read(Tokeniser t, CharacterReader r);
 
     static final char nullChar = '\u0000';
-    // char searches. must be sorted, used in inSorted. MUST update TokenisetStateTest if more arrays are added.
+    // char searches. must be sorted, used in inSorted. MUST update TokeniserStateTest if more arrays are added.
     static final char[] attributeNameCharsSorted = new char[]{'\t', '\n', '\f', '\r', ' ', '"', '\'', '/', '<', '=', '>', '?'};
     static final char[] attributeValueUnquoted = new char[]{nullChar, '\t', '\n', '\f', '\r', ' ', '"', '&', '\'', '<', '=', '>', '`'};
 

src/test/java/org/jsoup/integration/ConnectIT.java

@@ -129,6 +129,42 @@ public void slowReadOk() throws IOException {
         assertEquals("outatime", h1.text());
     }
 
+    @Test void readFullyThrowsOnTimeout() throws IOException {
+        // tests that response.readFully excepts on timeout
+        boolean caught = false;
+        Connection.Response res = Jsoup.connect(SlowRider.Url).timeout(3000).execute();
+        try {
+            res.readFully();
+        } catch (IOException e) {
+            caught = true;
+        }
+        assertTrue(caught);
+    }
+
+    @Test void readBodyThrowsOnTimeout() throws IOException {
+        // tests that response.readBody excepts on timeout
+        boolean caught = false;
+        Connection.Response res = Jsoup.connect(SlowRider.Url).timeout(3000).execute();
+        try {
+            res.readBody();
+        } catch (IOException e) {
+            caught = true;
+        }
+        assertTrue(caught);
+    }
+
+    @Test void bodyThrowsUncheckedOnTimeout() throws IOException {
+        // tests that response.body unchecked excepts on timeout
+        boolean caught = false;
+        Connection.Response res = Jsoup.connect(SlowRider.Url).timeout(3000).execute();
+        try {
+            res.body();
+        } catch (UncheckedIOException e) {
+            caught = true;
+        }
+        assertTrue(caught);
+    }
+
     @Test
     public void infiniteReadSupported() throws IOException {
         Document doc = Jsoup.connect(SlowRider.Url)
@@ -249,6 +285,21 @@ public void noLimitAfterFirstRead() throws IOException {
         }
     }
 
+    @Test public void bodyStreamConstrainedViaReadFully() throws IOException {
+        int cap = 5 * 1024;
+        String url = FileServlet.urlTo("/htmltests/large.html"); // 280 K
+        try (BufferedInputStream stream = Jsoup
+            .connect(url)
+            .maxBodySize(cap)
+            .execute()
+            .readFully()
+            .bodyStream()) {
+
+            ByteBuffer cappedRead = DataUtil.readToByteBuffer(stream, 0);
+            assertEquals(cap, cappedRead.limit());
+        }
+    }
+
     @Test public void bodyStreamConstrainedViaBufferUp() throws IOException {
         int cap = 5 * 1024;
         String url = FileServlet.urlTo("/htmltests/large.html"); // 280 K

src/test/java/org/jsoup/integration/ConnectTest.java

@@ -393,6 +393,17 @@ public void postFiles(String url) throws IOException {
          */
     }
 
+    @Test
+    public void multipleParsesOkAfterReadFully() throws IOException {
+        Connection.Response res = Jsoup.connect(echoUrl).execute().readFully();
+
+        Document doc = res.parse();
+        assertTrue(doc.title().contains("Environment"));
+
+        Document doc2 = res.parse();
+        assertTrue(doc2.title().contains("Environment"));
+    }
+
     @Test
     public void multipleParsesOkAfterBufferUp() throws IOException {
         Connection.Response res = Jsoup.connect(echoUrl).execute().bufferUp();
@@ -842,6 +853,12 @@ public void maxBodySizeInReadToByteBuffer() throws IOException {
         assertEquals(200 * 1024, mediumRes.body().length());
         assertEquals(actualDocText, largeRes.body().length());
         assertEquals(actualDocText, unlimitedRes.body().length());
+
+        assertEquals(actualDocText, defaultRes.readBody().length());
+        assertEquals(50 * 1024, smallRes.readBody().length());
+        assertEquals(200 * 1024, mediumRes.readBody().length());
+        assertEquals(actualDocText, largeRes.readBody().length());
+        assertEquals(actualDocText, unlimitedRes.readBody().length());
     }
 
     @Test void formLoginFlow() throws IOException {