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: SUMDB detection from GOPROXY fails when GOPROXY is a file URL #32227

Closed
bcmills opened this issue May 24, 2019 · 5 comments
Closed

cmd/go: SUMDB detection from GOPROXY fails when GOPROXY is a file URL #32227

bcmills opened this issue May 24, 2019 · 5 comments
Assignees
Milestone

Comments

@bcmills
Copy link
Member

@bcmills bcmills commented May 24, 2019

I believe this is the root cause of the symptom reported in #32216.

(*modfetch.dbClient).initBase attempts to derive the GOSUMDB setting from the GOPROXY setting. GOPROXY may be set to a file:// URL, in which case the call to web.GetBytes here will fail.

Since the sum database requires an HTTPS transport, we should probably default it to either direct or off if the only GOPROXY values are file URLs.

CC @FiloSottile @bradfitz @jayconrod

@bcmills bcmills changed the title cmd/go: defaul SUMDB to 'off' or 'direct' when GOPROXY is a file URL cmd/go: default SUMDB to 'off' or 'direct' when GOPROXY is a file URL May 24, 2019
@bcmills bcmills added this to the Go1.13 milestone May 24, 2019
@pyjac

This comment has been minimized.

Copy link

@pyjac pyjac commented May 24, 2019

@bcmills I'm new here, can I send a PR for this?

base on your suggestion, I believe having a check like this should work, I might be wrong

if strings.HasPrefix(proxyURL, "file://") {
	break
}
@bcmills

This comment has been minimized.

Copy link
Member Author

@bcmills bcmills commented May 24, 2019

@pyjac, the behavior here is pretty subtle, so I'd rather have @rsc clarify the intended behavior before we move forward. Thanks!

@bcmills bcmills changed the title cmd/go: default SUMDB to 'off' or 'direct' when GOPROXY is a file URL cmd/go: SUMDB detection from GOPROXY fails when GOPROXY is a file URL May 30, 2019
@bcmills bcmills assigned bcmills and unassigned rsc Jun 4, 2019
@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Jun 4, 2019

To clarify, it's not trying to derive the GOSUMDB setting so much as the connection transport for getting to it.

I think we do want file:// proxies to be able to provide GOSUMDB files, since the checksum database is (or can be) a static file tree too. But when it does try the file URL, it needs to get a 404 from the sumdb/supported endpoint, so that it can fall back to the next thing it would normally try (probably a direct connection).

So I think we just need to make the checksum fetches use the same access mechanism that the proxy fetches do, so that file:// is implemented during that fetch, and we also need to make sure that the response is a not-found, but that seems like it will work OK, since it is already using errors.Is(err, os.ErrNotExist).

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Jun 6, 2019

Change https://golang.org/cl/181037 mentions this issue: cmd/go/internal/web: support file:// URLs

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Jun 6, 2019

Worth remembering to check that <meta> tags can't point at file:/// if you make the general web library handle those.

@gopherbot gopherbot closed this in ec3ebf7 Jun 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.