← 输入框|github.com|刷新
Skip to content

count good faith introspection fields on repeated fragment spreads - #4515

Open
nab-iha3 wants to merge 1 commit into
graphql-java:masterfrom
nab-iha3:good-faith-repeated-fragment-spreads
Open

nab-iha3 wants to merge 1 commit into
graphql-java:masterfrom
nab-iha3:good-faith-repeated-fragment-spreads

Conversation

@nab-iha3

Copy link
Copy Markdown
Contributor

a named fragment that is spread more than once gets around the good faith cap on __Type.fields / inputFields / interfaces / possibleTypes:

query {
  __schema { a1: types { ...T } a2: types { ...T } ... a120: types { ...T } }
}
fragment T on __Type { fields { type { name } } }

# schema with 300 types x 20 fields
inline, 2 selections:                 BadFaithIntrospectionError: __Type.fields is present too often!
fragment, 120 spreads (2.5 KB query): no error, 14.6 MB response, 5.8 s
fragment, 120 spreads, patched:       BadFaithIntrospectionError: __Type.fields is present too often!

OperationValidator.checkFragmentSpread only traverses a fragment on its first spread in an operation. later spreads add the cached FragmentComplexityInfo field count and depth, so the 500 field / 20 depth limits still hold (120 spreads x 4 fields stays under 500), but checkGoodFaithIntrospection never sees the fields inside them, and every re-spread repeats the full types x fields walk without being counted.

FragmentComplexityInfo now also keeps the good faith keys counted during the fragment's first traversal (nested fragments included), and a later spread counts them again, so it behaves the same as writing the selection inline. fragments without cycle forming fields, like TypeRef and InputValue in the standard introspection query, record nothing and are unaffected. this is the cached path that #4490 did not cover; that change handled __schema reached through a fragment's first traversal.

@github-actions

Copy link
Copy Markdown
Contributor

Test Report

Test Results

Java Version Total Passed Failed Errors Skipped
Java 11 6054 (+3 🟢) 5998 (+3 🟢) 0 (±0) 0 (±0) 56 (±0)
Java 17 6054 (+3 🟢) 5997 (+3 🟢) 0 (±0) 0 (±0) 57 (±0)
Java 21 6054 (+3 🟢) 5997 (+3 🟢) 0 (±0) 0 (±0) 57 (±0)
Java 25 6054 (+3 🟢) 5997 (+3 🟢) 0 (±0) 0 (±0) 57 (±0)
jcstress 32 (±0) 32 (±0) 0 (±0) 0 (±0) 0 (±0)
Total 24248 (+12 🟢) 24021 (+12 🟢) 0 (±0) 0 (±0) 227 (±0)

Code Coverage (Java 25)

Metric Covered Missed Coverage vs Master
Lines 30216 3087 90.7% ±0.0%
Branches 8864 1510 85.4% ±0.0%
Methods 8073 1188 87.2% ±0.0%

Changed Class Coverage (3 classes)

Class Line Branch Method
g.e.ExecutionStrategy ±0.0% -0.8% 🔴 ±0.0%
g.v.FragmentComplexityInfo +3.2% 🟢 ±0.0% +5.0% 🟢
g.v.OperationValidator -0.2% 🔴 ±0.0% ±0.0%
ExecutionStrategy — method details
Method Line Branch
lambda$buildFieldValueMap$0 100.0% 90.0% (-10.0% 🔴)
OperationValidator — method details
Method Line Branch
checkGoodFaithIntrospection 100.0% 96.2% (-0.3% 🔴)
countIntrospectionField new 100.0% 100.0%
introspectionFieldKeysAddedSince new 100.0% 100.0%
checkFragmentSpread 95.1% (-4.9% 🔴) 100.0%

Full HTML report: build artifact jacoco-html-report

Updated: 2026-10-27 16:11:51 UTC

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant