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

os: Open("file/.") does not produce an error on wasip1 #69509

Open
neild opened this issue Sep 17, 2024 · 5 comments
Open

os: Open("file/.") does not produce an error on wasip1 #69509

neild opened this issue Sep 17, 2024 · 5 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@neild
Copy link
Contributor

neild commented Sep 17, 2024

Go version

master

Output of go env in your module/workspace:

$ go env
GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/dneil/Library/Caches/go-build'
GOENV='/Users/dneil/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/dneil/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/dneil'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/Users/dneil/src/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/Users/dneil/src/go/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='devel go1.24-adf220a5d5 Mon Sep 9 17:11:52 2024 +0000'
GODEBUG=''
GOTELEMETRY='on'
GOTELEMETRYDIR='/Users/dneil/Library/Application Support/go/telemetry'
GCCGO='gccgo'
GOARM64='v8.0'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/tmp/m2/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/kw/0t4d_x2n4plg9157krpjtxmw0047mf/T/go-build1969548244=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

package main

import (
        "fmt"
        "os"
)

func main() {
        _, err := os.Open("file/.")
        fmt.Println(err)
}
$ touch file
$ go run main.go
open file/.: not a directory
$ GOOS=wasip1 GOARCH=wasm go run main.go
<nil>

What did you see happen?

When built with GOOS=wasip1, the os package performs some path cleaning on filenames which results in a terminal "/." being removed. This causes opening a non-directory file to unexpectedly succeed.

What did you expect to see?

An error opening "file/.", because "file" is not a directory.

@neild
Copy link
Contributor Author

neild commented Sep 17, 2024

Hmm, the problem seems to be that wasip1 is performing path cleaning in general prior to opening files, which is going to affect more than just a trailing "/.".

This produces incorrect results in a variety of situations, or at least results inconsistent with the usual behavior of Unix filesystems.

  • Open("a/../b") should be an error if a does not exist, or is a non-directory file.
  • Open("symlink/../b") should resolve any symlink in the first path component, and then open b in the parent of the linked-to directory.
  • Open("a/") should be an error if a is a non-directory file.

Testing with wasmtime, the WASI file API functions all seem to behave as I'd expect here (modulo what seems to be one bug: bytecodealliance/wasmtime#9272). The additional path-cleaning behavior is being added in the Go runtime, and I don't believe it's correct.

@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 18, 2024
@cagedmantis cagedmantis added this to the Go1.24 milestone Sep 18, 2024
@cagedmantis
Copy link
Contributor

@Zxilly
Copy link
Member

Zxilly commented Sep 21, 2024

I sent a patch that fixes file/. issue, but I need to investigate further for subsequent issues.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/614083 mentions this issue: syscall: add separator for filepath if contains "." in waspi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

5 participants