cmd/go/internal/get: VCS path regexp omits characters accepted by CheckImportPath #31376

stub42 · 2019-04-10T03:13:54Z

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

$ go version
go version go1.12.2 linux/amd64

Does this issue reproduce with the latest release?

Yes

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

go env Output

$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/stub/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/stub/go"
GOPROXY=""
GORACE=""
GOROOT="/snap/go/3540"
GOTMPDIR=""
GOTOOLDIR="/snap/go/3540/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
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-build922868437=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Attempted to fetch the git repo containing a go package, publicly available at:

git://git.launchpad.net/~stub/+git/go-eggs
git+ssh://git.launchpad.net/~stub/+git/go-eggs
https://git.launchpad.net/~stub/+git/go-eggs

Per go help importpath, the following should attempt a git download using https://, falling back to git+ssh://

go get -v git.launchpad.net/~stub/+git/go-eggs.git

Attempting to force ssh per git help environment does not help

GIT_ALLOW_PROTOCOL=ssh go get -v git.launchpad.net/~stub/+git/go-eggs.git

What did you expect to see?

go get attempt to clone the git repo at https://git.launchpad.net/~stub/+git/go-eggs, and if that failed, fall back to cloning the git repo at git+ssh://git.launchpad.net/~stub/+git/go-eggs.

Per go help importpath, because the import path has a version control qualifier (.git) then no attempt should be made to attempt to get the import over https: to look for a tag.

What did you see instead?

Go get attempts to parse meta tags from https://, which fails. No attempt is made to clone the git repo.

