fix: parse # in absolute path directory names as part of the path#20980
Merged
Conversation
When an absolute path contains `#` followed by a path separator (e.g. `/home/user/proj#1/file.js`), treat the `#` as part of the directory name rather than a fragment marker. A real fragment cannot have a path separator after it before any query, so this disambiguation is sound and preserves existing fragment behavior for paths like `/abs/path/file.js#fragment`. Fixes #16819
Adds a unit test using the precise absolute path + query string that webpack-dev-server generates when serving a project located in a directory containing `#`, demonstrating that `_parseResource` now correctly splits path from query for this real-world case.
) When a project lives in a directory containing `#` (e.g. `~/proj#1/`) and a tool like webpack-dev-server passes an absolute entry path with a query string (e.g. `/abs/proj#1/file.js?protocol=ws`), the resolver mis-split the path at the first `#`, leading to "Module not found". Pre-escape `#` characters in the directory portion of absolute path requests as `\0#` (the existing enhanced-resolve escape) before passing to the resolver. A `#` after the last path separator is left alone so explicit fragment requests like `/abs/path/file.js#fragment` still behave the same. Adds an integration test that compiles a project located in a directory containing `#` with an absolute entry path + query (mirroring the webpack-dev-server scenario), and unit coverage for the new escape utility and the exact URL from the issue.
Relative requests with `#` in a directory name plus a query (e.g. `./proj#1/file.js?q=1`) hit the same resolver failure as absolute ones. Extend the pre-escape to cover both, but restrict it to requests that contain `?` — without a query the resolver's own fallback handles directory `#` correctly, and skipping it preserves legitimate fragment semantics like `./resourceFragment/index#/some/fragment`. Renames `escapeHashInAbsolutePath` to `escapeHashInPathRequest`, expands unit coverage to relative and Windows forms, and adds a companion configCases test using a relative entry from a `#` directory.
🦋 Changeset detectedLatest commit: 40aef01 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Contributor
|
This PR is packaged and the instant preview is available (865c051). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@865c051
yarn add -D webpack@https://pkg.pr.new/webpack@865c051
pnpm add -D webpack@https://pkg.pr.new/webpack@865c051 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #20980 +/- ##
==========================================
- Coverage 91.38% 90.92% -0.46%
==========================================
Files 568 573 +5
Lines 56486 58639 +2153
Branches 14999 15774 +775
==========================================
+ Hits 51621 53320 +1699
- Misses 4865 5319 +454
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates module request handling so # characters in directory names are escaped before normal resource resolution, addressing failures when paths with # also include query strings.
Changes:
- Adds
escapeHashInPathRequestinlib/util/identifier.js. - Applies the escaping in
NormalModuleFactorybefore resolving resources. - Adds unit/config tests and a patch changeset for issue #16819.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
lib/util/identifier.js |
Adds hash escaping helper for path-like requests with query strings. |
lib/NormalModuleFactory.js |
Uses the new helper before resolving unresolved resources. |
test/identifier.unittest.js |
Adds unit coverage for hash escaping behavior. |
test/configCases/parsing/issue-16819-#-in-path-#/webpack.config.js |
Adds absolute-path regression config. |
test/configCases/parsing/issue-16819-#-in-path-#/index.js |
Adds regression test entry. |
test/configCases/parsing/issue-16819-relative-#/webpack.config.js |
Adds relative-path regression config. |
test/configCases/parsing/issue-16819-relative-#/#dir/index.js |
Adds relative regression test entry. |
.changeset/parse-resource-hash-in-absolute-path.md |
Adds patch release note. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+518
to
+524
| const lastSep = Math.max( | ||
| request.lastIndexOf("/", queryStart - 1), | ||
| request.lastIndexOf("\\", queryStart - 1) | ||
| ); | ||
| if (hashStart >= lastSep) return request; | ||
| const pathPart = request.slice(0, lastSep); | ||
| return pathPart.replace(HASH_REGEXP, "\0#") + request.slice(lastSep); |
Member
Author
There was a problem hiding this comment.
The case you describe (#/some/fragment?query parsed as a fragment containing both / and ?) is structurally identical to the actual bug case in #16819: /home/user/proj#/webpack/.../index.js?protocol=ws.... Both are "a segment ending with #, followed by /...?query". Pure string analysis cannot distinguish "directory # + query" from "fragment containing / + ? inside the fragment" — both interpretations are valid URL syntax, webpack has historically supported both, and the parser has no filesystem context.
Given the ambiguity, we have to choose. This fix biases toward the directory interpretation because:
- It is the case from webpack-dev-server fails if
#in project path #16819, which has been open since 2023 and affects anyone whose project path contains#plus uses webpack-dev-server. - The fragment-with-
/-and-?pattern is webpack-specific and rare. Existing tests using fragment semantics (e.g../resourceFragment/index#" + __dirname) don't include a query string, so none of them break. - The escape is gated on
?being present; requests without a query keep their existing semantics entirely, so the common fragment cases are untouched. - Users who genuinely want fragment-containing-
/-and-?semantics can still write\0#explicitly to opt out.
The changeset is being updated separately to remove the parseResource mention (your other comment) — that wording was stale.
Generated by Claude Code
| "webpack": patch | ||
| --- | ||
|
|
||
| Treat `#` in an absolute path's directory name as part of the path rather than a fragment separator. `parseResource` and resolver requests now correctly handle absolute paths containing `#`, so projects in directories like `/home/user/proj#1` (and tools like webpack-dev-server that build absolute entry requests with query strings) resolve correctly. |
Member
Author
There was a problem hiding this comment.
Fixed in 71f7156 — the changeset now describes the resolver-side escape only and no longer claims parseResource behavior changed.
Generated by Claude Code
The changeset previously implied parseResource was modified; in fact the exported parser is unchanged and the fix only pre-escapes `#` in path requests before the resolver. Rewording per Copilot review feedback.
Comment on lines
+520
to
+524
| request.lastIndexOf("\\", queryStart - 1) | ||
| ); | ||
| if (hashStart >= lastSep) return request; | ||
| const pathPart = request.slice(0, lastSep); | ||
| return pathPart.replace(HASH_REGEXP, "\0#") + request.slice(lastSep); |
Member
Author
There was a problem hiding this comment.
Good catch. Fixed in 40aef01 — HASH_REGEXP is now /(?<!\0)#/g (negative lookbehind), so \0# sequences are skipped and the explicit opt-out remains stable. Added unit coverage for pre-escaped inputs, including mixed pre-escaped + raw # in the same request.
Generated by Claude Code
Comment on lines
+119
to
+123
| // `#` in directory portion with query → escaped | ||
| ["/home/user/proj#1/file.js?q=1", "/home/user/proj\0#1/file.js?q=1"], | ||
| ["/home/user/a#b/c#d/file.js?q=1", "/home/user/a\0#b/c\0#d/file.js?q=1"], | ||
| ["C:\\Users\\proj#1\\file.js?q=1", "C:\\Users\\proj\0#1\\file.js?q=1"], | ||
| ["C:/Users/proj#1/file.js?q=1", "C:/Users/proj\0#1/file.js?q=1"], |
Member
Author
There was a problem hiding this comment.
Done in 40aef01 — added three pre-escaped cases to the escapeHashInPathRequest matrix: pure \0# preserved, mixed \0# + raw # (only the raw one gets escaped), and the same on a relative path.
Generated by Claude Code
Comment on lines
+1
to
+4
| "use strict"; | ||
|
|
||
| const path = require("path"); | ||
|
|
Member
Author
There was a problem hiding this comment.
Done in 40aef01 — added the JSDoc type annotation to both new configCases configs.
Generated by Claude Code
|
|
||
| // Companion to issue-16819-#-in-path-#: same project layout, but the entry is | ||
| // expressed as a relative request with a query string. Without escaping, the | ||
| // resolver mis-splits `#` in the directory portion and fails. |
Member
Author
There was a problem hiding this comment.
| // Reproduces https://github.com/webpack/webpack/issues/16819: webpack-dev-server | ||
| // adds entries as absolute paths with query strings, and when the project lives | ||
| // in a directory containing `#` (e.g. `/home/user/proj#1/`), webpack incorrectly | ||
| // split the path at the first `#`, failing to resolve. |
Member
Author
There was a problem hiding this comment.
- escapeHashInPathRequest now uses a negative-lookbehind regex so an
already-escaped `\0#` is not re-escaped into `\0\0#`, keeping the
explicit `\0#` opt-out stable. Adds unit coverage for pre-escaped
inputs (mixed with raw `#` chars).
- Adds the `Configuration` JSDoc type annotation to both new
configCases webpack.config.js files for consistency with the rest
of the suite.
- Fixes a present/past tense comment ("split" -> "splits").
All addressing Copilot review feedback on #20980.
Contributor
Types CoverageCoverage after merging claude/fix-issue-16819-x9nMl into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When an absolute path contains
#followed by a path separator (e.g./home/user/proj#1/file.js), treat the#as part of the directoryname rather than a fragment marker. A real fragment cannot have a path
separator after it before any query, so this disambiguation is sound
and preserves existing fragment behavior for paths like
/abs/path/file.js#fragment.fixes #16819
fixes #20877