Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
myronmarston opened this issue May 1, 2024 · 2 comments · Fixed by #4963
Closed

Comments

@myronmarston
Copy link

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.

@rmosolgo
Copy link
Owner

rmosolgo commented May 2, 2024

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!

@myronmarston
Copy link
Author

myronmarston commented May 3, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants