Go: cmd/go: "go get" fails on import path patterns with wildcards ("...")

[FiloSottile]

The fix for cmd/go: remote command execution during "go get -u" (#29230) as released in Go 1.10.6 and 1.11.3 introduced a regression causing commands like go get example.com/... to fail with invalid import path: malformed import path "example.com/...": double dot when fetching packages not already present in GOPATH.

See https://github.com/golang/go/issues/29236#issuecomment-447144051

[niaow]

Looking through the code, I saw the following mistakes that could have caused this error:

Error 1 - strings.Contains(path, "..") creates false positives with something/...
https://github.com/golang/go/commit/8954addb3294a5e664a9833354bafa58f163fe8f#diff-5b11f198c631a3d489e1be171853ab7bR40

Error 2 - the ... path would cause this to error too, since it is all .
https://github.com/golang/go/commit/8954addb3294a5e664a9833354bafa58f163fe8f#diff-5b11f198c631a3d489e1be171853ab7bR70

Suggested Fix

Remove the check listed in error 1. In addition to causing false positives, it is rendered unnecessary by the line mentioned in error 2. Leave the line in error 2 unchanged, but add a check that skips this validation for the last element if it exactly matches ....

[niaow]

Is someone currently working on this? If not, I can try to create a fix.

[niaow]

In absence of a response, I have started fixing this.

[FiloSottile]

Thank you, but I believe @dmitshur and @bcmills are already moving on a fix.

[niaow]

Ok. Thank you for letting me know.

[willbuckner]

diff --git a/src/cmd/go/internal/get/path.go b/src/cmd/go/internal/get/path.go
index d443bd28a9..2a109d6638 100644
--- a/src/cmd/go/internal/get/path.go
+++ b/src/cmd/go/internal/get/path.go
@@ -41,9 +41,6 @@ func checkPath(path string, fileName bool) error {
        if path == "" {
                return fmt.Errorf("empty string")
        }
-       if strings.Contains(path, "..") {
-               return fmt.Errorf("double dot")
-       }
        if strings.Contains(path, "//") {
                return fmt.Errorf("double slash")
        }
@@ -59,8 +56,10 @@ func checkPath(path string, fileName bool) error {
                        elemStart = i + 1
                }
        }
-       if err := checkElem(path[elemStart:], fileName); err != nil {
-               return err
+       if path[elemStart:] != "..." {
+               if err := checkElem(path[elemStart:], fileName); err != nil {
+                       return err
+               }
        }
        return nil
 }

I'd submit this, but I'm sure someone else can do it faster as I haven't yet made a Go contribution :)

[gopherbot]

Change https://golang.org/cl/154121 mentions this issue: cmd/go: fix regression allowing '...' wildcard as last element in go get

[FiloSottile]

Change https://golang.org/cl/154121

https://go-review.googlesource.com/154121

[bcmills]

@gopherbot, please backport to 1.11 and 1.10 immediately: this is a major regression.

[gopherbot]

Backport issue(s) opened: #29247 (for 1.10), #29248 (for 1.11).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

[gopherbot]

Change https://golang.org/cl/154108 mentions this issue: cmd/go/internal/get: move wildcard-trimming to before CheckImportPath

[thaJeztah]

@FiloSottile I noticed that go list ./... still works on Go 1.11.3 (and Go 1.10.6). While it's usable as a workaround for this regression (https://github.com/containerd/containerd/pull/2879), wondering if that's actually something that was missed in the patch release (or won't go list have the same security issue? I didn't dive deep into the fix that went into those patch releases 😅)

[bcmills]

@thaJeztah, go list does not exhibit the same issue because it does not run VCS commands.

[thaJeztah]

@bcmills ah, makes sense, thanks for confirming it's nothing to worry about 👍

[johan-lejdung]

Is there any workaround or does anyone know when the planned fix for this is released?

[ldez]

The release is already done since 2 days: go1.11.4 and go1.10.7

Note: the docker images are not up-to-date https://hub.docker.com/_/golang?tab=tags

[johan-lejdung]

Ah thanks! I was looking at the docker images. Do those follow a different release schedule?

[niaow]

@johan-lejdung https://github.com/docker-library/golang/issues/255 - the docker tags are currently in progress