Skip to content

Harden URL pathname normalization to collapse multiple leading slashes#15717

Merged
matthewp merged 3 commits intomainfrom
double-slash-a2
Mar 2, 2026
Merged

Harden URL pathname normalization to collapse multiple leading slashes#15717
matthewp merged 3 commits intomainfrom
double-slash-a2

Conversation

@matthewp
Copy link
Contributor

@matthewp matthewp commented Mar 2, 2026

Changes

  • Collapses multiple leading slashes (e.g. //admin/admin) in removeBase() and #createNormalizedUrl()
  • Ensures middleware context.url.pathname always reflects the canonical single-slash form that the router uses for matching

Testing

New unit test suite in packages/astro/test/units/app/double-slash-bypass.test.js with 6 tests

Docs

N/A, bug fix

@changeset-bot
Copy link

changeset-bot bot commented Mar 2, 2026

🦋 Changeset detected

Latest commit: 1e4c8b5

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Mar 2, 2026
@codspeed-hq
Copy link

codspeed-hq bot commented Mar 2, 2026

Merging this PR will not alter performance

✅ 18 untouched benchmarks


Comparing double-slash-a2 (1e4c8b5) with main (1118ac4)1

Open in CodSpeed

Footnotes

  1. No successful run was found on main (4b54eb5) during the generation of this report, so 1118ac4 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@matthewp matthewp marked this pull request as ready for review March 2, 2026 15:50
Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

@matthewp
Copy link
Contributor Author

matthewp commented Mar 2, 2026

Oh nice, will update.

@matthewp
Copy link
Contributor Author

matthewp commented Mar 2, 2026

@ematipico That's for trailing slashes, this is for leading. I'll add a similar utility.

@matthewp
Copy link
Contributor Author

matthewp commented Mar 2, 2026

@ematipico added in 0fd0b2f

Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

Cheers!

'astro': patch
---

Hardens URL pathname normalization in the SSR request pipeline to collapse multiple leading slashes before routing and middleware execution
Copy link
Member

Choose a reason for hiding this comment

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

Please update this changeset so that it's more use-facing. What we fixed by showing a possible use case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to avoid this coming across as alarmist; users should be checking these things in their own middleware, but it's good if we can prevent cases where they do not.

Happy to reword but not sure how to do so.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll give it a try.

@matthewp matthewp self-assigned this Mar 2, 2026
@matthewp matthewp merged commit 4000aaa into main Mar 2, 2026
26 checks passed
@matthewp matthewp deleted the double-slash-a2 branch March 2, 2026 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg: astro Related to the core `astro` package (scope)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants