Skip to content
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

treat "ERROR_INVALID_NAME" as "ENOENT" on windows #994

Merged
merged 1 commit into from
Mar 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,24 @@
# Changelog

## Unreleased

* Fix importing a path containing a `?` character on Windows ([#989](https://github.com/evanw/esbuild/issues/989))

On Windows, the `?` character is not allowed in path names. This causes esbuild to fail to import paths containing this character. This is usually fine because people don't put `?` in their file names for this reason. However, the import paths for some ancient CSS code contains the `?` character as a hack to work around a bug in Internet Explorer:

```css
@font-face {
src:
url("./icons.eot?#iefix") format('embedded-opentype'),
url("./icons.woff2") format('woff2'),
url("./icons.woff") format('woff'),
url("./icons.ttf") format('truetype'),
url("./icons.svg#icons") format('svg');
}
```

The intent is for the bundler to ignore the `?#iefix` part. However, there may actually be a file called `icons.eot?#iefix` on the file system so esbuild checks the file system for both `icons.eot?#iefix` and `icons.eot`. This check was triggering this issue. With this release, an invalid path is considered the same as a missing file so bundling code like this should now work on Windows.

## 0.9.3

* Fix path resolution with the `exports` field for scoped packages
Expand Down
62 changes: 33 additions & 29 deletions internal/fs/fs_real.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ func (fs *realFS) ReadDirectory(dir string) (DirEntries, error) {
}

// Cache miss: read the directory entries
names, err := readdir(dir)
names, err := fs.readdir(dir)
entries := DirEntries{dir, make(map[string]*Entry)}

// Unwrap to get the underlying error
Expand Down Expand Up @@ -184,19 +184,7 @@ func (fs *realFS) ReadFile(path string) (string, error) {
BeforeFileOpen()
defer AfterFileClose()
buffer, err := ioutil.ReadFile(path)

// Unwrap to get the underlying error
if pathErr, ok := err.(*os.PathError); ok {
err = pathErr.Unwrap()
}

// Windows returns ENOTDIR here even though nothing we've done yet has asked
// for a directory. This really means ENOENT on Windows. Return ENOENT here
// so callers that check for ENOENT will successfully detect this file as
// missing.
if err == syscall.ENOTDIR {
err = syscall.ENOENT
}
err = fs.canonicalizeError(err)

// Allocate the string once
fileContents := string(buffer)
Expand Down Expand Up @@ -282,23 +270,11 @@ func (fs *realFS) Rel(base string, target string) (string, bool) {
return "", false
}

func readdir(dirname string) ([]string, error) {
func (fs *realFS) readdir(dirname string) ([]string, error) {
BeforeFileOpen()
defer AfterFileClose()
f, err := os.Open(dirname)

// Unwrap to get the underlying error
if pathErr, ok := err.(*os.PathError); ok {
err = pathErr.Unwrap()
}

// Windows returns ENOTDIR here even though nothing we've done yet has asked
// for a directory. This really means ENOENT on Windows. Return ENOENT here
// so callers that check for ENOENT will successfully detect this directory
// as missing.
if err == syscall.ENOTDIR {
return nil, syscall.ENOENT
}
err = fs.canonicalizeError(err)

// Stop now if there was an error
if err != nil {
Expand All @@ -319,6 +295,34 @@ func readdir(dirname string) ([]string, error) {
return entries, err
}

func (fs *realFS) canonicalizeError(err error) error {
// Unwrap to get the underlying error
if pathErr, ok := err.(*os.PathError); ok {
err = pathErr.Unwrap()
}

// This has been copied from golang.org/x/sys/windows
const ERROR_INVALID_NAME syscall.Errno = 123

// Windows is much more restrictive than Unix about file names. If a file name
// is invalid, it will return ERROR_INVALID_NAME. Treat this as ENOENT (i.e.
// "the file does not exist") so that the resolver continues trying to resolve
// the path on this failure instead of aborting with an error.
if fs.fp.isWindows && err == ERROR_INVALID_NAME {
err = syscall.ENOENT
}

// Windows returns ENOTDIR here even though nothing we've done yet has asked
// for a directory. This really means ENOENT on Windows. Return ENOENT here
// so callers that check for ENOENT will successfully detect this file as
// missing.
if err == syscall.ENOTDIR {
err = syscall.ENOENT
}

return err
}

func (fs *realFS) kind(dir string, base string) (symlink string, kind EntryKind) {
entryPath := fs.fp.join([]string{dir, base})

Expand Down Expand Up @@ -401,7 +405,7 @@ func (fs *realFS) WatchData() WatchData {

case stateDirHasEntries:
paths[path] = func() bool {
names, err := readdir(path)
names, err := fs.readdir(path)
if err != nil || len(names) != len(data.dirEntries) {
return true
}
Expand Down
11 changes: 11 additions & 0 deletions scripts/end-to-end-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,17 @@
}),
)

// Test resolving paths with a question mark (an invalid path on Windows)
tests.push(
test(['entry.js', '--bundle', '--outfile=node.js'], {
'entry.js': `
import x from "./file.js?ignore-me"
if (x !== 123) throw 'fail'
`,
'file.js': `export default 123`,
}),
)

// Tests for symlinks
//
// Note: These are disabled on Windows because they fail when run with GitHub
Expand Down