$ go get -v git.launchpad.net/~stub/+git/go-eggs.git
Fetching https://git.launchpad.net/~stub/+git/go-eggs.git?go-get=1
Parsing meta tags from https://git.launchpad.net/~stub/+git/go-eggs.git?go-get=1 (status code 200)
package git.launchpad.net/~stub/+git/go-eggs.git: unrecognized import path "git.launchpad.net/~stub/+git/go-eggs.git" (parse https://git.launchpad.net/~stub/+git/go-eggs.git?go-get=1: no go-import meta tags ())

The text was updated successfully, but these errors were encountered:

bcmills · 2019-04-10T19:07:51Z

Duplicate of #26134

bcmills · 2019-04-11T15:43:33Z

The regular expression that looks for the .git suffix is here:

go/src/cmd/go/internal/get/vcs.go

Line 1028 in ccd9d9d

    
           re:   `^(?P<root>(?P<repo>([a-z0-9.\-]+\.)+[a-z0-9.\-]+(:[0-9]+)?(/~?[A-Za-z0-9_.\-]+)+?)\.(?P<vcs>bzr|fossil|git|hg|svn))(/~?[A-Za-z0-9_.\-]+)*$`,

Testing against that (playground) reveals that the problem here is that the + character fails to match the regular expression, which is more restrictive about + and ~ than our validation for import paths in general:

go/src/cmd/go/internal/get/path.go

Lines 125 to 136 in 3cb92fc

    
           // pathOK reports whether r can appear in an import path element. 
        
           // 
        
           // NOTE: This function DIVERGES from module mode pathOK by accepting Unicode letters. 
        
           func pathOK(r rune) bool { 
        
           	if r < utf8.RuneSelf { 
        
           		return r == '+' || r == '-' || r == '.' || r == '_' || r == '~' || 
        
           			'0' <= r && r <= '9' || 
        
           			'A' <= r && r <= 'Z' || 
        
           			'a' <= r && r <= 'z' 
        
           	} 
        
           	return unicode.IsLetter(r) 
        
           }

bcmills · 2019-04-11T15:46:08Z

This needs a fix, but it's not obvious to me whether the VCS path should be made more permissive, or CheckImportPath should be made more restrictive. I'll follow up with @rsc and @jayconrod.

rsc · 2019-04-30T15:19:24Z

I'm confused by whether the +git is essential here. The URL already starts and ends with git. Must it also have git in the middle? Somehow we have gotten this far without allowing + in go get paths, and I want to understand why.

stub42 · 2019-05-01T01:22:30Z

When Launchpad code hosting added git support it was necessary to add +git as a path component to avoid namespace clashes with Bazaar branches. Launchpad code hosting is pretty much stuck with it, but yes, + will be very uncommon, unlike other special characters such as ~. The trailing .git is optional, and only in my example because I wanted to skip the https: VCS detection (probably necessary for private repositories, when I get that far).

bcmills · 2019-05-08T17:11:23Z

I checked the new module index for other existing packages that use the + character, but haven't been able to locate any so far.

(That's not to say that they don't exist, but if they do they're rare.)

Perhaps we should disallow + from import paths entirely?

rsc · 2019-05-09T15:19:12Z

If this is not a breakage introduced after Go 1.12, I'm inclined to leave deciding what to do until after Go 1.13 is out.

ianlancetaylor · 2020-05-19T00:42:03Z

@bcmills This issue is just rolling forward through release milestones. Should we just move it to the Backlog milestone? That's what Backlog is for. Thanks.

rsc · 2020-05-28T18:41:22Z

Unicode characters is #29101. Assuming we exclude that from this discussion, this discussion is only about whether to start using + in paths.

Technically the looser module restrictions mean someone could set up a non-VCS-backed module with + today, but all the VCS-backed stuff can't use it. There's nothing with + in the module index (index.golang.org).

rsc · 2020-05-28T18:47:28Z

It seems like we should reject + and wait for complaints.
There are no uses we can find.
It doesn't seem to be used anywhere.
If we drop it from pathOK on day 1 of Go 1.16, we will have time to put it back.

stub42 · 2020-06-01T05:47:36Z

The original complaint is that Launchpad git hosting URLs such as https://git.launchpad.net/~stub/+git/go-eggs do not work with 'go get', and now Go modules. It is not used anywhere, because it doesn't work (and never has).

dmitshur · 2020-08-12T17:37:58Z

This issue is currently labeled as early-in-cycle for Go 1.16.
That time is now, so this is a friendly ping so the issue is looked at again.

gopherbot · 2020-08-26T21:22:04Z

Change https://golang.org/cl/250919 mentions this issue: module: reject "+" in CheckPath and CheckImportPath

"+" was allowed in some vcs regular expressions, but doesn't seem to be used in practice anymore. Stop accepting it in import paths. This is being submitted early in the Go 1.16 cycle so that if a usage is detected it can be reverted. See the discussion in golang.org/issue/31376 for more details. For golang/go#31376 Change-Id: I392fcdcf829886bd0a28450ba5e399e64dd01559 Reviewed-on: https://go-review.googlesource.com/c/mod/+/250919 Run-TryBot: Michael Matloob <matloob@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com> Reviewed-by: Bryan C. Mills <bcmills@google.com>

jamie-digital · 2021-01-07T16:48:21Z

This has affected me, but it's not a significant issue. I have a module containing a main package with the name etld+1, which is a tool for processing domain names. I can rename it to something less helpful, but it would be nice if I didn't have to.

gopherbot · 2021-01-07T23:17:02Z

Change https://golang.org/cl/282512 mentions this issue: content/static/doc: remove "+" from allowed module path characters

Go 1.16 will no longer accept "+" as a character in a module or import path. Amend the module docs to reflect that change. Fixes golang/go#31376 For golang/go#43052 Change-Id: Ie0b58888cf5023c69f112dcc32137fc69af6c659 Reviewed-on: https://go-review.googlesource.com/c/website/+/282512 Trust: Michael Matloob <matloob@golang.org> Run-TryBot: Michael Matloob <matloob@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com> Reviewed-by: Bryan C. Mills <bcmills@google.com>

stub42 changed the title ~~Unable to 'go get' git repo~~ Unable to 'go get' git repo, VCS qualifier ignored Apr 10, 2019

bcmills changed the title ~~Unable to 'go get' git repo, VCS qualifier ignored~~ cmd/go: Unable to 'go get' git repo, VCS qualifier ignored Apr 10, 2019

bcmills marked this as a duplicate of #26134 Apr 10, 2019

bcmills closed this as completed Apr 10, 2019

stub42 mentioned this issue Apr 11, 2019

cmd/go: private repos on hard-coded hosting providers require special configuration #26134

Open

bcmills marked this as not a duplicate of #26134 Apr 11, 2019

bcmills reopened this Apr 11, 2019

bcmills changed the title ~~cmd/go: Unable to 'go get' git repo, VCS qualifier ignored~~ cmd/go/internel/get: VCS path regexp omits characters accepted by CheckImportPAth Apr 11, 2019

bcmills changed the title ~~cmd/go/internel/get: VCS path regexp omits characters accepted by CheckImportPAth~~ cmd/go/internel/get: VCS path regexp omits characters accepted by CheckImportPath Apr 11, 2019

bcmills added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Apr 11, 2019

bcmills added this to the Go1.13 milestone Apr 11, 2019

rsc changed the title ~~cmd/go/internel/get: VCS path regexp omits characters accepted by CheckImportPath~~ cmd/go/internal/get: VCS path regexp omits characters accepted by CheckImportPath May 1, 2019

bcmills modified the milestones: Go1.13, Go1.14 May 9, 2019

rsc modified the milestones: Go1.14, Backlog Oct 9, 2019

bcmills modified the milestones: Backlog, Go1.14 Oct 10, 2019

ianlancetaylor modified the milestones: Go1.14, Go1.15 Dec 5, 2019

jayconrod mentioned this issue Feb 10, 2020

cmd/go: unzip github.com/bep/golibsass@v0.1.0 fails with invalid char '́' (U+0301) #37139

Closed

bcmills added the early-in-cycle A change that should be done early in the 3 month dev cycle. label May 28, 2020

bcmills modified the milestones: Go1.15, Go1.16 May 28, 2020

bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label May 29, 2020

gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label May 29, 2020

bcmills mentioned this issue Jan 7, 2021

cmd/go: add release note for rejection of non-ASCII import paths in module mode #43052

Closed

gopherbot closed this as completed in golang/website@9a4d5e8 Jan 8, 2021

bcmills mentioned this issue Mar 9, 2021

cmd/go: allow import paths ending with '+' (for executable like g++ or clang++) #44776

Closed

golang locked and limited conversation to collaborators Jan 8, 2022

gopherbot added the FrozenDueToAge label Jan 8, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/go/internal/get: VCS path regexp omits characters accepted by CheckImportPath #31376

cmd/go/internal/get: VCS path regexp omits characters accepted by CheckImportPath #31376

stub42 commented Apr 10, 2019

bcmills commented Apr 10, 2019

bcmills commented Apr 11, 2019

bcmills commented Apr 11, 2019

rsc commented Apr 30, 2019

stub42 commented May 1, 2019

bcmills commented May 8, 2019

rsc commented May 9, 2019

ianlancetaylor commented May 19, 2020

rsc commented May 28, 2020

rsc commented May 28, 2020

stub42 commented Jun 1, 2020

dmitshur commented Aug 12, 2020

gopherbot commented Aug 26, 2020

jamie-digital commented Jan 7, 2021

gopherbot commented Jan 7, 2021

cmd/go/internal/get: VCS path regexp omits characters accepted by CheckImportPath #31376

cmd/go/internal/get: VCS path regexp omits characters accepted by CheckImportPath #31376

Comments

stub42 commented Apr 10, 2019

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

Does this issue reproduce with the latest release?

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

What did you do?

What did you expect to see?

What did you see instead?

bcmills commented Apr 10, 2019

bcmills commented Apr 11, 2019

bcmills commented Apr 11, 2019

rsc commented Apr 30, 2019

stub42 commented May 1, 2019

bcmills commented May 8, 2019

rsc commented May 9, 2019

ianlancetaylor commented May 19, 2020

rsc commented May 28, 2020

rsc commented May 28, 2020

stub42 commented Jun 1, 2020

dmitshur commented Aug 12, 2020

gopherbot commented Aug 26, 2020

jamie-digital commented Jan 7, 2021

gopherbot commented Jan 7, 2021

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

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