Fix GraphiQL :simple interface to send headers from the Headers panel#308
Fix GraphiQL :simple interface to send headers from the Headers panel#308nallwhy wants to merge 1 commit intoabsinthe-graphql:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes the :simple GraphiQL interface so headers entered in the built-in Headers panel (and configured default_headers) are actually included in requests, aligning behavior with the :advanced interface.
Changes:
- Wraps the original Absinthe Socket fetcher with a custom fetcher that forwards UI-provided headers for HTTP
query/mutationoperations. - Preserves WebSocket handling for
subscriptionoperations via the original Absinthe fetcher. - Adds
default_headersrendering/support to the:simpleGraphiQL template.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| lib/absinthe/plug/graphiql/graphiql.html.eex | Adds custom fetcher logic to merge header-editor headers into HTTP requests and initializes the header editor with default_headers. |
| lib/absinthe/plug/graphiql.ex | Threads default_headers through the :simple template render function (updates template function arity/call). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| opts[:query], | ||
| opts[:var_string], | ||
| opts[:result], | ||
| opts[:default_headers], | ||
| opts[:socket_url], | ||
| opts[:assets] |
There was a problem hiding this comment.
:simple interface now renders and enables the Headers editor and threads default_headers through to the template, but the existing test suite assertions for default_headers only cover the :advanced workspace output (defaultHeaders: ...). Add a test that renders with interface: :simple and asserts the HTML includes the serialized default headers (and that header editor is enabled), to prevent regressions.
| var query = (graphQLParams.query || '').trim(); | ||
| if (/^subscription\b/i.test(query)) { |
There was a problem hiding this comment.
Subscription detection uses /^subscription\b/i on the trimmed document. This fails for valid subscription documents that begin with comments or fragment definitions (e.g., fragment ... then subscription ...), causing the operation to be sent over HTTP instead of the WebSocket fetcher. Consider determining the operation type via a GraphQL parser (preferred) or at least expanding the regex to skip leading comments/whitespace and fragment definitions before checking for subscription.
| var query = (graphQLParams.query || '').trim(); | |
| if (/^subscription\b/i.test(query)) { | |
| var query = graphQLParams.query || ''; | |
| var isSubscription = /^(?:\s*(?:#.*\n|fragment\s+[A-Za-z0-9_]+\s+on\s+[A-Za-z0-9_]+\s*\{[\s\S]*?\}\s*))*\s*subscription\b/i.test(query); | |
| if (isSubscription) { |
| query: '<%= query_string %>', | ||
| response: '<%= result_string %>', | ||
| variables: '<%= variables_string %>', |
There was a problem hiding this comment.
query_string, result_string, and variables_string are interpolated directly into a JavaScript string literal inside a <script> tag with only minimal escaping, which does not prevent </script> or other HTML-significant sequences from being injected. If an attacker can influence GraphQL queries/variables or any string data returned in the GraphQL result (for example via database-backed fields), they can craft values containing </script><script>... to prematurely terminate this script block and execute arbitrary JavaScript in the context of the GraphiQL origin. These values should be passed through a JS/HTML-safe encoder that also escapes </script>/</& (or embedded via a data attribute/JSON script tag and read at runtime) before being written into the script block.
| query: '<%= query_string %>', | |
| response: '<%= result_string %>', | |
| variables: '<%= variables_string %>', | |
| query: <%= query_string |> Jason.encode!() |> String.replace("<", "\\u003C") |> String.replace("&", "\\u0026") %>, | |
| response: <%= result_string |> Jason.encode!() |> String.replace("<", "\\u003C") |> String.replace("&", "\\u0026") %>, | |
| variables: <%= variables_string |> Jason.encode!() |> String.replace("<", "\\u003C") |> String.replace("&", "\\u0026") %>, |
Problem
In the GraphiQL
:simpleinterface, headers entered in the built-in Headers panel were not sent to the server. The originalAbsintheSocketGraphiql.createFetcherdoes not forward headers passed from the GraphiQL UI, so they were silently ignored.Note: the
:advancedinterface (graphiql_workspace.html.eex) already haddefault_headerssupport.Changes
subscriptionqueries toabsintheFetcher(WebSocket)query/mutationvia nativefetch(), merging headersfrom the GraphiQL Headers panel into the request
default_headerssupport to the:simpleinterface, consistent with the:advancedinterface