This is an automated email from the ASF dual-hosted git repository.
raducotescu pushed a commit to branch master
in repository
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-graphql-schema-aggregator.git
The following commit(s) were added to refs/heads/master by this push:
new 3a2903a SLING-13268 - PartialReader section slicing breaks when
Reader.skip() returns 0
3a2903a is described below
commit 3a2903a3992cc164eb0fd957589a999c147b6a4a
Author: Rishabh Kumar <[email protected]>
AuthorDate: Tue Jul 28 16:04:51 2026 +0530
SLING-13268 - PartialReader section slicing breaks when Reader.skip()
returns 0
* skip robustly to section start instead of a single Reader.skip() call
* stop depending on commons-io's BoundedReader for section bounds
* added a test that shows the reader returns nothing even when asked to skip
beyond EOF
* use int for section offsets and sizes in PartialReader to avoid
unnecessary
long/int casts; buffer skipFully for performance
---------
Co-authored-by: Rishabh Kumar <[email protected]>
Co-authored-by: Radu Cotescu <[email protected]>
---
.../schema/aggregator/impl/PartialReader.java | 85 +++++++++++++++++++++-
.../schema/aggregator/impl/PartialReaderTest.java | 54 ++++++++++++++
2 files changed, 136 insertions(+), 3 deletions(-)
diff --git
a/src/main/java/org/apache/sling/graphql/schema/aggregator/impl/PartialReader.java
b/src/main/java/org/apache/sling/graphql/schema/aggregator/impl/PartialReader.java
index 7ede0e5..7fad525 100644
---
a/src/main/java/org/apache/sling/graphql/schema/aggregator/impl/PartialReader.java
+++
b/src/main/java/org/apache/sling/graphql/schema/aggregator/impl/PartialReader.java
@@ -35,7 +35,6 @@ import java.util.regex.Pattern;
import org.apache.commons.codec.binary.Hex;
import org.apache.commons.codec.digest.DigestUtils;
import org.apache.commons.io.IOUtils;
-import org.apache.commons.io.input.BoundedReader;
import org.jetbrains.annotations.NotNull;
/** Reader for the partials format, which parses a partial file and
@@ -89,8 +88,88 @@ public class PartialReader implements Partial {
@Override
public Reader getContent() throws IOException {
final Reader r = sectionSource.get();
- r.skip(startCharIndex);
- return new BoundedReader(r, endCharIndex - startCharIndex);
+ skipFully(r, startCharIndex);
+ return new BoundedContentReader(r, endCharIndex - startCharIndex);
+ }
+
+ /**
+ * Skips up to {@code count} characters from {@code r}.
+ *
+ * This uses Reader.skip() repeatedly. If skip() makes no progress the
method falls back
+ * to reading into a temporary buffer (up to 8 KiB) to advance in bulk
instead of
+ * degrading to single-character reads. That avoids very slow behavior
when skip()
+ * consistently returns 0.
+ *
+ * If EOF is reached before the requested number of characters is
skipped the method
+ * returns normally after consuming available input; it does not
throw. Callers that
+ * require a strict guarantee that the requested start exists should
validate the source
+ * or check the reader state after this call.
+ */
+ private static void skipFully(Reader r, int count) throws IOException {
+ int remaining = count;
+ // start with a buffer sized to the remaining amount but never
larger than 8 KiB
+ char[] buf = new char[Math.max(1, Math.min(8192, remaining))];
+ while (remaining > 0) {
+ final long skipped = r.skip(remaining);
+ if (skipped > 0) {
+ remaining -= (int) skipped;
+ // shrink buffer if the remaining amount is smaller than
current buffer
+ if (remaining > 0 && buf.length > remaining) {
+ buf = new char[Math.min(8192, remaining)];
+ }
+ } else {
+ final int toRead = Math.min(buf.length, remaining);
+ final int n = r.read(buf, 0, toRead);
+ if (n == -1) {
+ // EOF reached before skipping everything - stop
+ break;
+ }
+ remaining -= n;
+ }
+ }
+ }
+ }
+
+ /** Bounds reads to at most {@code maxChars} characters.
+ * commons-io's BoundedReader stopped enforcing this bound on its
read(char[]) overload
+ * in 2.22.0 (only read() and read(char[],int,int) got the fix) -
IOUtils.copy() reads
+ * through exactly that overload, so a section's content would run
straight into the
+ * next one. Extending Reader directly, instead of commons-io's
ProxyReader, means the
+ * JDK's own default read()/read(char[]) delegate to read(char[],int,int)
below, so
+ * every overload stays bounded no matter which commons-io version is on
the classpath.
+ *
+ * Note: when the underlying reader reaches EOF, reads behave normally
and return -1.
+ * This class does not attempt to recover or throw when the section's
start offset was
+ * beyond EOF; callers that need that guarantee should validate the
source beforehand.
+ * For correctness and performance, this class explicitly implements
read(char[],int,int)
+ * so JDK and commons-io bulk read paths stay bounded; read() and
read(char[]) will
+ * delegate to that implementation.
+ */
+ private static final class BoundedContentReader extends Reader {
+ private final Reader target;
+ private int remaining;
+
+ BoundedContentReader(Reader target, int maxChars) {
+ this.target = target;
+ this.remaining = maxChars;
+ }
+
+ @Override
+ public int read(char[] cbuf, int off, int len) throws IOException {
+ if (remaining <= 0) {
+ return -1;
+ }
+ final int toRead = Math.min(len, remaining);
+ final int n = target.read(cbuf, off, toRead);
+ if (n > 0) {
+ remaining -= n;
+ }
+ return n;
+ }
+
+ @Override
+ public void close() throws IOException {
+ target.close();
}
}
diff --git
a/src/test/java/org/apache/sling/graphql/schema/aggregator/impl/PartialReaderTest.java
b/src/test/java/org/apache/sling/graphql/schema/aggregator/impl/PartialReaderTest.java
index 110ac0e..004a9c5 100644
---
a/src/test/java/org/apache/sling/graphql/schema/aggregator/impl/PartialReaderTest.java
+++
b/src/test/java/org/apache/sling/graphql/schema/aggregator/impl/PartialReaderTest.java
@@ -18,6 +18,7 @@
*/
package org.apache.sling.graphql.schema.aggregator.impl;
+import java.io.FilterReader;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
@@ -76,6 +77,20 @@ public class PartialReaderTest {
return () -> new StringReader(content);
}
+ /** Reader wrapper whose skip() always returns 0, as the Reader.skip()
contract
+ * allows, forcing callers to fall back to read()-based skipping.
+ */
+ private static class ZeroSkipReader extends FilterReader {
+ ZeroSkipReader(Reader in) {
+ super(in);
+ }
+
+ @Override
+ public long skip(long n) throws IOException {
+ return 0;
+ }
+ }
+
@Test
public void parseExample() throws Exception {
final PartialReader p = new PartialReader(
@@ -187,4 +202,43 @@ public class PartialReaderTest {
"SHA-256:
703bd06e9d65118c75abe9a7a06f6a2fcdb8a19ef62d994f4cc1be0b34420383",
p.getDigest());
}
+
+ @Test
+ public void sectionContentSkipsRobustlyWhenReaderSkipReturnsZero() throws
IOException {
+ // Exercise PartialReader.ParsedSection directly: PartialReader's own
line-ending
+ // normalization always hands ParsedSection a plain StringReader,
which happens to
+ // fully honor skip() in one call and would hide this bug.
+ final String content = "0123456789ABCDEF";
+ final Supplier<Reader> zeroSkipSource = () -> new ZeroSkipReader(new
StringReader(content));
+ final Partial.Section section =
+ new PartialReader.ParsedSection(zeroSkipSource,
SectionName.TYPES, "desc", 5, 10);
+ try (Reader r = section.getContent()) {
+ assertEquals("56789", IOUtils.toString(r));
+ }
+ }
+
+ @Test
+ public void sectionContentStartBeyondEOFReturnsEmpty() throws IOException {
+ // If the requested start index is beyond the source's length,
getContent() should
+ // return an empty reader and not throw.
+ final String content = "0123"; // only 4 chars
+ final Supplier<Reader> source = () -> new StringReader(content);
+ final Partial.Section section = new
PartialReader.ParsedSection(source, SectionName.TYPES, "desc", 10, 12);
+ try (Reader r = section.getContent()) {
+ assertEquals("", IOUtils.toString(r));
+ }
+ }
+
+ @Test
+ public void sectionContentStaysBoundedWhenCopiedViaBulkReadCharArray()
throws IOException {
+ // IOUtils.copy() reads through read(char[]) - the exact overload
commons-io's
+ // BoundedReader stopped bounding in 2.22.0. Section is much shorter
than its source,
+ // so this must still stop at the true end no matter which commons-io
version is loaded.
+ final String content = "0123456789ABCDEFGHIJ";
+ final Supplier<Reader> source = () -> new StringReader(content);
+ final Partial.Section section = new
PartialReader.ParsedSection(source, SectionName.TYPES, "desc", 5, 10);
+ try (Reader r = section.getContent()) {
+ assertEquals("56789", IOUtils.toString(r));
+ }
+ }
}
