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.

Sign up for GitHub

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

Jump to bottom

Correct escaping of forward slash in pattern regex, deprioritizing escaping of unnecessarily escaped forward slash in pattern regex #288

Merged
merged 5 commits into from
Apr 17, 2024
Merged

Correct escaping of forward slash in pattern regex, deprioritizing escaping of unnecessarily escaped forward slash in pattern regex #288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2f34f63
correct test expectation to look for a backslash
mjperrone Apr 16, 2024
54e4ea5
correct text expectation for validation.test.ts
mjperrone Apr 16, 2024
18f1509
add back replacing / with \/
mjperrone Apr 16, 2024
7757031
add test showing unnecessarily escaped forward slash rendering to a r…
mjperrone Apr 17, 2024
6fd06aa
add changeset
mjperrone Apr 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/purple-ducks-dance.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"openapi-zod-client": patch
---

Fix escaping of forward slash `/` in pattern regex so the output code will be `/\//`. This change also breaks escaping of unnecessarily escaped forward slash `\/` in pattern regex, such that the output code will be `/\\//`.
3 changes: 2 additions & 1 deletion lib/src/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ export const escapeControlCharacters = (str: string): string => {
if (dec <= 0xff) return `\\x${`00${hex}`.slice(-2)}`;
// eslint-disable-next-line sonarjs/no-nested-template-literals
return `\\u${`0000${hex}`.slice(-4)}`;
});
})
.replace(/\//g, "\\/");
};

export const toBoolean = (value: undefined | string | boolean, defaultValue: boolean) => match(value)
Expand Down
4 changes: 2 additions & 2 deletions lib/tests/regex-with-escapes.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ test("regex-with-escapes", () => {
properties: {
str: {
type: "string",
pattern: "^\/$"
pattern: "^/$"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When this string is loaded into memory it will be: ^/$.

console.log("^/$")
^/$

The previous \ was unnecessary, an escape character for a character that doesn't need escaping, resulting in the character anyways:

console.log("^\/$")
^/$

},
}
}})
).toMatchInlineSnapshot(
'"z.object({ str: z.string().regex(/^\/$/) }).partial().passthrough()"'
'"z.object({ str: z.string().regex(/^\\/$/) }).partial().passthrough()"'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point of this test is that we want the resulting code to include the escape character \ before the /, otherwise the / is interpreted as the end of the regex!

Since the \ wasn't escaped in the string definition, same as above it wasn't being included in the string in memory.

console.log("/^\/$/")
/^/$/
console.log("/^\\/$/")
/^\/$/

);
});
21 changes: 21 additions & 0 deletions lib/tests/regex-with-unnecessary-escape.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { getZodSchema } from "../src/openApiToZod";
import { test, expect } from "vitest";

test("regex-with-unnecessary-escape fails", () => {
expect(
getZodSchema({schema: {
type: "object",
properties: {
str: {
type: "string",
pattern: "^\\/$"
},
}
}})
).toMatchInlineSnapshot(
// This is what it should produce, but to prioritize escaping forward slashes without an unnecessary escape,
// we leave this is failing for now.
// '"z.object({ str: z.string().regex(/^\\/$/) }).partial().passthrough()"'
'"z.object({ str: z.string().regex(/^\\\\/$/) }).partial().passthrough()"'
);
});
2 changes: 1 addition & 1 deletion lib/tests/validations.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,6 @@ test("validations", () => {
},
})
).toMatchInlineSnapshot(
'"z.object({ str: z.string(), strWithLength: z.string().min(3).max(3), strWithMin: z.string().min(3), strWithMax: z.string().max(3), strWithPattern: z.string().regex(/^[a-z]+$/), strWithPatternWithSlash: z.string().regex(/abc/def/ghi/), email: z.string().email(), hostname: z.string().url(), url: z.string().url(), uuid: z.string().uuid(), number: z.number(), int: z.number().int(), intWithMin: z.number().int().gte(3), intWithMax: z.number().int().lte(3), intWithMinAndMax: z.number().int().gte(3).lte(3), intWithExclusiveMinTrue: z.number().int().gt(3), intWithExclusiveMinFalse: z.number().int().gte(3), intWithExclusiveMin: z.number().int().gt(3), intWithExclusiveMaxTrue: z.number().int().lt(3), intWithExclusiveMaxFalse: z.number().int().lte(3), intWithExclusiveMax: z.number().int().lt(3), intWithMultipleOf: z.number().int().multipleOf(3), bool: z.boolean(), array: z.array(z.string()), arrayWithMin: z.array(z.string()).min(3), arrayWithMax: z.array(z.string()).max(3), object: z.object({ str: z.string() }).passthrough(), objectWithRequired: z.object({ str: z.string() }).passthrough(), oneOf: z.union([z.string(), z.number()]), anyOf: z.union([z.string(), z.number()]), allOf: z.string().and(z.number()), nested: z.record(z.number()), nestedNullable: z.record(z.number().nullable()) }).passthrough()"'
'"z.object({ str: z.string(), strWithLength: z.string().min(3).max(3), strWithMin: z.string().min(3), strWithMax: z.string().max(3), strWithPattern: z.string().regex(/^[a-z]+$/), strWithPatternWithSlash: z.string().regex(/abc\\/def\\/ghi/), email: z.string().email(), hostname: z.string().url(), url: z.string().url(), uuid: z.string().uuid(), number: z.number(), int: z.number().int(), intWithMin: z.number().int().gte(3), intWithMax: z.number().int().lte(3), intWithMinAndMax: z.number().int().gte(3).lte(3), intWithExclusiveMinTrue: z.number().int().gt(3), intWithExclusiveMinFalse: z.number().int().gte(3), intWithExclusiveMin: z.number().int().gt(3), intWithExclusiveMaxTrue: z.number().int().lt(3), intWithExclusiveMaxFalse: z.number().int().lte(3), intWithExclusiveMax: z.number().int().lt(3), intWithMultipleOf: z.number().int().multipleOf(3), bool: z.boolean(), array: z.array(z.string()), arrayWithMin: z.array(z.string()).min(3), arrayWithMax: z.array(z.string()).max(3), object: z.object({ str: z.string() }).passthrough(), objectWithRequired: z.object({ str: z.string() }).passthrough(), oneOf: z.union([z.string(), z.number()]), anyOf: z.union([z.string(), z.number()]), allOf: z.string().and(z.number()), nested: z.record(z.number()), nestedNullable: z.record(z.number().nullable()) }).passthrough()"'
);
});
Loading