Skip to content

fix(dotenv): handle values with both quote types and escape sequences #7139

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

Open
algojogacor wants to merge 1 commit into
base: main
Choose a base branch
from
+78 −7
Open

fix(dotenv): handle values with both quote types and escape sequences #7139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 4 additions & 2 deletions dotenv/parse.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ type LineParseResult = {
};

const KEY_VALUE_REGEXP =
/^\s*(?:export\s+)?(?<key>[^\s=#]+?)\s*=[\ \t]*('\r?\n?(?<notInterpolated>(.|\r\n|\n)*?)\r?\n?'|"\r?\n?(?<interpolated>(.|\r\n|\n)*?)\r?\n?"|(?<unquoted>[^\r\n#]*)) *#*.*$/gm;
/^\s*(?:export\s+)?(?<key>[^\s=#]+?)\s*=[\ \t]*('\r?\n?(?<notInterpolated>(?:.|\r\n|\n)*?)\r?\n?'|"\r?\n?(?<interpolated>(?:[^"\\]|\\.)*)\r?\n?"|(?<unquoted>[^\r\n#]*)) *#*.*$/gm;
Copy link
Copy Markdown
Member

@bartlomieju bartlomieju May 26, 2026

Choose a reason for hiding this comment

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

Regression here. Switching the inner pattern from non-greedy (.|\r\n|\n)*? to greedy (?:[^"\\]|\\.)* breaks PRIVATE_KEY_DOUBLE_QUOTED in testdata/.env.test.

In JS regex, [^"\\] is a character class — it matches \n by default. So the greedy * happily consumes the trailing newline inside the quotes, leaving nothing for the outer \r?\n? to strip. Parsed value ends with an extra \n.

Two possible fixes:

  • Keep the new shape but exclude newlines from the negated class: (?:[^"\\\r\n]|\\.|\r\n|\n)* (and keep it non-greedy, or restructure so the outer \r?\n? still wins).
  • Or use (?:[^"\\]|\\.)*? (non-greedy) so the outer \r?\n? can still match the trailing newline.

Also a smaller edge case: \\. won't cross an actual \n (the . doesn't match newlines without the s flag), so a literal backslash immediately before a real newline inside a double-quoted value will fail to parse. Unlikely in practice but worth a test.


const VALID_KEY_REGEXP = /^[a-zA-Z_][a-zA-Z0-9_]*$/;

Expand All @@ -19,11 +19,13 @@ const CHARACTERS_MAP: { [key: string]: string } = {
"\\n": "\n",
"\\r": "\r",
"\\t": "\t",
'\\"': '"',
"\\\\": "\\",
};

function expandCharacters(str: string): string {
return str.replace(
/\\([nrt])/g,
/\\([nrt"\\])/g,
($1: keyof typeof CHARACTERS_MAP): string => CHARACTERS_MAP[$1] ?? "",
);
}
Expand Down
39 changes: 39 additions & 0 deletions dotenv/parse_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,3 +284,42 @@ Deno.test("parse() result is not affected by extended Object.prototype", () => {
assertEquals(result.foo, undefined);
assertEquals(result.bar, "1");
});

Deno.test("parse() round-trips through stringify()", async (t) => {
const { stringify } = await import("./stringify.ts");

await t.step("basic value", () => {
const original = { HELLO: "world" };
assertEquals(parse(stringify(original)), original);
});

await t.step("value with spaces", () => {
const original = { GREETING: "hello world" };
assertEquals(parse(stringify(original)), original);
});

await t.step("value with single quote", () => {
const original = { PARSE: "par'se" };
assertEquals(parse(stringify(original)), original);
});

await t.step("value with double quote (JSON-like)", () => {
const original = { JSON: '{"key":"value"}' };
assertEquals(parse(stringify(original)), original);
});

await t.step("value with both quote types", () => {
const original = { MIXED: `a'b"c` };
assertEquals(parse(stringify(original)), original);
});

await t.step("value with newline", () => {
const original = { MULTILINE: "hello\nworld" };
assertEquals(parse(stringify(original)), original);
});

await t.step("value with backslash and quotes", () => {
const original = { BS: String.raw`test\"value` };
assertEquals(parse(stringify(original)), original);
});
});
20 changes: 15 additions & 5 deletions dotenv/stringify.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
export function stringify(object: Record<string, string>): string {
const lines: string[] = [];
for (const [key, value] of Object.entries(object)) {
let quote;
let quote: string | undefined;

let escapedValue = value ?? "";
if (key.startsWith("#")) {
Expand All @@ -28,11 +28,21 @@ export function stringify(object: Record<string, string>): string {
`key starts with a '#' indicates a comment and is ignored: '${key}'`,
);
continue;
} else if (escapedValue.includes("\n") || escapedValue.includes("'")) {
// escape inner new lines
escapedValue = escapedValue.replaceAll("\n", "\\n");
}

const hasNewline = escapedValue.includes("\n");
const hasSingleQuote = escapedValue.includes("'");
const hasDoubleQuote = escapedValue.includes('"');

// Use double quotes when the value contains newlines (so they can be
// expanded back) or single quotes (which are safe inside double quotes).
Copy link
Copy Markdown
Member

@bartlomieju bartlomieju May 26, 2026

Choose a reason for hiding this comment

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

This is what causes your "handles backslash with double quotes" test to fail.

For a value containing " and \ but no ' or \n (e.g. String.raw\test\"value`), neither hasNewlinenorhasSingleQuoteis true, so we fall into this branch and pick'. The single-quote branch doesn't escape backslashes — round-trip happens to work only because parse()reads the single-quoted group *without*expandCharacters. But the stringified form is 'test\"value'`, not the double-quoted form your test asserts.

If you want the double-quoted form (matching the test), the condition needs to also trigger when the value contains " and \ (so escaping is meaningful). Otherwise, drop the failing test and keep the simpler single-quote routing. Either way, the code and tests need to agree.

if (hasNewline || hasSingleQuote) {
quote = `"`;
} else if (escapedValue.match(/\W/)) {
// Escape backslashes first so that existing backslashes are not
// confused with escape sequences when parsed.
escapedValue = escapedValue.replaceAll("\\", "\\\\");
if (hasNewline) escapedValue = escapedValue.replaceAll("\n", "\\n");
} else if (hasDoubleQuote || escapedValue.match(/\W/)) {
quote = "'";
}

Expand Down
20 changes: 20 additions & 0 deletions dotenv/stringify_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,4 +83,24 @@ Deno.test("stringify()", async (t) => {
stringify({ PARSE: "par'se" }),
`PARSE="par'se"`,
));
await t.step("handles double-quote characters", () =>
assertEquals(
stringify({ JSON: '{"key":"value"}' }),
`JSON='{"key":"value"}'`,
));
await t.step("handles both quote characters", () =>
assertEquals(
stringify({ MIXED: `a'b"c` }),
`MIXED="a'b\\"c"`,
));
await t.step("handles backslash with double quotes", () =>
assertEquals(
stringify({ BS: String.raw`test\"value` }),
`BS="test\\\\\\"value"`,
));
Copy link
Copy Markdown
Member

@bartlomieju bartlomieju May 26, 2026

Choose a reason for hiding this comment

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

Failing test — see the comment on stringify.ts. Once you've decided the desired routing for values with only " + \, either update this assertion or change the branch condition so it matches.

await t.step("handles newline with single quotes", () =>
assertEquals(
stringify({ NL: "hello\nit's me" }),
`NL="hello\\nit's me"`,
));
});
Loading