Skip to content

Commit d2a1caa

Browse files
authored
Change RequireSlicingArgs default to false. (#7666)
1 parent 417ee0d commit d2a1caa

12 files changed

+229
-41
lines changed

src/HotChocolate/AspNetCore/test/AspNetCore.Tests/HttpGetSchemaMiddlewareTests.cs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,33 @@ public async Task Download_GraphQL_Schema(string path)
5757
response.MatchMarkdownSnapshot();
5858
}
5959

60+
[Theory]
61+
[InlineData("/graphql?sdl")]
62+
[InlineData("/graphql/schema/")]
63+
[InlineData("/graphql/schema.graphql")]
64+
[InlineData("/graphql/schema")]
65+
public async Task Download_GraphQL_Schema_Slicing_Args_Enabled(string path)
66+
{
67+
// arrange
68+
var server = CreateStarWarsServer(
69+
configureServices: sp =>
70+
sp
71+
.RemoveAll<ITimeProvider>()
72+
.AddSingleton<ITimeProvider, StaticTimeProvider>()
73+
.AddGraphQL()
74+
.ModifyPagingOptions(o => o.RequirePagingBoundaries = true));
75+
var url = TestServerExtensions.CreateUrl(path);
76+
var request = new HttpRequestMessage(HttpMethod.Get, url);
77+
78+
// act
79+
var response = await server.CreateClient().SendAsync(request);
80+
81+
// assert
82+
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
83+
84+
response.MatchMarkdownSnapshot();
85+
}
86+
6087
[Theory]
6188
[InlineData("/graphql/?sdl")]
6289
[InlineData("/graphql/schema/")]

