Skip to content

Commit 825ee06

Browse files
arcanismerceyz
authored andcommitted
Improves string escaping during serialization (#5621)
**What's the problem this PR addresses?** Positional arguments were over-complexified during serialization. For example, `"foo bar"` was serialized as `$'foo bar'`. It works, but it's a surprising syntax for users. **How did you fix it?** Since I needed to use the serialization to generate part of the new website, I fixed it to make it look more natural, and added tests to avoid regressions. **Checklist** <!--- Don't worry if you miss something, chores are automatically tested. --> <!--- This checklist exists to help you remember doing the chores when you submit a PR. --> <!--- Put an `x` in all the boxes that apply. --> - [x] I have read the [Contributing Guide](https://yarnpkg.com/advanced/contributing). <!-- See https://yarnpkg.com/advanced/contributing#preparing-your-pr-to-be-released for more details. --> <!-- Check with `yarn version check` and fix with `yarn version check -i` --> - [x] I have set the packages that need to be released for my changes to be effective. <!-- The "Testing chores" workflow validates that your PR follows our guidelines. --> <!-- If it doesn't pass, click on it to see details as to what your PR might be missing. --> - [x] I will check that all automated PR checks pass before the PR gets reviewed. (cherry picked from commit bbcce29)
1 parent e5914ce commit 825ee06

File tree

3 files changed

+57
-13
lines changed

3 files changed

+57
-13
lines changed

.yarn/versions/d3925be6.yml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
releases:
2+
"@yarnpkg/parsers": patch
3+
4+
declined:
5+
- "@yarnpkg/plugin-essentials"
6+
- "@yarnpkg/plugin-nm"
7+
- "@yarnpkg/plugin-version"
8+
- "@yarnpkg/cli"
9+
- "@yarnpkg/core"
10+
- "@yarnpkg/sdks"
11+
- "@yarnpkg/shell"

packages/yarnpkg-parsers/sources/shell.ts

Lines changed: 42 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -82,22 +82,52 @@ export function stringifyValueArgument(argument: ValueArgument): string {
8282
return argument.segments.map(segment => stringifyArgumentSegment(segment)).join(``);
8383
}
8484

85+
const ESCAPED_CONTROL_CHARS = new Map([
86+
[`\f`, `\\f`],
87+
[`\n`, `\\n`],
88+
[`\r`, `\\r`],
89+
[`\t`, `\\t`],
90+
[`\v`, `\\v`],
91+
[`\0`, `\\0`],
92+
]);
93+
94+
const ESCAPED_DBL_CHARS = new Map([
95+
[`\\`, `\\\\`],
96+
[`$`, `\\$`],
97+
[`"`, `\\"`],
98+
...Array.from(ESCAPED_CONTROL_CHARS, ([c, replacement]): [string, string] => {
99+
return [c, `"$'${replacement}'"`];
100+
}),
101+
]);
102+
103+
const getEscapedControlChar = (c: string) => {
104+
return ESCAPED_CONTROL_CHARS.get(c) ?? `\\x${c.charCodeAt(0).toString(16).padStart(2, `0`)}`;
105+
};
106+
107+
const getEscapedDblChar = (match: string) => {
108+
return ESCAPED_DBL_CHARS.get(match) ?? `"$'${getEscapedControlChar(match)}'"`;
109+
};
110+
85111
export function stringifyArgumentSegment(argumentSegment: ArgumentSegment): string {
86-
const doubleQuoteIfRequested = (string: string, quote: boolean) => quote ? `"${string}"` : string;
112+
const doubleQuoteIfRequested = (string: string, quote: boolean) => quote
113+
? `"${string}"`
114+
: string;
115+
87116
const quoteIfNeeded = (text: string) => {
88117
if (text === ``)
89-
return `""`;
90-
if (!text.match(/[(){}<>$|&; \t"']/))
118+
return `''`;
119+
120+
if (!text.match(/[()}<>$|&;"'\n\t ]/))
91121
return text;
92-
return `$'${text
93-
.replace(/\\/g, `\\\\`)
94-
.replace(/'/g, `\\'`)
95-
.replace(/\f/g, `\\f`)
96-
.replace(/\n/g, `\\n`)
97-
.replace(/\r/g, `\\r`)
98-
.replace(/\t/g, `\\t`)
99-
.replace(/\v/g, `\\v`)
100-
.replace(/\0/g, `\\0`)}'`;
122+
123+
if (!text.match(/['\t\p{C}]/u))
124+
return `'${text}'`;
125+
126+
if (!text.match(/'/)) {
127+
return `$'${text.replace(/[\t\p{C}]/u, getEscapedControlChar)}'`;
128+
} else {
129+
return `"${text.replace(/["$\t\p{C}]/u, getEscapedDblChar)}"`;
130+
}
101131
};
102132

103133
switch (argumentSegment.type) {

packages/yarnpkg-parsers/tests/shell.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,9 @@ describe(`Shell parser`, () => {
162162

163163
const STRINGIFIER_TESTS: Array<[string, string]> = [
164164
[`echo foo`, `echo foo`],
165+
[`echo parapapa`, `echo parapapa`],
166+
[`echo 'foo bar'`, `echo 'foo bar'`],
167+
[`echo "foo' bar'"`, `echo "foo' bar'"`],
165168
[`echo foo; echo bar`, `echo foo; echo bar`],
166169
[`echo foo; echo bar;`, `echo foo; echo bar`],
167170
[`echo foo &`, `echo foo &`],
@@ -180,7 +183,7 @@ const STRINGIFIER_TESTS: Array<[string, string]> = [
180183
[`FOO=bar echo foo`, `FOO=bar echo foo`],
181184
[`FOO=bar BAZ=qux`, `FOO=bar BAZ=qux`],
182185
[`FOO=$'\\x09'`, `FOO=$'\\t'`],
183-
[`FOO=$'\\u0027'`, `FOO=$'\\''`],
186+
[`FOO=$'\\u0027'`, `FOO="'"`],
184187
[`FOO=$'\\U0001F601'`, `FOO=😁`],
185188
];
186189

0 commit comments

Comments
 (0)