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

cmd/go,x/mod/module: missing exclusion for COM0, LPT0 for embedded file names #66625

Closed
cookiengineer opened this issue Mar 31, 2024 · 4 comments
Assignees
Labels
GoCommand cmd/go modules NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@cookiengineer
Copy link

cookiengineer commented Mar 31, 2024

Go version

go version go1.22.1 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/cookiengineer/.cache/go-build'
GOENV='/home/cookiengineer/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/cookiengineer/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/cookiengineer/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/lib/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/lib/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.22.1'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/cookiengineer/whatever/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 -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build2124737729=/tmp/go-build -gno-record-gcc-switches'

What did you do?

I tried to embed a file called aux.json.gz on GNU/Linux:

//go:embed ./subfolder/aux.json.gz
var embedded_bytes []byte

What did you see happen?

Trying to embed files on GNU/Linux systems that have aux, (or con, nul, prn, com*, lpt*) as a basename of the file cannot be embedded via go:embed.

The reason is a call to isBadEmbedName() in the resolveEmbed() method in go/internal/load/pkg.go.

The isBadEmbedName() method leads to module.CheckFilePath(name) which compares the basename of the file to the badWindowsNames slice in module/module.go.

My questions are now the following:

  • Why is this the case on GNU/Linux systems?
  • Why does go:embed need valid filenames for go modules?
  • Why does go:embed need valid filenames for Windows? Is it unpacked temporarily when the binary is executed later, similar to squashfs, behind the scenes? Why is this not behind a host-specific tag?
  • If the decision is to keep the reserved basenames, COM0 and LPT0 are missing from that list. See the linked article from the module.go file.

What did you expect to see?

I expected to be able to embed filenames when they conform to the POSIX filesystem requirements, so I think it should be possible to embed filenames with the basename of con, prn, aux, nul, com*, lpt* etc.

@seankhliao seankhliao changed the title pattern <folder>/aux.json.gz: cannot embed file: invalid name cmd/go,x/mod/module: missing exclusion for COM0, LPT0 for embedded file names Mar 31, 2024
@seankhliao
Copy link
Member

we expect modules to be usable cross platform, so file name restrictions apply everywhere

@seankhliao seankhliao added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. GoCommand cmd/go labels Mar 31, 2024
@matloob matloob self-assigned this May 7, 2024
@matloob matloob added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels May 7, 2024
@matloob matloob added this to the Go1.23 milestone May 7, 2024
@matloob matloob added the modules label May 7, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/583836 mentions this issue: module: add COM0 and LPT0 to badWindowsNames

gopherbot pushed a commit to golang/mod that referenced this issue May 7, 2024
They have been added to the list of file names that are disallowed on
Windows that's referenced in the comment to badWindowsNames, so add
them to badWindowsNames.

For golang/go#67238
For golang/go#66625

Change-Id: I82e5d70f33330f746783fd22090a3ebaf9408dfc
Reviewed-on: https://go-review.googlesource.com/c/mod/+/583836
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Sam Thanawalla <samthanawalla@google.com>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/584335 mentions this issue: cmd: vendor golang.org/x/mod@6686f41

gopherbot pushed a commit that referenced this issue May 9, 2024
To pull in CL 583836

Commands run
    go get golang.org/x/mod@6686f416970d4b8e2f54f521955dee89e6763c4b
    go mod tidy
    go mod vendor

For #67238
For #66625

Change-Id: I77e49706481e068d27072a38d0d2464aa40d2dd0
Cq-Include-Trybots: luci.golang.try:gotip-windows-amd64-longtest,gotip-linux-amd64-longtest
Reviewed-on: https://go-review.googlesource.com/c/go/+/584335
Reviewed-by: Sam Thanawalla <samthanawalla@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@matloob matloob closed this as completed May 23, 2024
@matloob
Copy link
Contributor

matloob commented Jun 4, 2024

Because it's not clear if those filenames were intender to be excluded in the Microsoft documentation, I will revert this change in x/mod and revendor x/mod. If we get more conclusive evidence that the filenames are indeed excluded, we'll redo for 1.24.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GoCommand cmd/go modules NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants