Skip to content

Commit

Permalink
Fix sanitization of operation names
Browse files Browse the repository at this point in the history
Followup to #1568, where I misread the documentation of the `replace`
function, expecting it to replace all occurrences when it doesn't
(unless using a regexp with the `g` flag, which this patch does).
  • Loading branch information
Sylvain Lebresne committed Mar 8, 2022
1 parent 6f930d5 commit 82d810f
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 3 deletions.
69 changes: 68 additions & 1 deletion query-planner-js/src/__tests__/buildPlan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1129,7 +1129,74 @@ describe('fetch operation names', () => {
}
`);
const fetch = ((plan.node as SequenceNode).nodes[1] as FlattenNode).node as FetchNode;
expect(fetch.operation).toMatch(/^query myOp__non_graphqlname__1.*/i);
expect(fetch.operation).toMatch(/^query myOp__non_graphql_name__1.*/i);
});

test('ensures sanitization applies repeatedly', () => {
const subgraph1 = {
name: 'S1',
typeDefs: gql`
type Query {
t: T
}
type T @key(fields: "id") {
id: ID!
}
`
}

const subgraph2 = {
name: 'a-na&me-with-plen&ty-replace*ments',
typeDefs: gql`
type T @key(fields: "id") {
id: ID!
x: Int
}
`
}

const [api, queryPlanner] = composeAndCreatePlanner(subgraph1, subgraph2);
const operation = operationFromDocument(api, gql`
query myOp {
t {
x
}
}
`);

const plan = queryPlanner.buildQueryPlan(operation);
expect(plan).toMatchInlineSnapshot(`
QueryPlan {
Sequence {
Fetch(service: "S1") {
{
t {
__typename
id
}
}
},
Flatten(path: "t") {
Fetch(service: "a-na&me-with-plen&ty-replace*ments") {
{
... on T {
__typename
id
}
} =>
{
... on T {
x
}
}
},
},
},
}
`);
const fetch = ((plan.node as SequenceNode).nodes[1] as FlattenNode).node as FetchNode;
expect(fetch.operation).toMatch(/^query myOp__a_name_with_plenty_replacements__1.*/i);
});

test('handle very non-graph subgraph name', () => {
Expand Down
4 changes: 2 additions & 2 deletions query-planner-js/src/buildPlan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -628,8 +628,8 @@ function toValidGraphQLName(subgraphName: string): string {
// Note that this could theoretically lead to substantial changes to the name but should
// work well in practice (and if it's a huge problem for someone, we can change it).
const sanitized = subgraphName
.replace('-', '_')
.replace(/[^_0-9A-Za-z]/i, '');
.replace(/-/ig, '_')
.replace(/[^_0-9A-Za-z]/ig, '');
return sanitized.match(/^[0-9].*/i) ? '_' + sanitized : sanitized;
}

Expand Down

0 comments on commit 82d810f

Please sign in to comment.