Skip to content

Commit be15dcf

Browse files
committed
Apply validate_timeout to analysis, too
1 parent 575be8e commit be15dcf

File tree

6 files changed

+53
-6
lines changed

6 files changed

+53
-6
lines changed

guides/queries/timeout.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,16 +63,17 @@ class MySchema < GraphQL::Schema
6363
end
6464
```
6565

66-
## Validation
66+
## Validation and Analysis
6767

6868
Queries can originate from a user, and may be crafted in a manner to take a long time to validate against the schema.
6969

70-
It is possible to limit how many seconds the static validation rules are allowed to run before returning a validation timeout error. The default is no timeout.
70+
It is possible to limit how many seconds the static validation rules and analysers are allowed to run before returning a validation timeout error. The default is no timeout.
7171

7272
For example:
7373

7474
```ruby
7575
class MySchema < GraphQL::Schema
76+
# Applies to static validation and query analysis
7677
validate_timeout 10
7778
end
7879
```

lib/graphql/analysis/ast.rb

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
require "graphql/analysis/ast/max_query_complexity"
77
require "graphql/analysis/ast/query_depth"
88
require "graphql/analysis/ast/max_query_depth"
9+
require "timeout"
910

1011
module GraphQL
1112
module Analysis
@@ -63,7 +64,10 @@ def analyze_query(query, analyzers, multiplex_analyzers: [])
6364
analyzers: analyzers_to_run
6465
)
6566

66-
visitor.visit
67+
# `nil` or `0` causes no timeout
68+
Timeout::timeout(query.validate_timeout_remaining) do
69+
visitor.visit
70+
end
6771

6872
if visitor.rescued_errors.any?
6973
return visitor.rescued_errors
@@ -75,6 +79,8 @@ def analyze_query(query, analyzers, multiplex_analyzers: [])
7579
[]
7680
end
7781
end
82+
rescue Timeout::Error
83+
[GraphQL::AnalysisError.new("Timeout on validation of query")]
7884
end
7985

8086
def analysis_errors(results)

lib/graphql/query.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ def validation_pipeline
317317
end
318318

319319
def_delegators :validation_pipeline, :validation_errors,
320-
:analyzers, :ast_analyzers, :max_depth, :max_complexity
320+
:analyzers, :ast_analyzers, :max_depth, :max_complexity, :validate_timeout_remaining
321321

322322
attr_accessor :analysis_errors
323323
def valid?

lib/graphql/query/validation_pipeline.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ class Query
1414
#
1515
# @api private
1616
class ValidationPipeline
17-
attr_reader :max_depth, :max_complexity
17+
attr_reader :max_depth, :max_complexity, :validate_timeout_remaining
1818

1919
def initialize(query:, parse_error:, operation_name_error:, max_depth:, max_complexity:)
2020
@validation_errors = []
@@ -71,7 +71,7 @@ def ensure_has_validated
7171
validator = @query.static_validator || @schema.static_validator
7272
validation_result = validator.validate(@query, validate: @query.validate, timeout: @schema.validate_timeout, max_errors: @schema.validate_max_errors)
7373
@validation_errors.concat(validation_result[:errors])
74-
74+
@validate_timeout_remaining = validation_result[:remaining_timeout]
7575
if @validation_errors.empty?
7676
@validation_errors.concat(@query.variables.errors)
7777
end

lib/graphql/static_validation/validator.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ def initialize(schema:, rules: GraphQL::StaticValidation::ALL_RULES)
2828
# @return [Array<Hash>]
2929
def validate(query, validate: true, timeout: nil, max_errors: nil)
3030
query.current_trace.validate(validate: validate, query: query) do
31+
begin_t = Time.now
3132
errors = if validate == false
3233
[]
3334
else
@@ -52,11 +53,13 @@ def validate(query, validate: true, timeout: nil, max_errors: nil)
5253
end
5354

5455
{
56+
remaining_timeout: timeout ? (timeout - (Time.now - begin_t)) : nil,
5557
errors: errors,
5658
}
5759
end
5860
rescue GraphQL::ExecutionError => e
5961
{
62+
remaining_timeout: nil,
6063
errors: [e],
6164
}
6265
end

spec/graphql/analysis/ast_spec.rb

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,4 +457,41 @@ def article(id:)
457457
assert_equal [[["article", NilClass], ["title", NilClass]]], result
458458
end
459459
end
460+
461+
462+
describe ".validate_timeout" do
463+
class AnalysisTimeoutSchema < GraphQL::Schema
464+
class SlowAnalyzer < GraphQL::Analysis::AST::Analyzer
465+
def on_enter_field(node, parent, visitor)
466+
sleep 0.1
467+
super
468+
end
469+
470+
def result
471+
nil
472+
end
473+
end
474+
475+
class Query < GraphQL::Schema::Object
476+
field :f1, Int
477+
478+
def f1
479+
context[:int] ||= 0
480+
context[:int] += 1
481+
end
482+
end
483+
484+
query(Query)
485+
query_analyzer(SlowAnalyzer)
486+
validate_timeout 0.5
487+
end
488+
489+
it "covers analysis too" do
490+
res = AnalysisTimeoutSchema.execute("{ f1: f1 f2: f1 }")
491+
assert_equal({ "f1" => 1, "f2" => 2}, res["data"])
492+
493+
res2 = AnalysisTimeoutSchema.execute("{ f1: f1, f2: f1, f3: f1, f4: f1, f5: f1, f6: f1}")
494+
assert_equal ["Timeout on validation of query"], res2["errors"].map { |e| e["message"]}
495+
end
496+
end
460497
end

0 commit comments

Comments
 (0)