Escape values interpolated into graphql queries - #673
Open
Thayorns wants to merge 2 commits into
Open
Conversation
Author
|
@yegor256 happy to clarify anything here, just say the word. |
Author
|
@yegor256 fixed here, take a look please |
Member
yegor256
self-requested a review
September 3, 2026 06:22
Author
|
@yegor256 take a look please |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Every query in
lib/fbe/github_graph.rbdropped its arguments straight into thequery text, so an owner, a repository name, a branch or a pagination cursor
carrying a double quote silently rewrote the query instead of failing. Feeding
a") { id } repository(owner: "xas an owner turned one repository selectioninto two fields, and graphql-ruby accepted it, so the caller got an answer about
something it never asked for.
Every such value now goes through a small
Fbe::Graph::Literalobject thatrenders it as a quoted and escaped GraphQL string literal. The call sites lose
their hand-written quotes and read
owner: #{Literal.new(owner)}, which keepsthe query text fixed no matter what the value carries. This covers owner, name,
branch, node id, the
after:cursor in all four paginated queries, and the twosearch(query:)strings intotal_issues_created.Two tests drive it: one feeds
resolved_conversationsan owner that tries toopen a second
repositoryfield, one feedspull_requests_with_reviewsacursor that tries to add an argument. Both assert the value arrives as a single
escaped literal, and both failed before the change.
allow_dynamic_queriesstays on, since the multi-repo and multi-pull queries arestill assembled at runtime.
Closes #660