[
https://issues.apache.org/jira/browse/SLING-13268?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Rishabh Daim updated SLING-13268:
---------------------------------
Description:
h3. Summary
{{PartialReader}} builds a section's content by wrapping a reader with
commons-io's {{BoundedReader}} to stop at the section's end. Two independent
bugs in this path let one section's content run into the next one, leaking raw
{{QUERY:}}/{{TYPES:}} header lines into the aggregated SDL and breaking GraphQL
parsing downstream.
h3. Root causes
# *Unchecked {{skip()}} call* - the code called {{reader.skip(startCharIndex)}}
once and assumed it always fully advances. {{Reader.skip()}} is allowed to skip
fewer characters than requested per call.
# *commons-io {{BoundedReader}} regression (2.22.0+)* - confirmed by
reproduction. {{BoundedReader}} was refactored to extend {{ProxyReader}} in
2.22.0. It bounds {{read()}} and {{read(char[],int,int)}}, but *not*
{{read(char\[\])}} - that overload falls through to
{{ProxyReader.read(char[])}}, which delegates straight to the wrapped reader
with no bound check at all. {{IOUtils.copy(Reader, Writer)}}, used to copy each
section's content, reads through exactly that unbounded overload. 2.21.0 and
earlier didn't have this problem since {{BoundedReader}} extended
{{java.io.Reader}} directly, whose default {{read()}}/{{read(char[])}}
correctly delegate to the bounded 3-arg method.
This is why the aggregator worked fine through commons-io 2.21.0 and broke as
soon as a runtime picked up 2.22.0 - this project depends on commons-io as
{{provided}}, so the actual version is whatever the OSGi container resolves,
not what's pinned in this repo's pom.
h3. Impact
Reproduced against real production schema partials: with commons-io 2.22.0 on
the classpath, a {{TYPES:}} header line and the following section's content
leak into the previous section, and {{graphql-java}}'s {{SchemaParser}} fails
with {{SchemaProblem: token recognition error at: '--'}} - matching the failure
seen in {{GraphQLContentFragmentListIT}}.
h3. Fix
* Added {{skipFully()}}, looping (falling back to {{read()}}) until the
requested offset is reached or EOF.
* Replaced commons-io's {{BoundedReader}} with a small self-contained
{{BoundedContentReader}} that extends {{java.io.Reader}} directly and only
overrides {{read(char[],int,int)}} - the JDK's own default
{{read()}}/{{read(char[])}} delegate to it, so every overload stays bounded
regardless of which commons-io version ends up on the runtime classpath.
Verified byte-for-byte against the real production schema files with commons-io
2.22.0 pinned: output now matches the known-good (2.21.0) baseline exactly and
parses successfully.
was:
h3. Summary
PartialReader assumes a single Reader.skip(startCharIndex) fully advances to
section start.
With commons-io 2.22.0 behavior exposure, section boundaries can shift.
h3. Impact
Schema aggregation may leak raw section headers (QUERY:/TYPES: separators) into
SDL and break GraphQL parsing.
h3. Fix
Skip robustly until the requested offset is reached (or EOF), instead of
relying on one skip() call.
h3. Reproducer
Unit test with a Reader whose skip() returns 0 reproduces the bug.
> PartialReader section slicing breaks when Reader.skip() returns 0
> -----------------------------------------------------------------
>
> Key: SLING-13268
> URL: https://issues.apache.org/jira/browse/SLING-13268
> Project: Sling
> Issue Type: Bug
> Reporter: Rishabh Daim
> Assignee: Rishabh Daim
> Priority: Major
>
> h3. Summary
> {{PartialReader}} builds a section's content by wrapping a reader with
> commons-io's {{BoundedReader}} to stop at the section's end. Two independent
> bugs in this path let one section's content run into the next one, leaking
> raw {{QUERY:}}/{{TYPES:}} header lines into the aggregated SDL and breaking
> GraphQL parsing downstream.
> h3. Root causes
> # *Unchecked {{skip()}} call* - the code called
> {{reader.skip(startCharIndex)}} once and assumed it always fully advances.
> {{Reader.skip()}} is allowed to skip fewer characters than requested per call.
> # *commons-io {{BoundedReader}} regression (2.22.0+)* - confirmed by
> reproduction. {{BoundedReader}} was refactored to extend {{ProxyReader}} in
> 2.22.0. It bounds {{read()}} and {{read(char[],int,int)}}, but *not*
> {{read(char\[\])}} - that overload falls through to
> {{ProxyReader.read(char[])}}, which delegates straight to the wrapped reader
> with no bound check at all. {{IOUtils.copy(Reader, Writer)}}, used to copy
> each section's content, reads through exactly that unbounded overload. 2.21.0
> and earlier didn't have this problem since {{BoundedReader}} extended
> {{java.io.Reader}} directly, whose default {{read()}}/{{read(char[])}}
> correctly delegate to the bounded 3-arg method.
> This is why the aggregator worked fine through commons-io 2.21.0 and broke as
> soon as a runtime picked up 2.22.0 - this project depends on commons-io as
> {{provided}}, so the actual version is whatever the OSGi container resolves,
> not what's pinned in this repo's pom.
> h3. Impact
> Reproduced against real production schema partials: with commons-io 2.22.0 on
> the classpath, a {{TYPES:}} header line and the following section's content
> leak into the previous section, and {{graphql-java}}'s {{SchemaParser}} fails
> with {{SchemaProblem: token recognition error at: '--'}} - matching the
> failure seen in {{GraphQLContentFragmentListIT}}.
> h3. Fix
> * Added {{skipFully()}}, looping (falling back to {{read()}}) until the
> requested offset is reached or EOF.
> * Replaced commons-io's {{BoundedReader}} with a small self-contained
> {{BoundedContentReader}} that extends {{java.io.Reader}} directly and only
> overrides {{read(char[],int,int)}} - the JDK's own default
> {{read()}}/{{read(char[])}} delegate to it, so every overload stays bounded
> regardless of which commons-io version ends up on the runtime classpath.
> Verified byte-for-byte against the real production schema files with
> commons-io 2.22.0 pinned: output now matches the known-good (2.21.0) baseline
> exactly and parses successfully.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
