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: "go get" fails on import path patterns with wildcards ("...") #29241

Closed
FiloSottile opened this issue Dec 13, 2018 · 18 comments
Closed
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker Soon This needs to be done soon. (regressions, serious bugs, outages)
Milestone

Comments

@FiloSottile
Copy link
Contributor

FiloSottile commented Dec 13, 2018

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 #29236 (comment)

@FiloSottile FiloSottile added NeedsFix The path to resolution is known, but the work has not been done. Soon This needs to be done soon. (regressions, serious bugs, outages) labels Dec 13, 2018
@FiloSottile FiloSottile added this to the Go1.12 milestone Dec 13, 2018
@niaow
Copy link

niaow commented Dec 13, 2018

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/...
8954add#diff-5b11f198c631a3d489e1be171853ab7bR40

Error 2 - the ... path would cause this to error too, since it is all .
8954add#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 ....

@FiloSottile FiloSottile pinned this issue Dec 13, 2018
@josharian josharian unpinned this issue Dec 13, 2018
@cespare cespare pinned this issue Dec 13, 2018
@cespare cespare unpinned this issue Dec 13, 2018
@josharian josharian pinned this issue Dec 13, 2018
@niaow
Copy link

niaow commented Dec 13, 2018

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

@niaow
Copy link

niaow commented Dec 14, 2018

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

@FiloSottile
Copy link
Contributor Author

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

@niaow
Copy link

niaow commented Dec 14, 2018

Ok. Thank you for letting me know.

@willbuckner
Copy link

willbuckner commented Dec 14, 2018

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
Copy link

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

@FiloSottile
Copy link
Contributor Author

@bcmills
Copy link
Contributor

bcmills commented Dec 14, 2018

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

@gopherbot
Copy link

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
Copy link

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

@bcmills
Copy link
Contributor

bcmills commented Dec 14, 2018

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

@thaJeztah
Copy link
Contributor

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

@myitcv myitcv unpinned this issue Dec 14, 2018
@FiloSottile FiloSottile pinned this issue Dec 14, 2018
shentubot pushed a commit to google/gvisor that referenced this issue Dec 15, 2018
Go 1.11.3 has a bug:
golang/go#29241

This is fixed/workarounded in containerd:
containerd/containerd@52de371

Until that commit has made it into a tagged version, we will use containerd at
head.

PiperOrigin-RevId: 225636987
Change-Id: I7e32beb7751f566f5b41682a29a14442c1aa56c2
@myitcv myitcv unpinned this issue Dec 15, 2018
jgillich referenced this issue in docker-library/official-images Dec 16, 2018
@johan-lejdung
Copy link

johan-lejdung commented Dec 16, 2018

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

@ldez
Copy link

ldez commented Dec 16, 2018

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
Copy link

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

aswild added a commit to aswild/gitea that referenced this issue Dec 16, 2018
Go 1.11.3's CVE fix for 'remote command execution during "go get -u"'
broke getting wildcard paths, which Gitea's makefile uses to fetch
go-bindata and swagger.

The error is:
'package github.com/jteeuwen/go-bindata/...: github.com/jteeuwen/go-bindata/...:
invalid import path: malformed import path
"github.com/jteeuwen/go-bindata/...": double dot'

If the source already exists (get without the ...) then it works.

This should be fixed in go 1.11.4.
See golang/go#29241
@niaow
Copy link

niaow commented Dec 16, 2018

@johan-lejdung docker-library/golang#255 - the docker tags are currently in progress

tonistiigi pushed a commit to tonistiigi/gvisor that referenced this issue Jan 30, 2019
Go 1.11.3 has a bug:
golang/go#29241

This is fixed/workarounded in containerd:
containerd/containerd@52de371

Until that commit has made it into a tagged version, we will use containerd at
head.

PiperOrigin-RevId: 225636987
Change-Id: I7e32beb7751f566f5b41682a29a14442c1aa56c2
Upstream-commit: 13e4930
@golang golang locked and limited conversation to collaborators Dec 16, 2019
@rsc rsc unassigned dmitshur and bcmills Jun 23, 2022
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. release-blocker Soon This needs to be done soon. (regressions, serious bugs, outages)
Projects
None yet
Development

No branches or pull requests

9 participants