Authorization failures on types defined by anonymous classes in large applications can be very slow

[robdimarco]

Describe the bug

Authorization failures on types defined by anonymous classes in large applications can be very slow.

Versions

graphql version: 1.8.7
ruby version: < 2.7

Steps to reproduce

1. Be running a ruby version prior to 2.7
2. Be running a large application with lots of constants in the constant namespace of the VM.
3. Have a schema that is using an anonymous class (e.g from Class.new(GraphQL::Object)) as a type and has not explicitly defined a name method.
4. Make a request where unauthorized? for the type will return false.

Expected behavior

Unauthorized requests should return quickly.

Actual behavior

In our (large) application, we were consistently seeing authentication failures taking 100-150 seconds.

Additional context

Prior to a patch in Ruby 2.7, Module#name has poor performance on anonymous classes, due to searching the entire constant namespace of the VM. [0][1][2]. This comes to play if you have a large application that is calling #name on an anonymous class or module.

In GraphQL::UnauthorizedError, if no message is specified, a default message is created [3]. In this default message creation, we call type.name. If the type is an anonymous class which has not explicitly defined a name method, the performance issue will be triggered.

A GraphQL::UnauthorizedError is instantiated without a message when an object fails its authorization [4].

I believe if we change the default message creation in GraphQL::UnauthorizedError#initialize to use type.graphql_name instead of type.name, we would get both a better error message AND would solve the performance regression.

In our application, we monkeypatched the GraphQL::UnauthorizedError#initialize method to not call type.name and saw our unauthorized requests go from taking 100 - 150 seconds to a few milliseconds.

Footnotes:

[0] https://bugs.ruby-lang.org/issues/11119
[1] https://bugs.ruby-lang.org/issues/15625
[2] https://bugs.ruby-lang.org/issues/15765 - Patch for Ruby 2.7
[3]

graphql-ruby/lib/graphql/unauthorized_error.rb

Line 25 in 5cb105f

message ||= "An instance of #{object.class} failed #{type.name}'s authorization check"

[4]

graphql-ruby/lib/graphql/schema/object.rb

Line 53 in 5cb105f

err = GraphQL::UnauthorizedError.new(object: object, type: self, context: context)

[rmosolgo]

Wow, what an interesting bug 🤓 ! Thanks for sharing your finding. I think a fix like you suggest (swapping .name for .graphql_name). Since class names don't match type names, I consider .name a bug in that case anyways, too bad I missed it before.

Are you interested in submitting a PR for it? If you submit a patch for master, I'll backport it to 1.8.x. However, the last 1.8 version is 1.8.15, so you'd still have to do an update to get the patch.

[robdimarco]

Thanks for the quick response, I added a PR with the proposed change.

[rmosolgo]

Thanks again for taking care of this. Besides merging #3176 to master, I also released 1.8.18 with this change 💁‍♂️ https://rubygems.org/gems/graphql/versions/1.8.18