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

(core) - Prevent ignored characters from being sanitized in strings #2295

Merged
merged 2 commits into from
Feb 23, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/serious-queens-suffer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@urql/core': patch
---

Prevent ignored characters in GraphQL queries from being replaced inside strings and block strings. Previously we accepted sanitizing strings via regular expressions causing duplicate hashes as acceptable, since it'd only be caused when a string wasn't extracted into variables. This is fixed now however.
57 changes: 47 additions & 10 deletions packages/core/src/utils/request.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { parse, print } from 'graphql';
import { gql } from '../gql';
import { createRequest } from './request';
import { createRequest, stringifyDocument } from './request';

jest.mock('./hash', () => ({
hash: jest.requireActual('./hash').hash,
Expand Down Expand Up @@ -79,13 +79,50 @@ it('should return a valid query object with variables', () => {
});
});

it('should remove comments', () => {
const doc = `
{ #query
# broken
test
}
`;
const val = createRequest(doc);
expect(print(val.query)).toBe(`{\n test\n}`);
describe('stringifyDocument (internal API)', () => {
it('should remove comments', () => {
const doc = `
{ #query
# broken
test
}
`;
expect(stringifyDocument(createRequest(doc).query)).toBe('{ test }');
});

it('should remove duplicate spaces', () => {
const doc = `
{
abc ,, test
}
`;
expect(stringifyDocument(createRequest(doc).query)).toBe('{ abc test }');
});

it('should not sanitize within strings', () => {
const doc = `
{
field(arg: "test #1")
}
`;
expect(stringifyDocument(createRequest(doc).query)).toBe(
'{ field(arg:"test #1") }'
);
});

it('should not sanitize within block strings', () => {
const doc = `
{
field(
arg: """
hello
hello
"""
)
}
`;
expect(stringifyDocument(createRequest(doc).query)).toBe(
'{ field(arg:"""\n hello\n hello\n """) }'
);
});
});
10 changes: 9 additions & 1 deletion packages/core/src/utils/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,22 @@ export interface KeyedDocumentNode extends DocumentNode {
__key: number;
}

const GRAPHQL_STRING_RE = /("{3}[\s\S]*"{3}|"(?:\\.|[^"])*")/g;
const REPLACE_CHAR_RE = /([\s,]|#[^\n\r]+)+/g;

const replaceOutsideStrings = (str: string, idx: number) =>
idx % 2 === 0 ? str.replace(REPLACE_CHAR_RE, ' ').trim() : str;

export const stringifyDocument = (
node: string | DefinitionNode | DocumentNode
): string => {
let str = (typeof node !== 'string'
? (node.loc && node.loc.source.body) || print(node)
: node
)
.replace(/([\s,]|#[^\n\r]+)+/g, ' ')
.split(GRAPHQL_STRING_RE)
.map(replaceOutsideStrings)
.join('')
.trim();

if (typeof node !== 'string') {
Expand Down