rmosolgo/graphql-ruby

Query#fingerprint raises "TypeError: no implicit conversion of nil into String"

myronmarston opened this issue · 2 comments

Describe the bug

GraphQL::Query#fingerprint raises an error if query_string is nil. That makes fingerprint dangerous to call.

Versions

graphql version: 2.3.2
rails (or other framework): N/A
other applicable versions (graphql-batch, etc) N/A

GraphQL schema

Doesn't matter, but here's the schema from my reproduction script:

  type Query {
    foo: Int
  }

GraphQL query

nil

Steps to reproduce

Put this in graphql_gem_bug.rb:

require "bundler/inline"

gemfile do
  source "https://rubygems.org"
  gem "graphql", ENV.fetch("GRAPHQL_GEM_VERSION", "2.3.2")
end

require "graphql"

schema_string = <<~EOS
  type Query {
    foo: Int
  }
EOS

schema = ::GraphQL::Schema.from_definition(schema_string)

query = ::GraphQL::Query.new(
  schema,
  nil,
  variables: {},
  context: {}
)

puts query.fingerprint

Then run it.

Expected behavior

I expect query.fingerprint to not raise an exception. I think it would be acceptable if it returned nil, or returned the same fingerprint that an empty-string query produces, or some similar benign behavior.

Actual behavior

An exception is raised.

Click to view exception backtrace
$ ruby tmp/graphql_gem_bug.rb
/Users/myron/.rvm/gems/ruby-3.2.3/gems/graphql-2.3.2/lib/graphql/query/fingerprint.rb:21:in `digest': no implicit conversion of nil into String (TypeError)

        bytes = Digest::SHA256.digest(input_str)
                                      ^^^^^^^^^
        from /Users/myron/.rvm/gems/ruby-3.2.3/gems/graphql-2.3.2/lib/graphql/query/fingerprint.rb:21:in `generate'
        from /Users/myron/.rvm/gems/ruby-3.2.3/gems/graphql-2.3.2/lib/graphql/query.rb:307:in `operation_fingerprint'
        from /Users/myron/.rvm/gems/ruby-3.2.3/gems/graphql-2.3.2/lib/graphql/query.rb:302:in `fingerprint'
        from tmp/graphql_gem_bug.rb:25:in `<main>'

Additional context

None.

Hey, thanks for reporting this. I certainly agree this could be handled better.

What were you trying to do when you encountered this? I don't quite see how query.fingerprint would be useful without a query string!

What were you trying to do when you encountered this? I don't quite see how query.fingerprint would be useful without a query string!

There's a bit of a mismatch going on between the layers of my application. The HTTP endpoint handles two types of requests: GraphQL requests and status health checks from envoy. After making some code changes to the logic that routes between these two types of requests, I (briefly) deployed a bug that caused the status health checks to get processed as GraphQL requests. That lead to a situation where The query_string was nil (since the status health checks are just simple GET /_status calls...), and when the GraphQL endpoint logic went to log query.fingerprint I got the exception. Obviously, this was caused by a bug on my end, which I rectified, but I thought it worth reporting the exception I got from query.fingerprint, too.