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

embed: Files starting with - (dash) are not embedded (undocumented behavior) #45447

Closed
breml opened this issue Apr 8, 2021 · 4 comments
Closed
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@breml
Copy link
Contributor

breml commented Apr 8, 2021

What version of Go are you using (go version)?

$ go version
go version go1.16.2 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

amd64, linux

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="redacted/.cache/go-build"
GOENV="redacted/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="redacted/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="redacted/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="redacted/go1.16.2.linux.amd64"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="redacted/go1.16.2.linux.amd64/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.16.2"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="redacted/embedtest/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build424344437=/tmp/go-build -gno-record-gcc-switches"

What did you do?

  • Create a new Go module.
  • Create the directories ./files and ./files/subdir
  • Create the files: ./files/file, ./files/-file
  • Add the following source to ./main.go
package main

import (
	"embed"
	"fmt"
	"io/fs"
)

//go:embed files
var files embed.FS

func main() {
	err := fs.WalkDir(files, "files", walk)
	if err != nil {
		panic(err)
	}
}

func walk(path string, d fs.DirEntry, _ error) error {
	fmt.Println(path)
	return nil
}
  • Run with go run .

What did you expect to see?

files
files/-file
files/file
files/subdir/-file

What did you see instead?

files
files/file

Analysis

This behavior was introduced with commit 930c2c9 where isBadEmbedName has been extended with if err := module.CheckFilePath(name); err != nil {.
The thing with module.CheckFilePath is, that it expects a full path as argument. For such a path, the first segment is not allowed to start with a - (dash) according to https://github.com/golang/mod/blob/d6ab96f2441f9631f81862375ef66782fc4a9c12/module/module.go#L374.
In the case of isBadEmbedName, the function module.CheckFilePath is called with the filename only (without the rest of the path), which prevents any file, starting with - to be embedded.

This behavior is nowhere documented. The current documentation only mentions, that files starting with . and _ are ignored.

Additionally, nor the documentation of module.CheckFilePath neither of module.CheckImportPath do mention, that paths starting with - are not allowed.

Related: #45197, #43854, #44486

Update: add #44486 as related issue

@jayconrod jayconrod added NeedsFix The path to resolution is known, but the work has not been done. and removed Documentation labels Apr 8, 2021
@jayconrod jayconrod added this to the Go1.17 milestone Apr 8, 2021
@jayconrod
Copy link
Contributor

This seems like a bug. Files with names starting with - in the module root directory should not be embeddable, but files in other directories should be embeddable.

@gopherbot
Copy link

Change https://golang.org/cl/311529 mentions this issue: cmd/go: permit embedded path to start with dash

@ianlancetaylor ianlancetaylor self-assigned this Apr 19, 2021
@gopherbot
Copy link

Change https://golang.org/cl/316629 mentions this issue: module: accept leading dash in a file path

gopherbot pushed a commit to golang/mod that referenced this issue May 4, 2021
For golang/go#45447

Change-Id: I38b13bc912851aa9b6cdeb851330a5ed97b743ac
Reviewed-on: https://go-review.googlesource.com/c/mod/+/316629
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
@gopherbot
Copy link

Change https://golang.org/cl/316789 mentions this issue: cmd: update to latest version of x/mod

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge 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