Skip to content

Commit

Permalink
fix #822: "/..." paths are absolute on windows
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Feb 17, 2021
1 parent 91cf837 commit 5b346ca
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 2 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@

Previously path resolution would fail because these files do not end with the `.eot?#iefix` or `.svg#icons` extensions. Now path resolution should succeed. The URL query and fragment are not unconditionally stripped because there is apparently [code in the wild that uses `#` as a directory name](https://github.com/medikoo/es5-ext/tree/3ddd2066b19e7c25a782869a304ae35d8188c8f1/string/%23). So esbuild will still try to resolve the full import path first and only try to reinterpret the path as a URL if that fails.

* Prevent paths starting with `/` from being used as relative paths on Windows ([#822](https://github.com/evanw/esbuild/issues/822))

On Windows, absolute paths start with a drive letter such as `C:\...` instead of with a slash like `/...`. This means that paths starting with a `/` can actually be used as relative paths. For example, this means an import of `/subfolder/image.png` will match the file at the path `./subfolder/image.png`. This is problematic for Windows users because they may accidentally make use of these paths and then try to run their code on a non-Windows platform only for it to fail to build.

Now paths starting with a `/` are always treated as an absolute path on all platforms. This means you can no longer import files at a relative path that starts with `/` on Windows. You should be using a `./` prefix instead.

## 0.8.46

* Fix minification of `.0` in CSS ([#804](https://github.com/evanw/esbuild/issues/804))
Expand Down
12 changes: 10 additions & 2 deletions internal/resolver/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -370,8 +370,16 @@ func (r *resolver) resolveWithoutSymlinks(sourceDir string, importPath string, k
// described here: https://nodejs.org/api/modules.html#modules_all_together
var result PathPair

// Return early if this is already an absolute path
if r.fs.IsAbs(importPath) {
// Return early if this is already an absolute path. In addition to asking
// the file system whether this is an absolute path, we also explicitly check
// whether it starts with a "/" and consider that an absolute path too. This
// is because relative paths can technically start with a "/" on Windows
// because it's not an absolute path on Windows. Then people might write code
// with imports that start with a "/" that works fine on Windows only to
// experience unexpected build failures later on other operating systems.
// Treating these paths as absolute paths on all platforms means Windows
// users will not be able to accidentally make use of these paths.
if strings.HasPrefix(importPath, "/") || r.fs.IsAbs(importPath) {
// First, check path overrides from the nearest enclosing TypeScript "tsconfig.json" file
if dirInfo := r.dirInfoCached(sourceDir); dirInfo != nil && dirInfo.tsConfigJSON != nil && dirInfo.tsConfigJSON.Paths != nil {
if absolute, ok := r.matchTSConfigPaths(dirInfo.tsConfigJSON, importPath, kind); ok {
Expand Down
19 changes: 19 additions & 0 deletions scripts/end-to-end-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -2446,6 +2446,25 @@
}),
)

// Test for a Windows-specific issue where paths starting with "/" could be
// treated as relative paths, leading to inconvenient cross-platform failures:
// https://github.com/evanw/esbuild/issues/822
tests.push(
test(['in.js', '--bundle'], {
'in.js': `
import "/file.js"
`,
'file.js': `This file should not be imported on Windows`,
}, {
expectedStderr: ` > in.js: error: Could not read from file: /file.js
2 │ import "/file.js"
╵ ~~~~~~~~~~
1 error
`,
}),
)

function test(args, files, options) {
return async () => {
const hasBundle = args.includes('--bundle')
Expand Down

0 comments on commit 5b346ca

Please sign in to comment.