src/HotChocolate/AspNetCore/test/AspNetCore.Tests/__snapshots__/HttpGetSchemaMiddlewareTests.Download_GraphQL_SDL.snap

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ type Droid implements Character {
1717
id: ID!
1818
name: String!
1919
appearsIn: [Episode]
20-
friends("Returns the first _n_ elements from the list." first: Int "Returns the elements in the list that come after the specified cursor." after: String "Returns the last _n_ elements from the list." last: Int "Returns the elements in the list that come before the specified cursor." before: String): FriendsConnection @listSize(assumedSize: 50, slicingArguments: [ "first", "last" ], sizedFields: [ "edges", "nodes" ])
20+
friends("Returns the first _n_ elements from the list." first: Int "Returns the elements in the list that come after the specified cursor." after: String "Returns the last _n_ elements from the list." last: Int "Returns the elements in the list that come before the specified cursor." before: String): FriendsConnection @listSize(assumedSize: 50, slicingArguments: [ "first", "last" ], sizedFields: [ "edges", "nodes" ], requireOneSlicingArgument: false)
2121
height(unit: Unit): Float
2222
primaryFunction: String
2323
traits: JSON
@@ -45,7 +45,7 @@ type Human implements Character {
4545
id: ID!
4646
name: String!
4747
appearsIn: [Episode]
48-
friends("Returns the first _n_ elements from the list." first: Int "Returns the elements in the list that come after the specified cursor." after: String "Returns the last _n_ elements from the list." last: Int "Returns the elements in the list that come before the specified cursor." before: String): FriendsConnection @listSize(assumedSize: 50, slicingArguments: [ "first", "last" ], sizedFields: [ "edges", "nodes" ])
48+
friends("Returns the first _n_ elements from the list." first: Int "Returns the elements in the list that come after the specified cursor." after: String "Returns the last _n_ elements from the list." last: Int "Returns the elements in the list that come before the specified cursor." before: String): FriendsConnection @listSize(assumedSize: 50, slicingArguments: [ "first", "last" ], sizedFields: [ "edges", "nodes" ], requireOneSlicingArgument: false)
4949
otherHuman: Human
5050
height(unit: Unit): Float
5151
homePlanet: String

src/HotChocolate/AspNetCore/test/AspNetCore.Tests/__snapshots__/HttpGetSchemaMiddlewareTests.Download_GraphQL_SDL_Explicit_Route.snap

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ type Droid implements Character {
1717
id: ID!
1818
name: String!
1919
appearsIn: [Episode]
20-
friends("Returns the first _n_ elements from the list." first: Int "Returns the elements in the list that come after the specified cursor." after: String "Returns the last _n_ elements from the list." last: Int "Returns the elements in the list that come before the specified cursor." before: String): FriendsConnection @listSize(assumedSize: 50, slicingArguments: [ "first", "last" ], sizedFields: [ "edges", "nodes" ])
20+
friends("Returns the first _n_ elements from the list." first: Int "Returns the elements in the list that come after the specified cursor." after: String "Returns the last _n_ elements from the list." last: Int "Returns the elements in the list that come before the specified cursor." before: String): FriendsConnection @listSize(assumedSize: 50, slicingArguments: [ "first", "last" ], sizedFields: [ "edges", "nodes" ], requireOneSlicingArgument: false)
2121
height(unit: Unit): Float
2222
primaryFunction: String
2323
traits: JSON
@@ -45,7 +45,7 @@ type Human implements Character {
4545
id: ID!
4646
name: String!
4747
appearsIn: [Episode]
48-
friends("Returns the first _n_ elements from the list." first: Int "Returns the elements in the list that come after the specified cursor." after: String "Returns the last _n_ elements from the list." last: Int "Returns the elements in the list that come before the specified cursor." before: String): FriendsConnection @listSize(assumedSize: 50, slicingArguments: [ "first", "last" ], sizedFields: [ "edges", "nodes" ])
48+
friends("Returns the first _n_ elements from the list." first: Int "Returns the elements in the list that come after the specified cursor." after: String "Returns the last _n_ elements from the list." last: Int "Returns the elements in the list that come before the specified cursor." before: String): FriendsConnection @listSize(assumedSize: 50, slicingArguments: [ "first", "last" ], sizedFields: [ "edges", "nodes" ], requireOneSlicingArgument: false)
4949
otherHuman: Human
5050
height(unit: Unit): Float
5151
homePlanet: String

src/HotChocolate/AspNetCore/test/AspNetCore.Tests/__snapshots__/HttpGetSchemaMiddlewareTests.Download_GraphQL_SDL_Explicit_Route_Explicit_Pattern.snap

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ type Droid implements Character {
1717
id: ID!
1818
name: String!
1919
appearsIn: [Episode]
20-
friends("Returns the first _n_ elements from the list." first: Int "Returns the elements in the list that come after the specified cursor." after: String "Returns the last _n_ elements from the list." last: Int "Returns the elements in the list that come before the specified cursor." before: String): FriendsConnection @listSize(assumedSize: 50, slicingArguments: [ "first", "last" ], sizedFields: [ "edges", "nodes" ])
20+
friends("Returns the first _n_ elements from the list." first: Int "Returns the elements in the list that come after the specified cursor." after: String "Returns the last _n_ elements from the list." last: Int "Returns the elements in the list that come before the specified cursor." before: String): FriendsConnection @listSize(assumedSize: 50, slicingArguments: [ "first", "last" ], sizedFields: [ "edges", "nodes" ], requireOneSlicingArgument: false)
2121
height(unit: Unit): Float
2222
primaryFunction: String
2323
traits: JSON
@@ -45,7 +45,7 @@ type Human implements Character {
4545
id: ID!
4646
name: String!
4747
appearsIn: [Episode]
48-
friends("Returns the first _n_ elements from the list." first: Int "Returns the elements in the list that come after the specified cursor." after: String "Returns the last _n_ elements from the list." last: Int "Returns the elements in the list that come before the specified cursor." before: String): FriendsConnection @listSize(assumedSize: 50, slicingArguments: [ "first", "last" ], sizedFields: [ "edges", "nodes" ])
48+
friends("Returns the first _n_ elements from the list." first: Int "Returns the elements in the list that come after the specified cursor." after: String "Returns the last _n_ elements from the list." last: Int "Returns the elements in the list that come before the specified cursor." before: String): FriendsConnection @listSize(assumedSize: 50, slicingArguments: [ "first", "last" ], sizedFields: [ "edges", "nodes" ], requireOneSlicingArgument: false)
4949
otherHuman: Human
5050
height(unit: Unit): Float
5151
homePlanet: String

src/HotChocolate/AspNetCore/test/AspNetCore.Tests/__snapshots__/HttpGetSchemaMiddlewareTests.Download_GraphQL_Schema.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@
22

33
```text
44
Headers:
5-
ETag: "1-B2t9cwf/BRF8NYIpFDtJE4DLg8FfD5d5HdAF9KObaUc="
5+
ETag: "1-LzuFZZrknIre5etJZHfNRC1e/vj+qW9tAf9pYpS8bQM="
66
Cache-Control: public, must-revalidate, max-age=3600
77
Content-Type: application/graphql; charset=utf-8
88
Content-Disposition: attachment; filename="schema.graphql"
99
Last-Modified: Fri, 01 Jan 2021 00:00:00 GMT
10-
Content-Length: 7193
10+
Content-Length: 7261
1111
-------------------------->
1212
Status Code: OK
1313
-------------------------->
@@ -30,7 +30,7 @@ type Droid implements Character {
3030
id: ID!
3131
name: String!
3232
appearsIn: [Episode]
33-
friends("Returns the first _n_ elements from the list." first: Int "Returns the elements in the list that come after the specified cursor." after: String "Returns the last _n_ elements from the list." last: Int "Returns the elements in the list that come before the specified cursor." before: String): FriendsConnection @listSize(assumedSize: 50, slicingArguments: [ "first", "last" ], sizedFields: [ "edges", "nodes" ])
33+
friends("Returns the first _n_ elements from the list." first: Int "Returns the elements in the list that come after the specified cursor." after: String "Returns the last _n_ elements from the list." last: Int "Returns the elements in the list that come before the specified cursor." before: String): FriendsConnection @listSize(assumedSize: 50, slicingArguments: [ "first", "last" ], sizedFields: [ "edges", "nodes" ], requireOneSlicingArgument: false)
3434
height(unit: Unit): Float
3535
primaryFunction: String
3636
traits: JSON
@@ -58,7 +58,7 @@ type Human implements Character {
5858
id: ID!
5959
name: String!
6060
appearsIn: [Episode]
61-
friends("Returns the first _n_ elements from the list." first: Int "Returns the elements in the list that come after the specified cursor." after: String "Returns the last _n_ elements from the list." last: Int "Returns the elements in the list that come before the specified cursor." before: String): FriendsConnection @listSize(assumedSize: 50, slicingArguments: [ "first", "last" ], sizedFields: [ "edges", "nodes" ])
61+
friends("Returns the first _n_ elements from the list." first: Int "Returns the elements in the list that come after the specified cursor." after: String "Returns the last _n_ elements from the list." last: Int "Returns the elements in the list that come before the specified cursor." before: String): FriendsConnection @listSize(assumedSize: 50, slicingArguments: [ "first", "last" ], sizedFields: [ "edges", "nodes" ], requireOneSlicingArgument: false)
6262
otherHuman: Human
6363
height(unit: Unit): Float
6464
homePlanet: String
Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
# Download_GraphQL_Schema_Slicing_Args_Enabled
2+
3+
```text
4+
Headers:
5+
ETag: "1-B2t9cwf/BRF8NYIpFDtJE4DLg8FfD5d5HdAF9KObaUc="
6+
Cache-Control: public, must-revalidate, max-age=3600
7+
Content-Type: application/graphql; charset=utf-8
8+
Content-Disposition: attachment; filename="schema.graphql"
9+
Last-Modified: Fri, 01 Jan 2021 00:00:00 GMT
10+
Content-Length: 7193
11+
-------------------------->
12+
Status Code: OK
13+
-------------------------->
14+
schema {
15+
query: Query
16+
mutation: Mutation
17+
subscription: Subscription
18+
}
19+
20+
interface Character {
21+
id: ID!
22+
name: String!
23+
friends("Returns the first _n_ elements from the list." first: Int "Returns the elements in the list that come after the specified cursor." after: String "Returns the last _n_ elements from the list." last: Int "Returns the elements in the list that come before the specified cursor." before: String): FriendsConnection
24+
appearsIn: [Episode]
25+
traits: JSON
26+
height(unit: Unit): Float
27+
}
28+
29+
type Droid implements Character {
30+
id: ID!
31+
name: String!
32+
appearsIn: [Episode]
33+
friends("Returns the first _n_ elements from the list." first: Int "Returns the elements in the list that come after the specified cursor." after: String "Returns the last _n_ elements from the list." last: Int "Returns the elements in the list that come before the specified cursor." before: String): FriendsConnection @listSize(assumedSize: 50, slicingArguments: [ "first", "last" ], sizedFields: [ "edges", "nodes" ])
34+
height(unit: Unit): Float
35+
primaryFunction: String
36+
traits: JSON
37+
}
38+
39+
"A connection to a list of items."
40+
type FriendsConnection {
41+
"Information to aid in pagination."
42+
pageInfo: PageInfo!
43+
"A list of edges."
44+
edges: [FriendsEdge!]
45+
"A flattened list of the nodes."
46+
nodes: [Character]
47+
}
48+
49+
"An edge in a connection."
50+
type FriendsEdge {
51+
"A cursor for use in pagination."
52+
cursor: String!
53+
"The item at the end of the edge."
54+
node: Character
55+
}
56+
57+
type Human implements Character {
58+
id: ID!
59+
name: String!
60+
appearsIn: [Episode]
61+
friends("Returns the first _n_ elements from the list." first: Int "Returns the elements in the list that come after the specified cursor." after: String "Returns the last _n_ elements from the list." last: Int "Returns the elements in the list that come before the specified cursor." before: String): FriendsConnection @listSize(assumedSize: 50, slicingArguments: [ "first", "last" ], sizedFields: [ "edges", "nodes" ])
62+
otherHuman: Human
63+
height(unit: Unit): Float
64+
homePlanet: String
65+
traits: JSON
66+
}
67+
68+
type Mutation {
69+
createReview(episode: Episode! review: ReviewInput!): Review! @cost(weight: "10")
70+
complete(episode: Episode!): Boolean! @cost(weight: "10")
71+
}
72+
73+
"Information about pagination in a connection."
74+
type PageInfo {
75+
"Indicates whether more edges exist following the set defined by the clients arguments."
76+
hasNextPage: Boolean!
77+
"Indicates whether more edges exist prior the set defined by the clients arguments."
78+
hasPreviousPage: Boolean!
79+
"When paginating backwards, the cursor to continue."
80+
startCursor: String
81+
"When paginating forwards, the cursor to continue."
82+
endCursor: String
83+
}
84+
85+
type Query {
86+
hero(episode: Episode! = NEW_HOPE): Character
87+
heroByTraits(traits: JSON!): Character
88+
heroes(episodes: [Episode!]!): [Character!]
89+
character(characterIds: [String!]!): [Character!]! @cost(weight: "10")
90+
search(text: String!): [SearchResult]
91+
human(id: String!): Human
92+
droid(id: String!): Droid
93+
time: Long!
94+
evict: Boolean!
95+
wait(m: Int!): Boolean! @cost(weight: "10")
96+
someDeprecatedField(deprecatedArg: String! = "foo" @deprecated(reason: "use something else")): String! @deprecated(reason: "use something else")
97+
}
98+
99+
type Review {
100+
commentary: String @cost(weight: "10")
101+
stars: Int!
102+
}
103+
104+
type Starship {
105+
id: ID!
106+
name: String!
107+
length(unit: Unit): Float!
108+
}
109+
110+
type Subscription {
111+
onReview(episode: Episode!): Review!
112+
onNext: String! @cost(weight: "10")
113+
onException: String! @cost(weight: "10")
114+
delay(delay: Int! count: Int!): String! @cost(weight: "10")
115+
}
116+
117+
union SearchResult = Starship | Human | Droid
118+
119+
input ReviewInput {
120+
stars: Int!
121+
commentary: String
122+
}
123+
124+
enum Episode {
125+
NEW_HOPE
126+
EMPIRE
127+
JEDI
128+
}
129+
130+
enum Unit {
131+
FOOT
132+
METERS
133+
}
134+
135+
"The purpose of the `cost` directive is to define a `weight` for GraphQL types, fields, and arguments. Static analysis can use these weights when calculating the overall cost of a query or response."
136+
directive @cost("The `weight` argument defines what value to add to the overall cost for every appearance, or possible appearance, of a type, field, argument, etc." weight: String!) on SCALAR | OBJECT | FIELD_DEFINITION | ARGUMENT_DEFINITION | ENUM | INPUT_FIELD_DEFINITION
137+
138+
"The `@defer` directive may be provided for fragment spreads and inline fragments to inform the executor to delay the execution of the current fragment to indicate deprioritization of the current fragment. A query with `@defer` directive will cause the request to potentially return multiple responses, where non-deferred data is delivered in the initial response and data deferred is delivered in a subsequent response. `@include` and `@skip` take precedence over `@defer`."
139+
directive @defer("If this argument label has a value other than null, it will be passed on to the result of this defer directive. This label is intended to give client applications a way to identify to which fragment a deferred result belongs to." label: String "Deferred when true." if: Boolean) on FRAGMENT_SPREAD | INLINE_FRAGMENT
140+
141+
"The purpose of the `@listSize` directive is to either inform the static analysis about the size of returned lists (if that information is statically available), or to point the analysis to where to find that information."
142+
directive @listSize("The `assumedSize` argument can be used to statically define the maximum length of a list returned by a field." assumedSize: Int "The `slicingArguments` argument can be used to define which of the field's arguments with numeric type are slicing arguments, so that their value determines the size of the list returned by that field. It may specify a list of multiple slicing arguments." slicingArguments: [String!] "The `sizedFields` argument can be used to define that the value of the `assumedSize` argument or of a slicing argument does not affect the size of a list returned by a field itself, but that of a list returned by one of its sub-fields." sizedFields: [String!] "The `requireOneSlicingArgument` argument can be used to inform the static analysis that it should expect that exactly one of the defined slicing arguments is present in a query. If that is not the case (i.e., if none or multiple slicing arguments are present), the static analysis may throw an error." requireOneSlicingArgument: Boolean! = true) on FIELD_DEFINITION
143+
144+
"The `@stream` directive may be provided for a field of `List` type so that the backend can leverage technology such as asynchronous iterators to provide a partial list in the initial response, and additional list items in subsequent responses. `@include` and `@skip` take precedence over `@stream`."
145+
directive @stream("If this argument label has a value other than null, it will be passed on to the result of this stream directive. This label is intended to give client applications a way to identify to which fragment a streamed result belongs to." label: String "The initial elements that shall be send down to the consumer." initialCount: Int! = 0 "Streamed when true." if: Boolean) on FIELD
146+
147+
scalar JSON
148+
149+
"The `Long` scalar type represents non-fractional signed whole 64-bit numeric values. Long can represent values between -(2^63) and 2^63 - 1."
150+
scalar Long
151+
```

src/HotChocolate/CostAnalysis/src/CostAnalysis/CostTypeInterceptor.cs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,12 @@ public override void OnAfterCompleteName(ITypeCompletionContext completionContex
7272
// https://ibm.github.io/graphql-specs/cost-spec.html#sec-requireOneSlicingArgument
7373
// Per default, requireOneSlicingArgument is enabled,
7474
// and has to be explicitly disabled if not desired for a field.
75+
// However, we have found that users turn the whole cost feature of because of this setting
76+
// which leads to less overall security for the deployed GraphQL server.
77+
// For this reason we have decided to disable slicing arguments by default.
7578
var requirePagingBoundaries =
76-
slicingArgs.Length > 0 && (options.RequirePagingBoundaries ?? true);
79+
slicingArgs.Length > 0
80+
&& (options.RequirePagingBoundaries ?? false);
7781

7882
fieldDef.AddDirective(
7983
new ListSizeDirective(

src/HotChocolate/CostAnalysis/test/CostAnalysis.Tests/PagingTests.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ public async Task Ensure_Paging_Defaults_Are_Applied()
2121
.AddQueryType<Query>()
2222
.AddFiltering()
2323
.AddSorting()
24+
.ModifyPagingOptions(o => o.RequirePagingBoundaries = true)
2425
.BuildSchemaAsync();
2526

2627
schema.MatchSnapshot();
@@ -109,6 +110,7 @@ public async Task Require_Paging_Boundaries_By_Default_With_Connections()
109110
.AddQueryType<Query>()
110111
.AddFiltering()
111112
.AddSorting()
113+
.ModifyPagingOptions(o => o.RequirePagingBoundaries = true)
112114
.BuildRequestExecutorAsync();
113115

114116
// act
@@ -277,6 +279,7 @@ public async Task Require_Paging_Boundaries_Two_Boundaries_Mixed()
277279
.AddQueryType<Query>()
278280
.AddFiltering()
279281
.AddSorting()
282+
.ModifyPagingOptions(o => o.RequirePagingBoundaries = true)
280283
.BuildRequestExecutorAsync();
281284

282285
// act
@@ -324,6 +327,7 @@ public async Task Require_Paging_Nested_Boundaries()
324327
.AddQueryType<Query>()
325328
.AddFiltering()
326329
.AddSorting()
330+
.ModifyPagingOptions(o => o.RequirePagingBoundaries = true)
327331
.BuildRequestExecutorAsync();
328332

329333
// act

0 commit comments

Comments
 (0)