Merge branch 'master' into fix-cve-2025-27407

Author: rmosolgo

CHANGELOG.md

@@ -10,6 +10,24 @@
 
 ### Bug fixes
 
+# 2.4.12 (11 Mar 2025)
+
+### Breaking changes
+
+- Remove `InvalidNullError#value` which is always `nil` #5256
+
+### New features
+
+- `validate_timeout` is 3 seconds by default #5258
+
+### Bug fixes
+
+- New Relic: reimplement skipping scalars by default #5271
+- Resolver: revert inheriting overridden `graphql_name` #5260
+- Analysis: manually implement timeout to handle I/O better #5263
+- Parser: properly handle extra token at the end of the query string #5267
+- Validation: fix conflicting aliases inside fragment #5268
+
 # 2.4.11 (28 Feb 2025)
 
 ### New features

cop/development/trace_methods_cop.rb

@@ -73,6 +73,7 @@ def on_module(node)
             # Not really necessary for making a good trace:
             :lex, :analyze_query, :execute_query, :execute_query_lazy,
             # Only useful for isolated event tracking:
+            :begin_dataloader, :end_dataloader,
             :dataloader_fiber_exit, :dataloader_spawn_execution_fiber, :dataloader_spawn_source_fiber
           ]
           missing_defs.each do |missing_def|

guides/authorization/visibility.md

@@ -151,8 +151,8 @@ For big schemas, this can be a worthwhile speed-up.
 - `Visibility` speeds up Rails app boot because it doesn't require all types to be loaded during boot and only loads types as they are used by queries.
 - `Visibility` supports predefined, reusable visibility profiles which speeds up queries using complicated `visible?` checks.
 - `Visibility` hides types differently in a few edge cases:
-  - Previously, `Warden` hide interface and union types which had no possible types. `Visibility` doesn't check possible types (in order to support performance improvements), so those types must return `false` for `visible?` in the same cases where all possible types were hidden. Otherwise, that interface or union type will be visible but have no possible types.
-  - Some other thing, see TODO
+  - Previously, `Warden` hid interface and union types which had no possible types. `Visibility` doesn't check possible types (in order to support performance improvements), so those types must return `false` for `visible?` in the same cases where all possible types were hidden. Otherwise, that interface or union type will be visible but have no possible types.
+  - When an object type is connected to the schema as a field return type or a union member, and also implements and interface, if the object type's _other_ connection(s) to the schema are hidden, then it won't appear as an implementer of that interface unless it's registered with `orphan_types` (either by the schema or interface). `Warden` used a "global" map of types so it could discover object types in this case, but `Visibility` doesn't have that global map. (Since time of writing, `Visibility` _does_ have some global type tracking, so maybe this could be fixed.)
 - When `Visibility` is used, several (Ruby-level) Schema introspection methods don't work because the caches they draw on haven't been calculated (`Schema.references_to`, `Schema.union_memberships`). If you're using these, please get in touch so that we can find a way forward.
 
 ### Migration Mode

guides/queries/timeout.md

@@ -67,15 +67,21 @@ end
 
 Queries can originate from a user, and may be crafted in a manner to take a long time to validate against the schema.
 
-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.
+It is possible to limit how many seconds the static validation rules and analysers are allowed to run before returning a validation timeout error. By default, validation and query analysis have a 3-second timeout. You can customize this timeout or disable it completely:
 
 For example:
 
 ```ruby
+# Customize timeout (in seconds)
 class MySchema < GraphQL::Schema
   # Applies to static validation and query analysis
   validate_timeout 10
 end
+
+# OR disable timeout completely
+class MySchema < GraphQL::Schema
+  validate_timeout nil
+end
 ```
 
 **Note:** This configuration uses Ruby's built-in `Timeout` API, which can interrupt IO calls mid-flight, resulting in [very weird bugs](https://www.mikeperham.com/2015/05/08/timeout-rubys-most-dangerous-api/). None of GraphQL-Ruby's validators make IO calls but if you want to use this configuration and you have custom static validators that make IO calls, open an issue to discuss implementing this in an IO-safe way.

lib/graphql/analysis.rb

@@ -6,11 +6,16 @@
 require "graphql/analysis/max_query_complexity"
 require "graphql/analysis/query_depth"
 require "graphql/analysis/max_query_depth"
-require "timeout"
-
 module GraphQL
   module Analysis
     AST = self
