diff options
author | charlieablett <cablett@gitlab.com> | 2019-08-23 00:17:38 +1000 |
---|---|---|
committer | charlieablett <cablett@gitlab.com> | 2019-10-23 15:40:12 +1300 |
commit | 1689559facc7d50130c11c8fdc496641f719ae75 (patch) | |
tree | 5172eab690cda555a78e29d4ef88cc10c84f8426 /spec | |
parent | 1425a56c75beecaa289ad59587d636f8f469509e (diff) | |
download | gitlab-ce-1689559facc7d50130c11c8fdc496641f719ae75.tar.gz |
Check for recursion and fail if too recursive
- List all overly-recursive fields
- Reduce recursion threshold to 2
- Add test for not-recursive-enough query
- Use reusable methods in tests
- Add changelog
- Set changeable acceptable recursion level
- Add error check test helpers
Diffstat (limited to 'spec')
-rw-r--r-- | spec/fixtures/api/graphql/recursive-introspection.graphql | 57 | ||||
-rw-r--r-- | spec/fixtures/api/graphql/recursive-query.graphql | 47 | ||||
-rw-r--r-- | spec/fixtures/api/graphql/small-recursive-introspection.graphql | 15 | ||||
-rw-r--r-- | spec/requests/api/graphql/gitlab_schema_spec.rb | 47 | ||||
-rw-r--r-- | spec/support/helpers/graphql_helpers.rb | 17 |
5 files changed, 176 insertions, 7 deletions
diff --git a/spec/fixtures/api/graphql/recursive-introspection.graphql b/spec/fixtures/api/graphql/recursive-introspection.graphql new file mode 100644 index 00000000000..db970bb14b6 --- /dev/null +++ b/spec/fixtures/api/graphql/recursive-introspection.graphql @@ -0,0 +1,57 @@ +query allSchemaTypes { + __schema { + types { + fields { + type{ + fields { + type { + fields { + type { + fields { + type { + fields { + type { + fields { + type { + fields { + type { + fields { + type { + fields { + type { + fields { + type { + fields { + type { + fields { + type { + fields { + name + } + } + } + } + } + } + } + } + } + } + } + } + } + } + } + } + } + } + } + } + } + } + } + } + } + } + } +}
\ No newline at end of file diff --git a/spec/fixtures/api/graphql/recursive-query.graphql b/spec/fixtures/api/graphql/recursive-query.graphql new file mode 100644 index 00000000000..d1616c4de6e --- /dev/null +++ b/spec/fixtures/api/graphql/recursive-query.graphql @@ -0,0 +1,47 @@ +{ + project(fullPath: "gitlab-org/gitlab-ce") { + group { + projects { + edges { + node { + group { + projects { + edges { + node { + group { + projects { + edges { + node { + group { + projects { + edges { + node { + group { + projects { + edges { + node { + group { + description + } + } + } + } + } + } + } + } + } + } + } + } + } + } + } + } + } + } + } + } + } + } +} diff --git a/spec/fixtures/api/graphql/small-recursive-introspection.graphql b/spec/fixtures/api/graphql/small-recursive-introspection.graphql new file mode 100644 index 00000000000..1025043b474 --- /dev/null +++ b/spec/fixtures/api/graphql/small-recursive-introspection.graphql @@ -0,0 +1,15 @@ +query allSchemaTypes { + __schema { + types { + fields { + type { + fields { + type { + name + } + } + } + } + } + } +} diff --git a/spec/requests/api/graphql/gitlab_schema_spec.rb b/spec/requests/api/graphql/gitlab_schema_spec.rb index e1eb7c7f738..1f2bc67a9d2 100644 --- a/spec/requests/api/graphql/gitlab_schema_spec.rb +++ b/spec/requests/api/graphql/gitlab_schema_spec.rb @@ -13,7 +13,7 @@ describe 'GitlabSchema configurations' do subject - expect(graphql_errors.flatten.first['message']).to include('which exceeds max complexity of 1') + expect_graphql_errors_to_include /which exceeds max complexity of 1/ end end end @@ -21,12 +21,11 @@ describe 'GitlabSchema configurations' do describe '#max_depth' do context 'when query depth is too high' do it 'shows error' do - errors = { "message" => "Query has depth of 2, which exceeds max depth of 1" } allow(GitlabSchema).to receive(:max_query_depth).and_return 1 subject - expect(graphql_errors.flatten).to include(errors) + expect_graphql_errors_to_include /exceeds max depth/ end end @@ -36,7 +35,41 @@ describe 'GitlabSchema configurations' do subject - expect(Array.wrap(graphql_errors).compact).to be_empty + expect_graphql_errors_to_be_empty + end + end + end + end + + context 'depth, complexity and recursion checking' do + context 'unauthenticated recursive queries' do + context 'a not-quite-recursive-enough introspective query' do + it 'succeeds' do + query = File.read(Rails.root.join('spec/fixtures/api/graphql/small-recursive-introspection.graphql')) + + post_graphql(query, current_user: nil) + + expect_graphql_errors_to_be_empty + end + end + + context 'a deep but simple recursive introspective query' do + it 'fails due to recursion' do + query = File.read(Rails.root.join('spec/fixtures/api/graphql/recursive-introspection.graphql')) + + post_graphql(query, current_user: nil) + + expect_graphql_errors_to_include [/Recursive query/] + end + end + + context 'a deep recursive non-introspective query' do + it 'fails due to recursion, complexity and depth' do + query = File.read(Rails.root.join('spec/fixtures/api/graphql/recursive-query.graphql')) + + post_graphql(query, current_user: nil) + + expect_graphql_errors_to_include [/Recursive query/, /exceeds max complexity/, /exceeds max depth/] end end end @@ -86,7 +119,7 @@ describe 'GitlabSchema configurations' do # Expect errors for each query expect(graphql_errors.size).to eq(3) graphql_errors.each do |single_query_errors| - expect(single_query_errors.first['message']).to include('which exceeds max complexity of 4') + expect_graphql_errors_to_include(/which exceeds max complexity of 4/) end end end @@ -103,12 +136,12 @@ describe 'GitlabSchema configurations' do end context 'when IntrospectionQuery' do - it 'is not too complex' do + it 'is not too complex nor recursive' do query = File.read(Rails.root.join('spec/fixtures/api/graphql/introspection.graphql')) post_graphql(query, current_user: nil) - expect(graphql_errors).to be_nil + expect_graphql_errors_to_be_empty end end diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb index 4d2ad165fd6..69bf1f0f3fa 100644 --- a/spec/support/helpers/graphql_helpers.rb +++ b/spec/support/helpers/graphql_helpers.rb @@ -213,6 +213,23 @@ module GraphqlHelpers end end + def expect_graphql_errors_to_include(regexes_to_match) + raise "No errors. Was expecting to match #{regexes_to_match}" if graphql_errors.nil? || graphql_errors.empty? + + error_messages = flattened_errors.collect { |error_hash| error_hash["message"] } + Array.wrap(regexes_to_match).flatten.each do |regex| + expect(error_messages).to include a_string_matching regex + end + end + + def expect_graphql_errors_to_be_empty + expect(flattened_errors).to be_empty + end + + def flattened_errors + Array.wrap(graphql_errors).flatten.compact + end + # Raises an error if no response is found def graphql_mutation_response(mutation_name) graphql_data.fetch(GraphqlHelpers.fieldnamerize(mutation_name)) |