fix: refine dotenv serialization for special characters handling#7592
Conversation
Values containing # characters were being truncated when saved to .env files because the dotenv parser interprets # as a comment character. This fix adds a shared jsonToDotenv utility in bruno-common that properly quotes values containing special characters (#, \n, ", ', \) to ensure they are preserved through serialization and parsing. - Add jsonToDotenv utility with comprehensive test coverage - Update bruno-electron and bruno-app to use shared utility - Remove duplicate serialization logic from multiple locations Fixes usebruno#7375 Fixes usebruno#7327
Updated the jsonToDotenv utility to improve quoting strategies for values containing special characters. Single quotes are now used for values with hashes (#) that do not contain single quotes, while double quotes are applied when both hashes and single quotes are present. Additionally, values with newlines and carriage returns are properly escaped. This ensures accurate preservation of values during serialization and parsing, addressing issues with previous handling of special characters. - Adjusted quoting logic in jsonToDotenv - Enhanced test coverage for various special character scenarios - Ensured round-trip consistency for values with backslashes and quotes
… in .env serialization The dotenv parser does not unescape \\ or \" in double-quoted values, causing backslashes and double quotes to grow on every save. Also, values containing #, ', and " were being corrupted due to premature quote termination. - Use unquoted for values with \, ", ' (dotenv preserves them as-is) - Use single quotes for values with # (fully literal) - Use backtick quotes for values with # and ' (fully literal) - Reserve double quotes only for actual newlines/carriage returns - Fix rawToVariables to match the new quoting strategy
…r-escaping # Conflicts: # packages/bruno-common/src/utils/jsonToDotenv.spec.ts # packages/bruno-common/src/utils/jsonToDotenv.ts
WalkthroughThe PR changes dotenv parsing to treat single-quoted and backtick-quoted values as literal (strip quotes only), narrows double-quoted unescaping to Changes
Sequence Diagram(s)sequenceDiagram
participant JSON as JSON variables
participant Serializer as jsonToDotenv
participant File as .env text
participant Parser as rawToVariables
participant App as DotEnv Editor
JSON->>Serializer: provide variables[]
Serializer->>File: produce dotenv string (choose quoting)
File->>Parser: parse lines (detect quoting type)
Parser->>App: return parsed variables (single/backtick literal, double unescape)
Note over Serializer,Parser: round-trip expected idempotency for backslashes/quotes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bruno-app/src/components/Environments/DotEnvFileEditor/utils.js`:
- Around line 37-40: The code is currently unescaping \" in the double-quoted
value branch, which violates the dotenv contract — remove the unescape for
escaped quotes so double-quoted values only expand \n and \r; in the function
that processes double-quoted values (the branch that does value = value.slice(1,
-1); value = value.replace(...)), delete the .replace(/\\"/g, '"') step and keep
only the replacements for \\n and \\r so rawToVariables (or the value-processing
logic in utils.js) preserves literal backslash-quote sequences.
In `@packages/bruno-common/src/utils/jsonToDotenv.ts`:
- Around line 55-56: The current jsonToDotenv generation returns unquoted values
(`return \`${v.name}=${value}\``) which loses leading/trailing whitespace
because dotenv trims unquoted values; update the quoting logic in the
jsonToDotenv function so that before falling back to the unquoted return you
detect values with leading or trailing whitespace and wrap them in quotes,
preferring single quotes, then backticks, then escaped double quotes (e.g.
choose `'${value}'` if it contains no single quote, else `` `${value}` `` if
safe, else use an escaped double-quoted form), and place this branch before the
unquoted fallback that uses `v.name`/`value` so those edge-case values are
preserved when parsed by dotenv.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3fc63e92-2261-4a77-9654-49a995e2727d
📒 Files selected for processing (3)
packages/bruno-app/src/components/Environments/DotEnvFileEditor/utils.jspackages/bruno-common/src/utils/jsonToDotenv.spec.tspackages/bruno-common/src/utils/jsonToDotenv.ts
| } else if (value.startsWith('"') && value.endsWith('"')) { | ||
| // Double-quoted values: unescape \", \n, and \r (the escapes we produce) | ||
| value = value.slice(1, -1); | ||
| value = value.replace(/\\"/g, '"').replace(/\\n/g, '\n').replace(/\\r/g, '\r'); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file under review to understand the full context
find . -name "utils.js" -path "*DotEnvFileEditor*" -type fRepository: usebruno/bruno
Length of output: 132
🏁 Script executed:
# Let's also find the test file mentioned as the oracle
find . -name "jsonToDotenv.spec.ts" -type fRepository: usebruno/bruno
Length of output: 112
🏁 Script executed:
# Search for dotenv usage and imports in the codebase
rg "dotenv\.parse|require.*dotenv|import.*dotenv" -t js -t ts --max-count 20Repository: usebruno/bruno
Length of output: 1385
🏁 Script executed:
# Read the utils.js file to see the full context around lines 37-40
cat -n packages/bruno-app/src/components/Environments/DotEnvFileEditor/utils.jsRepository: usebruno/bruno
Length of output: 2002
🏁 Script executed:
# Read the test file to understand the expected behavior
cat -n packages/bruno-common/src/utils/jsonToDotenv.spec.tsRepository: usebruno/bruno
Length of output: 10154
🏁 Script executed:
# Find the jsonToDotenv implementation
find . -name "jsonToDotenv.ts" -o -name "jsonToDotenv.js"Repository: usebruno/bruno
Length of output: 107
🏁 Script executed:
# Read the jsonToDotenv implementation
cat -n packages/bruno-common/src/utils/jsonToDotenv.tsRepository: usebruno/bruno
Length of output: 2680
🏁 Script executed:
# Let's verify the actual behavior of dotenv.parse() with escaped quotes
# by checking if there's any test that shows this scenario
rg "\\\\\"" packages/bruno-common/src/utils/jsonToDotenv.spec.tsRepository: usebruno/bruno
Length of output: 40
🏁 Script executed:
# Let's also check the jsonToDotenv.ts more carefully for when it produces escaped quotes
grep -n "\\\\\"" packages/bruno-common/src/utils/jsonToDotenv.tsRepository: usebruno/bruno
Length of output: 117
🏁 Script executed:
# Search for any documentation or comments about dotenv behavior in the codebase
rg "dotenv.*only.*\\\\n.*\\\\r" -A 2 -B 2 -t ts -t jsRepository: usebruno/bruno
Length of output: 40
🏁 Script executed:
# Let's verify the official dotenv behavior by checking if there are any existing examples
# in the codebase that show what dotenv.parse() actually does with escaped quotes
rg "say.*hello" packages/bruno-common/src/utils/jsonToDotenv.spec.ts -B 2 -A 2Repository: usebruno/bruno
Length of output: 1238
🏁 Script executed:
# Check if there are any tests that would break if we remove the \" unescaping
rg "\\\\\"" packages/bruno-common/src/utils/jsonToDotenv.spec.ts -B 3 -A 3Repository: usebruno/bruno
Length of output: 40
🏁 Script executed:
# Let's also verify the exact behavior by looking at what jsonToDotenv produces in the edge case
sed -n '40,60p' packages/bruno-common/src/utils/jsonToDotenv.tsRepository: usebruno/bruno
Length of output: 828
🏁 Script executed:
# Let me check if there's a test case covering the extreme scenario
# where jsonToDotenv escapes quotes (value with #, ', and `)
rg -A 5 "extremely rare" packages/bruno-common/src/utils/jsonToDotenv.tsRepository: usebruno/bruno
Length of output: 40
🏁 Script executed:
# Let's verify what the actual official dotenv package does with escaped quotes
# by checking the package version and any inline documentation
cat packages/bruno-common/package.json | grep -A 2 -B 2 '"dotenv"'Repository: usebruno/bruno
Length of output: 40
🏁 Script executed:
# Search for any explicit tests or documentation about dotenv behavior with \"
rg "dotenv.*escape|escape.*dotenv|\\\\\\\\\"" -t ts -t js packages/Repository: usebruno/bruno
Length of output: 2581
🏁 Script executed:
# Search for any test that exercises the extreme edge case where jsonToDotenv escapes quotes
rg "value.*#.*'.*\`" packages/bruno-common/src/utils/jsonToDotenv.spec.tsRepository: usebruno/bruno
Length of output: 40
🏁 Script executed:
# Let's create a simple test to verify the actual behavior of dotenv.parse() with escaped quotes
# by checking if there's a Node.js environment available
python3 << 'EOF'
import subprocess
import json
# Let's check if we can understand the dotenv behavior from package info
result = subprocess.run(['cat', 'packages/bruno-common/package.json'],
capture_output=True, text=True)
content = json.loads(result.stdout)
dotenv_version = content.get('dependencies', {}).get('dotenv', 'unknown')
print(f"dotenv version: {dotenv_version}")
EOFRepository: usebruno/bruno
Length of output: 81
🏁 Script executed:
# Search for the actual test that covers the extreme case
rg "hash.*single.*quote.*backtick|#.*'.*\`" packages/bruno-common/src/utils/jsonToDotenv.spec.ts -B 5 -A 5Repository: usebruno/bruno
Length of output: 40
🏁 Script executed:
# Let me verify if there's a test that would break if we remove the \" unescaping
# by checking if the extreme case is tested
grep -n "it's.*#.*complex" packages/bruno-common/src/utils/jsonToDotenv.spec.ts -B 2 -A 8Repository: usebruno/bruno
Length of output: 40
🏁 Script executed:
# Let's also check if rawToVariables is actually tested anywhere
rg "rawToVariables" packages/Repository: usebruno/bruno
Length of output: 453
🏁 Script executed:
# Let's see if there's a test for the round-trip through the editor parser
rg "DotEnvFileEditor" packages/ --type ts --type jsRepository: usebruno/bruno
Length of output: 840
🏁 Script executed:
# Let me verify the actual behavior by checking the official dotenv package specification
# Search for any inline documentation about escape handling
rg "unescape|escape" packages/bruno-common/src/utils/jsonToDotenv.ts -B 2 -A 2Repository: usebruno/bruno
Length of output: 774
🏁 Script executed:
# Let's also check if there are any tests in the codebase that would fail if we remove the \" unescaping
# by searching for tests related to rawToVariables
find . -name "*.spec.js" -o -name "*.spec.ts" | xargs grep -l "rawToVariables" 2>/dev/nullRepository: usebruno/bruno
Length of output: 42
🏁 Script executed:
# Let's check what the comment in jsonToDotenv.ts actually says about escaping
sed -n '11,22p' packages/bruno-common/src/utils/jsonToDotenv.tsRepository: usebruno/bruno
Length of output: 615
🏁 Script executed:
# Critical verification: Let's check what dotenv actually does
# by searching for any existing test that exercises the extreme case of value with #, ', and backtick
rg "it's.*#.*complex|complex.*it's" packages/bruno-common/src/utils/jsonToDotenv.spec.ts -B 3 -A 5Repository: usebruno/bruno
Length of output: 40
🏁 Script executed:
# Let me check the exact test case on line 157-162
sed -n '157,162p' packages/bruno-common/src/utils/jsonToDotenv.spec.tsRepository: usebruno/bruno
Length of output: 396
🏁 Script executed:
# Let me verify the key docstring claim by checking if there's any test case
# that would have jsonToDotenv produce escaped quotes and then parse them back
python3 << 'EOF'
# Simulate the extreme case: value with #, ', and backtick
test_value = "it's#`complex`"
print(f"Test value: {test_value}")
print(f"Has #: {test_value.count('#') > 0}")
print(f"Has ': {test_value.count(\"'\") > 0}")
print(f"Has backtick: {test_value.count('`') > 0}")
print("\nIn jsonToDotenv logic:")
print("1. Has #? Yes")
print("2. Has '? Yes")
print("3. Has backtick? Yes")
print("-> Would escape \" and use double quotes")
print(f"-> Output would be: FOO=\"{test_value.replace('\"', '\\\\\"')}\"")
EOFRepository: usebruno/bruno
Length of output: 249
🏁 Script executed:
# Now let's verify the test - does it actually test the extreme case?
grep -n "backtick\|\\`" packages/bruno-common/src/utils/jsonToDotenv.spec.tsRepository: usebruno/bruno
Length of output: 126
🏁 Script executed:
# Let me verify by examining if there's actually a test for the extreme case
sed -n '50,107p' packages/bruno-common/src/utils/jsonToDotenv.spec.ts | grep -A 5 "backtick"Repository: usebruno/bruno
Length of output: 594
🏁 Script executed:
# Check if there's any test that would produce escaped quotes in the output
sed -n '50,107p' packages/bruno-common/src/utils/jsonToDotenv.spec.tsRepository: usebruno/bruno
Length of output: 2515
🏁 Script executed:
# Let me verify if there's a test that would break if we remove the \" unescaping
# by checking what happens in the round-trip tests
sed -n '109,216p' packages/bruno-common/src/utils/jsonToDotenv.spec.ts | grep -B 3 -A 5 "round-trip\|COMPLEX"Repository: usebruno/bruno
Length of output: 3450
🏁 Script executed:
# Critical check: Does jsonToDotenv ever produce escaped quotes in practice?
# by checking if any serialized output contains \"
grep -n "\\\\\\\\\"" packages/bruno-common/src/utils/jsonToDotenv.spec.tsRepository: usebruno/bruno
Length of output: 40
🏁 Script executed:
# Let me trace through the logic: when does jsonToDotenv produce escaped quotes?
# It's in the extreme case - let's see if there's a test that covers this path
python3 << 'EOFPYTHON'
# The extreme case: value has #, ', and backtick
test_cases = [
("simple", False, False, False),
("has#hash", True, False, False),
("has'quote", False, True, False),
("has`backtick", False, False, True),
("has'#both", True, True, False),
("has'`both", False, True, True),
("has#`both", True, False, True),
("has'#`all", True, True, True), # This triggers the escaped quotes path
]
for value, has_hash, has_single, has_backtick in test_cases:
if has_hash:
if not has_single:
result = f"'{value}'"
elif not has_backtick:
result = f"`{value}`"
else:
result = f'"{value.replace(chr(34), chr(92) + chr(34))}"' # Escape quotes
else:
result = value
print(f"{value:20} -> {result}")
EOFPYTHONRepository: usebruno/bruno
Length of output: 336
🏁 Script executed:
# Let's check if there's any test that would have failed if the escape logic was wrong
rg "escaped.*quote|quote.*escape" packages/bruno-common/src/utils/jsonToDotenv.spec.tsRepository: usebruno/bruno
Length of output: 309
🏁 Script executed:
# Verify the exact docstring claim about dotenv behavior
sed -n '11,22p' packages/bruno-common/src/utils/jsonToDotenv.tsRepository: usebruno/bruno
Length of output: 615
🏁 Script executed:
# Final verification: check if there's any test that would fail if we remove the \" unescaping
# and verify the exact behavior claimed in the docstring
sed -n '157,162p' packages/bruno-common/src/utils/jsonToDotenv.spec.tsRepository: usebruno/bruno
Length of output: 396
🏁 Script executed:
# Check if there's a test that exercises the extreme case that produces escaped quotes
python3 << 'EOFPYTHON'
# The extreme case path in jsonToDotenv that produces escaped quotes
# happens only when: has # AND has ' AND has backtick
# Looking at tests, the closest is line 66-70: has ' and #, but also has "
# But that value is 'it\'s#"complex"' which doesn't have backtick
# So the path that produces \" is never tested
print("Case analysis:")
print("1. Value: it's#complex -> backtick quoted (no escape)")
print("2. Value: it's#\"complex\" -> backtick quoted (no escape)")
print("3. Value: it's#`complex` -> would need \" escaping (NEVER TESTED)")
print("\nConclusion: The path that produces \" is untested")
EOFPYTHONRepository: usebruno/bruno
Length of output: 297
Remove the \" unescaping from line 40.
The dotenv parser contract documented in packages/bruno-common/src/utils/jsonToDotenv.ts (lines 11–22) explicitly states that in double-quoted values, only \n and \r are expanded; everything else—including \"—is treated as literal. Unescaping \" here breaks round-trip compatibility: if jsonToDotenv produces escaped quotes (in the extreme case where a value contains #, ', and backtick), and dotenv.parse() leaves them as literal backslash-quote, then rawToVariables must do the same.
Suggested change
- value = value.replace(/\\"/g, '"').replace(/\\n/g, '\n').replace(/\\r/g, '\r');
+ value = value.replace(/\\n/g, '\n').replace(/\\r/g, '\r');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bruno-app/src/components/Environments/DotEnvFileEditor/utils.js`
around lines 37 - 40, The code is currently unescaping \" in the double-quoted
value branch, which violates the dotenv contract — remove the unescape for
escaped quotes so double-quoted values only expand \n and \r; in the function
that processes double-quoted values (the branch that does value = value.slice(1,
-1); value = value.replace(...)), delete the .replace(/\\"/g, '"') step and keep
only the replacements for \\n and \\r so rawToVariables (or the value-processing
logic in utils.js) preserves literal backslash-quote sequences.
dotenv trims whitespace from unquoted values, so values like " hello " would silently lose their whitespace on round-trip. Now such values are quoted using the same priority chain (single → backtick → double).
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/bruno-common/src/utils/jsonToDotenv.spec.ts (1)
66-70: Consider adding test for extreme edge case with all quote types.The implementation has a fallback path for values containing
#,',", AND`which would trigger double-quoting with escaped". This path isn't covered by existing tests.🧪 Optional: Add edge case test
+ test('it should double-quote values containing hash, single quote, double quote, and backtick', () => { + const variables = [{ name: 'EXTREME', value: 'it\'s#"complex`value"' }]; + const output = jsonToDotenv(variables); + expect(output).toBe('EXTREME="it\'s#\\"complex`value\\""'); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-common/src/utils/jsonToDotenv.spec.ts` around lines 66 - 70, The test suite for jsonToDotenv is missing an edge-case that triggers the fallback branch where a value contains #, single quote ('), double quote ("), and backtick (`); add a new unit test in jsonToDotenv.spec.ts that calls jsonToDotenv with a variable whose value includes all four characters to exercise the fallback path in jsonToDotenv and assert the result uses double-quoting with internal double quotes escaped (i.e., the variable is wrapped in "..." and any " inside the value are escaped) to match the function's intended output.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/bruno-common/src/utils/jsonToDotenv.spec.ts`:
- Around line 66-70: The test suite for jsonToDotenv is missing an edge-case
that triggers the fallback branch where a value contains #, single quote ('),
double quote ("), and backtick (`); add a new unit test in jsonToDotenv.spec.ts
that calls jsonToDotenv with a variable whose value includes all four characters
to exercise the fallback path in jsonToDotenv and assert the result uses
double-quoting with internal double quotes escaped (i.e., the variable is
wrapped in "..." and any " inside the value are escaped) to match the function's
intended output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 179bb26c-47d3-44ae-8e18-b8cb16bf9850
📒 Files selected for processing (2)
packages/bruno-common/src/utils/jsonToDotenv.spec.tspackages/bruno-common/src/utils/jsonToDotenv.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bruno-common/src/utils/jsonToDotenv.ts
|
Query builder tests fixed in #7618, ignoring them for merge |
Description
Updated the jsonToDotenv utility to improve quoting strategies for values containing special characters. Single quotes are now used for values with hashes (#) that do not contain single quotes, while double quotes are applied when both hashes and single quotes are present. Additionally, values with newlines and carriage returns are properly escaped. This ensures accurate preservation of values during serialization and parsing, addressing issues with previous handling of special characters.
Adjusted quoting logic in jsonToDotenv
Enhanced test coverage for various special character scenarios
Ensured round-trip consistency for values with backslashes and quotes
JIRA: https://usebruno.atlassian.net/browse/BRU-2817
Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
Publishing to New Package Managers
Please see here for more information.
Summary by CodeRabbit
Bug Fixes
Tests