+
+    class TimeoutError < AnalysisError
+      def initialize(...)
+        super("Timeout on validation of query")
+      end
+    end
+
     module_function
     # Analyze a multiplex, and all queries within.
     # Multiplex analyzers are ran for all queries, keeping state.
@@ -61,13 +66,11 @@ def analyze_query(query, analyzers, multiplex_analyzers: [])
           if !analyzers_to_run.empty?
             visitor = GraphQL::Analysis::Visitor.new(
               query: query,
-              analyzers: analyzers_to_run
+              analyzers: analyzers_to_run,
+              timeout: query.validate_timeout_remaining,
             )
 
-            # `nil` or `0` causes no timeout
-            Timeout::timeout(query.validate_timeout_remaining) do
-              visitor.visit
-            end
+            visitor.visit
 
             if !visitor.rescued_errors.empty?
               return visitor.rescued_errors
@@ -79,8 +82,8 @@ def analyze_query(query, analyzers, multiplex_analyzers: [])
           []
         end
       end
-    rescue Timeout::Error
-      [GraphQL::AnalysisError.new("Timeout on validation of query")]
+    rescue TimeoutError => err
+      [err]
     rescue GraphQL::UnauthorizedError, GraphQL::ExecutionError
       # This error was raised during analysis and will be returned the client before execution
       []

lib/graphql/analysis/visitor.rb

@@ -10,7 +10,7 @@ module Analysis
     #
     # @see {GraphQL::Analysis::Analyzer} AST Analyzers for queries
     class Visitor < GraphQL::Language::StaticVisitor
-      def initialize(query:, analyzers:)
+      def initialize(query:, analyzers:, timeout:)
         @analyzers = analyzers
         @path = []
         @object_types = []
@@ -24,6 +24,11 @@ def initialize(query:, analyzers:)
         @types = query.types
         @response_path = []
         @skip_stack = [false]
+        @timeout_time = if timeout
+          Process.clock_gettime(Process::CLOCK_MONOTONIC, :float_second) + timeout
+        else
+          Float::INFINITY
+        end
         super(query.selected_operation)
       end
 
@@ -73,21 +78,17 @@ def response_path
         module_eval <<-RUBY, __FILE__, __LINE__
         def call_on_enter_#{node_type}(node, parent)
           @analyzers.each do |a|
-            begin
-              a.on_enter_#{node_type}(node, parent, self)
-            rescue AnalysisError => err
-              @rescued_errors << err
-            end
+            a.on_enter_#{node_type}(node, parent, self)
+          rescue AnalysisError => err
+            @rescued_errors << err
           end
         end
 
         def call_on_leave_#{node_type}(node, parent)
           @analyzers.each do |a|
-            begin
-              a.on_leave_#{node_type}(node, parent, self)
-            rescue AnalysisError => err
-              @rescued_errors << err
-            end
+            a.on_leave_#{node_type}(node, parent, self)
+          rescue AnalysisError => err
+            @rescued_errors << err
           end
         end
 
@@ -96,6 +97,7 @@ def call_on_leave_#{node_type}(node, parent)
       # rubocop:enable Development/NoEvalCop
 
       def on_operation_definition(node, parent)
