-
-
Notifications
You must be signed in to change notification settings - Fork 780
fix(cloudflare): Filter inherited values from Cloudflare preset _headers file #3938
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.
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
base: v2
Are you sure you want to change the base?
Conversation
|
@gapple is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
π WalkthroughWalkthroughA tree-based header processing system is introduced for the Cloudflare preset. A new Changes
Estimated code review effortπ― 3 (Moderate) | β±οΈ ~25 minutes π₯ Pre-merge checks | β 2 | β 1β Failed checks (1 warning)
β Passed checks (2 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 |
|
@coderabbitai review |
β Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
π€ Fix all issues with AI agents
In `@src/presets/cloudflare/utils.ts`:
- Around line 130-134: When creating a new child node in the header tree,
initialize all required CloudflareHeaderTreeNode fields: include wildcardHeaders
and headers (both as empty arrays/objects as appropriate) along with path and
children; update the node creation at currentNode.children[part] to set path,
children, wildcardHeaders, and headers so the new node conforms to
CloudflareHeaderTreeNode and passes strict TS type-checking.
| currentNode.children[part] = currentNode.children[part] || { | ||
| path: (currentNode.path == "/" ? "" : currentNode.path) + "/" + part, | ||
| children: {}, | ||
| }; | ||
| currentNode = currentNode.children[part]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initialize required fields for child nodes.
CloudflareHeaderTreeNode requires wildcardHeaders and headers; omitting them will fail type-checking in strict TS setups.
π οΈ Proposed fix
currentNode.children[part] = currentNode.children[part] || {
path: (currentNode.path == "/" ? "" : currentNode.path) + "/" + part,
+ wildcardHeaders: undefined,
+ headers: undefined,
children: {},
};π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| currentNode.children[part] = currentNode.children[part] || { | |
| path: (currentNode.path == "/" ? "" : currentNode.path) + "/" + part, | |
| children: {}, | |
| }; | |
| currentNode = currentNode.children[part]; | |
| currentNode.children[part] = currentNode.children[part] || { | |
| path: (currentNode.path == "/" ? "" : currentNode.path) + "/" + part, | |
| wildcardHeaders: undefined, | |
| headers: undefined, | |
| children: {}, | |
| }; | |
| currentNode = currentNode.children[part]; |
π€ Prompt for AI Agents
In `@src/presets/cloudflare/utils.ts` around lines 130 - 134, When creating a new
child node in the header tree, initialize all required CloudflareHeaderTreeNode
fields: include wildcardHeaders and headers (both as empty arrays/objects as
appropriate) along with path and children; update the node creation at
currentNode.children[part] to set path, children, wildcardHeaders, and headers
so the new node conforms to CloudflareHeaderTreeNode and passes strict TS
type-checking.
π Linked issue
#2909
Baroshem/nuxt-security#575
β Type of change
π Description
Cloudflare's
_headerfile merges all rules that apply to a path, resulting in duplicated or conflicting values. This may prevent a browser from parsing the header value, or to interpret the value incorrectly.https://developers.cloudflare.com/workers/static-assets/headers/#attach-a-header
Examples
Nuxt provides some nested wildcard path rules. Responses at increasing path depth receive successively appended values.
A route matching
/_nuxt/builds/meta/*will receive all three values appended:nuxt-security defines some headers to apply to all paths, and also defines those values for each prerendered page.
Will result in the
/path receiving the header values:The change
This PR first builds all routeRules that include headers into a tree before processing the values to be output. This allows checking path rules against matching wildcard rules in order to either:
With this PR the above examples output:
π Checklist