Skip to content

Commit

Permalink
[graphql/rpc] enforce ban on directives (MystenLabs#15124)
Browse files Browse the repository at this point in the history
  • Loading branch information
oxade authored Nov 30, 2023
1 parent 3800740 commit ceffc59
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 2 deletions.
42 changes: 42 additions & 0 deletions crates/sui-graphql-e2e-tests/tests/limits/directives.exp
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
processed 3 tasks

init:
A: object(0,0)

task 1 'run-graphql'. lines 6-10:
Response: {
"data": null,
"errors": [
{
"message": "Fields with directives are not supported",
"locations": [
{
"line": 2,
"column": 3
}
],
"extensions": {
"code": "INTERNAL_SERVER_ERROR"
}
}
]
}

task 2 'run-graphql'. lines 12-40:
Response: {
"data": null,
"errors": [
{
"message": "Fragments with directives are not supported",
"locations": [
{
"line": 1,
"column": 1
}
],
"extensions": {
"code": "INTERNAL_SERVER_ERROR"
}
}
]
}
40 changes: 40 additions & 0 deletions crates/sui-graphql-e2e-tests/tests/limits/directives.move
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Copyright (c) Mysten Labs, Inc.
// SPDX-License-Identifier: Apache-2.0

//# init --addresses Test=0x0 --accounts A --simulator

//# run-graphql

{
chainIdentifier @deprecated
}

//# run-graphql

fragment Modules on Object @deprecated {
location
asMovePackage {
module(name: "m") {
name
package { asObject { location } }

fileFormatVersion
bytes
disassembly
}
}
}

{
transactionBlockConnection(last: 1) {
nodes {
effects {
objectChanges {
outputState {
...Modules
}
}
}
}
}
}
23 changes: 21 additions & 2 deletions crates/sui-graphql-rpc/src/extensions/query_limits_checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,6 @@ impl Extension for QueryLimitsChecker {
variables: &Variables,
next: NextParseQuery<'_>,
) -> ServerResult<ExecutableDocument> {
// TODO: limit/ban directives for now

let cfg = ctx
.data::<ServiceConfig>()
.expect("No service config provided in schema data");
Expand Down Expand Up @@ -212,6 +210,13 @@ impl QueryLimitsChecker {

match &curr_sel.node {
Selection::Field(f) => {
if !f.node.directives.is_empty() {
return Err(graphql_error_at_pos(
INTERNAL_SERVER_ERROR,
"Fields with directives are not supported",
f.pos,
));
}
for field_sel in f.node.selection_set.node.items.iter() {
que.push_back(field_sel);
cost.num_nodes += 1;
Expand All @@ -234,13 +239,27 @@ impl QueryLimitsChecker {
// TODO: this is inefficient as we might loop over same fragment multiple times
// Ideally web should cache the costs of fragments we've seen before
// Will do as enhancement
if !frag_def.node.directives.is_empty() {
return Err(graphql_error_at_pos(
INTERNAL_SERVER_ERROR,
"Fragments with directives are not supported",
frag_def.pos,
));
}
for frag_sel in frag_def.node.selection_set.node.items.iter() {
que.push_back(frag_sel);
cost.num_nodes += 1;
check_limits(limits, cost.num_nodes, cost.depth, Some(frag_sel.pos))?;
}
}
Selection::InlineFragment(fs) => {
if !fs.node.directives.is_empty() {
return Err(graphql_error_at_pos(
INTERNAL_SERVER_ERROR,
"Inline fragments with directives are not supported",
fs.pos,
));
}
for in_frag_sel in fs.node.selection_set.node.items.iter() {
que.push_back(in_frag_sel);
cost.num_nodes += 1;
Expand Down

0 comments on commit ceffc59

Please sign in to comment.