Fix Go 1.13 private repository go get issue #8100

rutgerbrf · 2019-09-05T10:36:06Z

Signed-off-by: Rutger Broekhoff <rutger@viasalix.nl>

lunny · 2019-09-05T11:18:00Z

modules/context/context.go

@@ -249,6 +249,17 @@ func Contexter() macaron.Handler {
 		if ctx.Query("go-get") == "1" {
 			ownerName := c.Params(":username")
 			repoName := c.Params(":reponame")
+			if ownerName == "" || repoName == "" {
+				_, _ = c.Write([]byte(`<!doctype html>


if returned error, you should give a 500 I think.

Why not see the problem the other way and simply ignore go-get params instead of erroring ?

if ctx.Query("go-get") == "1" && ownerName != "" && repoName != "" { }

This could be break down in two if obviously.

if ctx.Query("go-get") == "1" ownerName := c.Params(":username") repoName := c.Params(":reponame") if ownerName != "" && repoName != "" { ... Do go-get work } }

But else we still need return 404 but not continue the other work.

Once ctx.Query("go-get") == "1" , it should return anyway here.

I think a 400 should be returned because in this case the client has not provided adequate information for the server to provide an import URL from the its request.

gitea/routers/repo/http.go

Line 58 in 85202d4

reponame := strings.TrimSuffix(ctx.Params(":reponame"), ".git")

I suppose this should also be applied in this case.

Or perhaps in trimmedRepoName, such that this is not interfered with:

gitea/modules/context/context.go

Line 258 in bb609ca

prefix := setting.AppURL + path.Join(url.PathEscape(ownerName), url.PathEscape(repoName), "src", "branch", util.PathEscapeSegments(branchName))

Why we should return an error ? For exemple github doesn't trigger an error on an profile if you use go-get param. Futhermore triggering an error from a supervision point could be misleading. I will not block as it need a quick fix.

lunny · 2019-09-05T11:18:41Z

modules/context/repo.go

@@ -202,6 +202,9 @@ func ComposeGoGetImport(owner, repo string) string {
 func EarlyResponseForGoGetMeta(ctx *Context) {
 	username := ctx.Params(":username")
 	reponame := ctx.Params(":reponame")
+	if username == "" || reponame == "" {
+		ctx.PlainText(404, []byte("invalid repository path"))


sapk · 2019-09-05T12:17:14Z

modules/context/repo.go

@@ -202,6 +202,9 @@ func ComposeGoGetImport(owner, repo string) string {
 func EarlyResponseForGoGetMeta(ctx *Context) {


Same here we can check previously to calling this method and maybe pass username and reponame as args since they are already extracted and cleaned from context before calling:

gitea/routers/repo/http.go

Line 58 in 85202d4

reponame := strings.TrimSuffix(ctx.Params(":reponame"), ".git")

Would it not be preferable to add this for username as well in case only .git is appended to the Gitea instance URL?

I guess not because it seems possible to have the suffix .git in a username or organization name.

Signed-off-by: Rutger Broekhoff <rutger@viasalix.nl>

techknowlogick

This should be sent to master branch first. I'm going to mark this as blocked so we don't merge this prematurely.

lunny · 2019-09-06T03:46:20Z

@rutgerbrf please send a PR to master also.

Signed-off-by: Rutger Broekhoff <rutger@viasalix.nl>

* Fix Go 1.13 invalid import path creation Signed-off-by: Rutger Broekhoff <rutger@viasalix.nl> * Apply suggested changes from #8100 Signed-off-by: Rutger Broekhoff <rutger@viasalix.nl>

lunny · 2019-09-06T13:47:07Z

Since #8112 merged, I have removed the blocked label. @techknowlogick

lafriks · 2019-09-06T13:58:15Z

@techknowlogick it is been merged to master please approve

merged to master

Fix Go 1.13 invalid import path creation

c598f7d

Signed-off-by: Rutger Broekhoff <rutger@viasalix.nl>

lunny added the type/bug label Sep 5, 2019

lunny added this to the 1.10.0 milestone Sep 5, 2019

lunny added the backport/v1.9 label Sep 5, 2019

lunny requested changes Sep 5, 2019

View reviewed changes

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 5, 2019

sapk reviewed Sep 5, 2019

View reviewed changes

Apply suggested changes from #8100

275c833

Signed-off-by: Rutger Broekhoff <rutger@viasalix.nl>

lunny approved these changes Sep 5, 2019

View reviewed changes

GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Sep 5, 2019

lunny mentioned this pull request Sep 6, 2019

Go get from a git site will not read git config insteadOf any more on go1.13 but go1.12 golang/go#34102

Closed

guillep2k approved these changes Sep 6, 2019

View reviewed changes

GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Sep 6, 2019

Merge branch 'release/v1.9' into fix-go1.13-invalid-import-path

9270a74

techknowlogick previously requested changes Sep 6, 2019

View reviewed changes

techknowlogick added the status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR label Sep 6, 2019

lunny removed the backport/v1.9 label Sep 6, 2019

lunny modified the milestones: 1.10.0, 1.9.3 Sep 6, 2019

lunny pushed a commit to lunny/gitea that referenced this pull request Sep 6, 2019

Apply suggested changes from go-gitea#8100

cea91f6

Signed-off-by: Rutger Broekhoff <rutger@viasalix.nl>

lunny mentioned this pull request Sep 6, 2019

Fix Go 1.13 private repository go get issue #8112

Merged

lunny removed the status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR label Sep 6, 2019

techknowlogick merged commit 89c5748 into go-gitea:release/v1.9 Sep 6, 2019

rutgerbrf deleted the fix-go1.13-invalid-import-path branch December 24, 2019 09:23

go-gitea locked and limited conversation to collaborators Nov 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Go 1.13 private repository go get issue #8100

Fix Go 1.13 private repository go get issue #8100

rutgerbrf commented Sep 5, 2019

lunny Sep 5, 2019

sapk Sep 5, 2019

sapk Sep 5, 2019

lunny Sep 5, 2019

lunny Sep 5, 2019

rutgerbrf Sep 5, 2019 •

edited

rutgerbrf Sep 5, 2019 •

edited

rutgerbrf Sep 5, 2019

sapk Sep 5, 2019 •

edited

lunny Sep 5, 2019

sapk Sep 5, 2019

rutgerbrf Sep 5, 2019

rutgerbrf Sep 5, 2019

techknowlogick left a comment

lunny commented Sep 6, 2019 •

edited

lunny commented Sep 6, 2019

lafriks commented Sep 6, 2019

		@@ -202,6 +202,9 @@ func ComposeGoGetImport(owner, repo string) string {
		func EarlyResponseForGoGetMeta(ctx *Context) {

Fix Go 1.13 private repository go get issue #8100

Fix Go 1.13 private repository go get issue #8100

Conversation

rutgerbrf commented Sep 5, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rutgerbrf Sep 5, 2019 • edited

Choose a reason for hiding this comment

rutgerbrf Sep 5, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sapk Sep 5, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

techknowlogick left a comment

Choose a reason for hiding this comment

lunny commented Sep 6, 2019 • edited

lunny commented Sep 6, 2019

lafriks commented Sep 6, 2019

rutgerbrf Sep 5, 2019 •

edited

rutgerbrf Sep 5, 2019 •

edited

sapk Sep 5, 2019 •

edited

lunny commented Sep 6, 2019 •

edited