Feature Request: C parser workaround for line terminators in single quoted strings

[x-eleos]

Is your feature request related to a problem? Please describe.

Hey @rmosolgo from Github! We were upgrading our Ruby gem and unfortunately out integrators got bitten by...

https://github.com/rmosolgo/graphql-ruby/blob/master/CHANGELOG.md#230-20-mar-2024

Parser: line terminators are no longer allowed in single-quoted strings (as per the GraphQL spec). Escape newline characters instead; see GraphQL::Language.escape_single_quoted_newline(query_str) if you need to transform incoming query strings #4834

We definitely appreciate the workaround, however i think in this scenario given the number of calls we get a day it would significantly impact performance if we were to attempt to transform incoming query strings within Ruby.

Describe the solution you'd like

Do you have any suggestions for a more performant workaround? We worry about the cost of doing this in ruby and were wondering if there's any modifications to be made within the C parser itself, perhaps some form of feature flag etc to tell the parser to attempt this.

Describe alternatives you've considered

Unfortunately our only solution right now is to play the waiting game with our integrators to update their queries/mutations to send queries in the correct syntax.

Additional context

Add any other context or screenshots about the feature request here.

[rmosolgo]

Hey, thanks for the detailed writeup and sorry for the trouble with that change! I have a couple of thoughts:

• Yes, it would be possible to handle this dynamically in the C parser itself. But it would be very hard :S There's a similar compatibility flag here: https://github.com/rmosolgo/graphql-ruby/blob/312e0a67dc07d3d79a44c574c9a6e0054a639095/graphql-c_parser/ext/graphql_c_parser_ext/lexer.rl#L202

• What about rescuing parse errors and applying the escape method only when necessary, for example:

  def parse_with_escape(graphql_str)
    GraphQL.parse(graphql_str)
  rescue GraphQL::ParseError => err
    if err.message.include?('syntax error, unexpected invalid token ("\\"")') # Or "Expected string or block string, but it was malformed" for the Ruby parser
      better_str = GraphQL::Language.escape_single_quoted_newlines(graphql_str)
      GraphQL.parse(better_str)
      # TODO also log this event and notify the client somehow to update their query
    else
      raise
    end
  end
  

  That would reduce the overhead of escaping when you don't actually need it. Depending on your setup, you could call that method to parse incoming strings before passing them to .execute.

• I bet it's also possible to speed up that escape method a bit. I see it's using .scan but ignoring the return value; those could be migrated to .skip at a minimum. I'll take a look.

[rmosolgo]

Hey, I don't have any plans to work more on this. I'm hoping that my performance improvements to escape_single_quoted_newlines worked alright for you. If you find that they don't, please let me know and I can keep investigating!