feat(headers-parser): replace dependencies with native APIs - #7147
Conversation
This replaces the following in `headers-parser`: - `is-plain-object` with a basic `typeof` check (as we are in control of the inputs) - `map-obj` with `Object.entries(obj).map(fn)` - `path-exists` with `fs.access` - `fast-safe-stringify` with `JSON.stringify` On the last point, `fast-safe-stringify` really exists to handle things like circular references when stringifying. However, we don't use that functionality and only use the `stableStringify` export. We use the `stableStringify` export to basically sort the keys so we can have a stable string representation, and we handle `RegExp` by `toString`-ing it. All of this can be done without `fast-safe-stringify` as we're in full control of the type (`MinimalHeader`). So, this changes it to use `JSON.stringify` of the keys we know, and sorts the header `values` directly.
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe headers parser removes several utility dependencies, adds local object validation and value normalization, and changes duplicate detection to use deterministic JSON keys. Header and Netlify configuration parsing now uses Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
e18e dependency analysisNo dependency warnings found. |
@netlify/build
@netlify/build-info
@netlify/cache-utils
@netlify/config
@netlify/edge-bundler
@netlify/functions-utils
@netlify/git-utils
@netlify/headers-parser
@netlify/api
@netlify/nock-udp
@netlify/opentelemetry-sdk-setup
@netlify/opentelemetry-utils
@netlify/redirect-parser
@netlify/run-utils
@netlify/zip-it-and-ship-it
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/headers-parser/src/line_parser.ts`:
- Around line 24-26: Replace the fs.access preflight in the header parsing flow
at packages/headers-parser/src/line_parser.ts:24-26 with direct-read error
handling, allowing the existing read/TOML parse path to determine empty-result
behavior. Apply the same change to the corresponding preflight in
packages/headers-parser/src/netlify_config_parser.ts:12-16; remove both access
checks without changing successful parsing behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b53f7312-f17e-4983-9e72-dc09fe508439
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
packages/headers-parser/package.jsonpackages/headers-parser/src/line_parser.tspackages/headers-parser/src/merge.tspackages/headers-parser/src/netlify_config_parser.tspackages/headers-parser/src/normalize.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
netlify/blueprints(manual)
| try { | ||
| await fs.access(headersFile) | ||
| } catch { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
fd -p 'packages/headers-parser/src/(line_parser|netlify_config_parser)\.ts$' . || true
echo "== relevant line_parser.ts =="
if [ -f packages/headers-parser/src/line_parser.ts ]; then
nl -ba packages/headers-parser/src/line_parser.ts | sed -n '1,80p'
fi
echo "== relevant netlify_config_parser.ts =="
if [ -f packages/headers-parser/src/netlify_config_parser.ts ]; then
nl -ba packages/headers-parser/src/netlify_config_parser.ts | sed -n '1,120p'
fi
echo "== search fs.access and parseConfig usages =="
rg -n "fs\.access|parseConfig|splitResults|parseHeadersFile|read text|readFileSync" packages/headers-parser/src packages/headers-parser -S || trueRepository: netlify/build
Length of output: 340
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package files mentioning headers-parser =="
rg -n "headers-parser|line_parser|netlify_config_parser|parseHeadersFile" package.json pnpm-workspace.yaml packages -S || true
echo "== TypeScript fs access definitions availability =="
pnpm list fs-extra 2>/dev/null | head -20 || true
rg -n "from '(fs-extra|fs)'|require\('fs'|require\('fs-extra'\)" packages/headers-parser/src/tsconfig.json -S || trueRepository: netlify/build
Length of output: 15650
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== line_parser.ts relevant sections =="
awk 'NR>=1 && NR<=120 { printf "%4d %s\n", NR, $0 }' packages/headers-parser/src/line_parser.ts
echo "== netlify_config_parser.ts relevant sections =="
awk 'NR>=1 && NR<=160 { printf "%4d %s\n", NR, $0 }' packages/headers-parser/src/netlify_config_parser.ts
echo "== all fs.access/access/read usages in headers-parser source =="
python3 - <<'PY'
from pathlib import Path
for path in sorted(Path("packages/headers-parser/src").rglob("*.ts")):
text = path.read_text()
lines = text.splitlines()
for i, line in enumerate(lines, 1):
if "fs.access" in line or "access(" in line or "readTextFile" in line or "promises.readFile" in line:
start=max(1,i-3); end=min(len(lines),i+4)
print(f"\n--- {path}:{start}-{end} ---")
for n in range(start,end+1):
print(f"{n:4d} {lines[n-1]}")
PYRepository: netlify/build
Length of output: 6586
🌐 Web query:
Node.js fs.access default flags F_OK read permissions R_OK file permission error documentation
💡 Result:
In Node.js, the fs.access function is used to test a user's permissions for a file or directory [1][2]. The mode argument is an optional integer that specifies the accessibility checks to be performed, with a default value of fs.constants.F_OK [3][4]. Key details regarding fs.access constants and usage: Default Behavior: The default value for the mode argument is fs.constants.F_OK, which indicates that the file is visible to the calling process (i.e., it exists) [3][4]. Permission Constants: The mode argument can also be a bitwise OR of any of the following constants: fs.constants.F_OK: Visible to the calling process [3][4]. fs.constants.R_OK: Readable by the calling process [1][5]. fs.constants.W_OK: Writable by the calling process [1][5]. fs.constants.X_OK: Executable by the calling process [1][5]. Error Handling: If the accessibility check fails, the function returns a rejected promise (when using fsPromises.access) or invokes the callback with an error object (when using fs.access) [1][2]. Important Recommendation: Node.js documentation explicitly advises against using fs.access to check for accessibility before performing operations like fs.open, fs.readFile, or fs.writeFile [1][2]. Doing so creates a race condition, as the file's state could change between the access check and the actual operation [1][6]. Instead, it is best practice to attempt the operation directly and handle any errors that occur [1][4]. Additionally, users should be aware that on Windows, fs.access does not check Access Control Lists (ACLs), so it may return success even if ACLs would otherwise restrict the user from reading or writing to the file [7][8].
Citations:
- 1: https://nodejs.org/api/fs.html
- 2: https://nodejs.org/docs/latest-v24.x/api/fs.html
- 3: https://nodejs.org/api/fs.md
- 4: https://github.com/nodejs/node/blob/main/doc/api/fs.md
- 5: https://nodejs.org/docs/latest-v18.x/api/fs.html
- 6: https://nodejs.org/docs/latest-v23.x/api/fs.html
- 7: https://nodejs.org/docs/latest-v22.x/api/fs.html
- 8: https://nodejs.org/docs/v22.14.0/api/fs.html
Replace the fs.access preflight with direct-read error handling.
These checks default to F_OK, so they don’t reliably detect unreadable files and still race with the later fs.readFile/TOML parse calls. Remove the preflight and let the actual read/parse path handle the intended empty-result behavior consistently.
📍 Affects 2 files
packages/headers-parser/src/line_parser.ts#L24-L26(this comment)packages/headers-parser/src/netlify_config_parser.ts#L12-L16
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/headers-parser/src/line_parser.ts` around lines 24 - 26, Replace the
fs.access preflight in the header parsing flow at
packages/headers-parser/src/line_parser.ts:24-26 with direct-read error
handling, allowing the existing read/TOML parse path to determine empty-result
behavior. Apply the same change to the corresponding preflight in
packages/headers-parser/src/netlify_config_parser.ts:12-16; remove both access
checks without changing successful parsing behavior.
1 participant
This replaces the following in
headers-parser:is-plain-objectwith a basictypeofcheck (as we are in control of the inputs)map-objwithObject.entries(obj).map(fn)path-existswithfs.accessfast-safe-stringifywithJSON.stringifyOn the last point,
fast-safe-stringifyreally exists to handle things like circular references when stringifying. However, we don't use that functionality and only use thestableStringifyexport.We use the
stableStringifyexport to basically sort the keys so we can have a stable string representation, and we handleRegExpbytoString-ing it.All of this can be done without
fast-safe-stringifyas we're in full control of the type (MinimalHeader). So, this changes it to useJSON.stringifyof the keys we know, and sorts the headervaluesdirectly.For us to review and ship your PR efficiently, please perform the following steps:
we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or
something that`s on fire 🔥 (e.g. incident related), you can skip this step.
your code follows our style guide and passes our tests.
A picture of a cute animal (not mandatory, but encouraged)