[
https://issues.apache.org/jira/browse/SLING-13268?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Radu Cotescu updated SLING-13268:
---------------------------------
Component/s: GraphQL
> 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
> Components: GraphQL
> 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)