+        check_timeout
         object_type = @schema.root_type_for_operation(node.operation_type)
         @object_types.push(object_type)
         @path.push("#{node.operation_type}#{node.name ? " #{node.name}" : ""}")
@@ -106,31 +108,27 @@ def on_operation_definition(node, parent)
         @path.pop
       end
 
-      def on_fragment_definition(node, parent)
-        on_fragment_with_type(node) do
-          @path.push("fragment #{node.name}")
-          @in_fragment_def = false
-          call_on_enter_fragment_definition(node, parent)
-          super
-          @in_fragment_def = false
-          call_on_leave_fragment_definition(node, parent)
-        end
-      end
-
       def on_inline_fragment(node, parent)
-        on_fragment_with_type(node) do
-          @path.push("...#{node.type ? " on #{node.type.name}" : ""}")
-          @skipping = @skip_stack.last || skip?(node)
-          @skip_stack << @skipping
-
-          call_on_enter_inline_fragment(node, parent)
-          super
-          @skipping = @skip_stack.pop
-          call_on_leave_inline_fragment(node, parent)
+        check_timeout
+        object_type = if node.type
+          @types.type(node.type.name)
+        else
+          @object_types.last
         end
+        @object_types.push(object_type)
+        @path.push("...#{node.type ? " on #{node.type.name}" : ""}")
+        @skipping = @skip_stack.last || skip?(node)
+        @skip_stack << @skipping
+        call_on_enter_inline_fragment(node, parent)
+        super
+        @skipping = @skip_stack.pop
+        call_on_leave_inline_fragment(node, parent)
+        @object_types.pop
+        @path.pop
       end
 
       def on_field(node, parent)
+        check_timeout
         @response_path.push(node.alias || node.name)
         parent_type = @object_types.last
         # This could be nil if the previous field wasn't found:
@@ -158,6 +156,7 @@ def on_field(node, parent)
       end
 
       def on_directive(node, parent)
+        check_timeout
         directive_defn = @schema.directives[node.name]
         @directive_definitions.push(directive_defn)
         call_on_enter_directive(node, parent)
@@ -167,6 +166,7 @@ def on_directive(node, parent)
       end
 
       def on_argument(node, parent)
+        check_timeout
         argument_defn = if (arg = @argument_definitions.last)
           arg_type = arg.type.unwrap
           if arg_type.kind.input_object?
@@ -192,6 +192,7 @@ def on_argument(node, parent)
       end
 
       def on_fragment_spread(node, parent)
+        check_timeout
         @path.push("... #{node.name}")
         @skipping = @skip_stack.last || skip?(node)
         @skip_stack << @skipping
@@ -269,16 +270,10 @@ def skip?(ast_node)
         !dir.empty? && !GraphQL::Execution::DirectiveChecks.include?(dir, query)
       end
 
-      def on_fragment_with_type(node)
-        object_type = if node.type
-          @types.type(node.type.name)
-        else
-          @object_types.last
+      def check_timeout
+        if Process.clock_gettime(Process::CLOCK_MONOTONIC, :float_second) > @timeout_time
+          raise GraphQL::Analysis::TimeoutError
         end
-        @object_types.push(object_type)
-        yield(node)
-        @object_types.pop
-        @path.pop
       end
     end
   end

lib/graphql/execution/interpreter/runtime.rb

@@ -473,7 +473,7 @@ def continue_value(value, field, is_non_null, ast_node, result_name, selection_r
                 # When this comes from a list item, use the parent object:
                 parent_type = selection_result.is_a?(GraphQLResultArray) ? selection_result.graphql_parent.graphql_result_type : selection_result.graphql_result_type
                 # This block is called if `result_name` is not dead. (Maybe a previous invalid nil caused it be marked dead.)
-                err = parent_type::InvalidNullError.new(parent_type, field, value, ast_node)
+                err = parent_type::InvalidNullError.new(parent_type, field, ast_node)
                 schema.type_error(err, context)
               end
             else

lib/graphql/invalid_null_error.rb

@@ -9,16 +9,12 @@ class InvalidNullError < GraphQL::Error
     # @return [GraphQL::Field] The field which failed to return a value
     attr_reader :field
 
-    # @return [nil, GraphQL::ExecutionError] The invalid value for this field
-    attr_reader :value
-
     # @return [GraphQL::Language::Nodes::Field] the field where the error occurred
     attr_reader :ast_node
 
-    def initialize(parent_type, field, value, ast_node)
+    def initialize(parent_type, field, ast_node)
       @parent_type = parent_type
       @field = field
-      @value = value
       @ast_node = ast_node
       super("Cannot return null for non-nullable field #{@parent_type.graphql_name}.#{@field.graphql_name}")
     end

lib/graphql/language/lexer.rb

@@ -13,17 +13,21 @@ def initialize(graphql_str, filename: nil, max_tokens: nil)
         @pos = nil
         @max_tokens = max_tokens || Float::INFINITY
         @tokens_count = 0
+        @finished = false
       end
 
-      def eos?
-        @scanner.eos?
+      def finished?
+        @finished
       end
 
       attr_reader :pos, :tokens_count
 
       def advance
         @scanner.skip(IGNORE_REGEXP)
-        return false if @scanner.eos?
+        if @scanner.eos?
+          @finished = true
+          return false
+        end
         @tokens_count += 1
         if @tokens_count > @max_tokens
           raise_parse_error("This query is too large to execute.")

lib/graphql/language/parser.rb

@@ -110,7 +110,7 @@ def document
           # Only ignored characters is not a valid document
           raise GraphQL::ParseError.new("Unexpected end of document", nil, nil, @graphql_str)
         end
-        while !@lexer.eos?
+        while !@lexer.finished?
           defns << definition
         end
         Document.new(pos: 0, definitions: defns, filename: @filename, source: self)