normalize HMR upgrade path comparison#5678
Conversation
…e configured path Adds trailing slash, case variations, percent-encoded path, and leading double slash to the existing HMR/proxy isolation tests so that all URL shapes recognized as the HMR upgrade are treated consistently.
Tighten the HMR-vs-proxy upgrade dispatch so URL variants of the
configured HMR path are consistently recognized as the HMR socket:
- lowercase comparison (case-insensitive)
- decode percent-encoding before comparison
- strip trailing slashes
- collapse leading multiple slashes so //foo is treated as /foo
instead of being parsed as a scheme-relative URL
- wrap URL parsing in try/catch so a malformed req.url does not
raise an uncaught exception
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5678 +/- ##
==========================================
- Coverage 83.60% 83.60% -0.01%
==========================================
Files 13 13
Lines 2086 2098 +12
Branches 773 778 +5
==========================================
+ Hits 1744 1754 +10
- Misses 306 308 +2
Partials 36 36 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/cc @bjohansebas can you take a look, thanks |
| const normalizeHmrPath = (pathToNormalize) => { | ||
| try { | ||
| return decodeURIComponent(pathToNormalize) | ||
| .toLowerCase() |
There was a problem hiding this comment.
I think the routes should be case-sensitive, just like Express handles them by default.
There was a problem hiding this comment.
The comment also comes from how ws handles its internal server, and partly from how I remember Express handling routes.
| ["uppercase", "/WS"], | ||
| ["mixed case", "/wS"], | ||
| ["percent-encoded path", "/%77%73"], |
There was a problem hiding this comment.
All of these should go to the proxy if we implement case-sensitive routes.
| */ | ||
| const normalizeHmrPath = (pathToNormalize) => { | ||
| try { | ||
| return decodeURIComponent(pathToNormalize) |
There was a problem hiding this comment.
By normalizing this ourselves, how much would it affect performance? It might be worth running some benchmarks, just as a side note, since I don't remember whether decodeURIComponent is expensive performance-wise.
3 participants
No description provided.