commit 7c9d6b22d0122bf65c53374cb1d24559dcd3c558
author Heschi Kreinick <heschi@google.com> Tue Nov 14 13:43:12 2023 -0500
committer Gopher Robot <gobot@golang.org> Thu Nov 16 16:37:11 2023 +0000
tree 37ea6a73cbfa022d7d20dcb866b3779936033652
parent cd7fd6ef1e0a4c054ea33877d79cb6607bf63e67

gerrit: fix project escaping for good (?)

Every time I try a new API with the internal project I find that it
doesn't escape the path properly. Try to get it right everywhere.

For golang/go#63147.

Change-Id: I16b4bd32f2b014208e5f058028d7212e8f436c5c
Reviewed-on: https://go-review.googlesource.com/c/build/+/542656
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Heschi Kreinick <heschi@google.com>

gerrit/gerrit.go

diff --git a/gerrit/gerrit.go b/gerrit/gerrit.go
index 66c9bd2..de7839b 100644
--- a/gerrit/gerrit.go
+++ b/gerrit/gerrit.go
@@ -777,7 +777,7 @@
 		pi = p[0]
 	}
 	var res ProjectInfo
-	err := c.do(ctx, &res, "PUT", fmt.Sprintf("/projects/%s", name), reqBodyJSON{&pi}, wantResStatus(http.StatusCreated))
+	err := c.do(ctx, &res, "PUT", fmt.Sprintf("/projects/%s", url.PathEscape(name)), reqBodyJSON{&pi}, wantResStatus(http.StatusCreated))
 	return res, err
 }
 
@@ -803,7 +803,7 @@
 //
 // See https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#put-edit-file.
 func (c *Client) ChangeFileContentInChangeEdit(ctx context.Context, changeID string, path string, content string) error {
-	return c.do(ctx, nil, "PUT", "/changes/"+changeID+"/edit/"+url.QueryEscape(path),
+	return c.do(ctx, nil, "PUT", "/changes/"+changeID+"/edit/"+url.PathEscape(path),
 		reqBodyRaw{strings.NewReader(content)}, wantResStatus(http.StatusNoContent))
 }
 
@@ -812,7 +812,7 @@
 //
 // See https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#delete-edit-file.
 func (c *Client) DeleteFileInChangeEdit(ctx context.Context, changeID string, path string) error {
-	return c.do(ctx, nil, "DELETE", "/changes/"+changeID+"/edit/"+url.QueryEscape(path), wantResStatus(http.StatusNoContent))
+	return c.do(ctx, nil, "DELETE", "/changes/"+changeID+"/edit/"+url.PathEscape(path), wantResStatus(http.StatusNoContent))
 }
 
 // PublishChangeEdit promotes the change edit to a regular patch set.
@@ -825,7 +825,7 @@
 // GetProjectInfo returns info about a project.
 func (c *Client) GetProjectInfo(ctx context.Context, name string) (ProjectInfo, error) {
 	var res ProjectInfo
-	err := c.do(ctx, &res, "GET", fmt.Sprintf("/projects/%s", name))
+	err := c.do(ctx, &res, "GET", fmt.Sprintf("/projects/%s", url.PathEscape(name)))
 	return res, err
 }
 
@@ -841,7 +841,7 @@
 // keyed by reference.
 func (c *Client) GetProjectBranches(ctx context.Context, name string) (map[string]BranchInfo, error) {
 	var res []BranchInfo
-	err := c.do(ctx, &res, "GET", fmt.Sprintf("/projects/%s/branches/", name))
+	err := c.do(ctx, &res, "GET", fmt.Sprintf("/projects/%s/branches/", url.PathEscape(name)))
 	if err != nil {
 		return nil, err
 	}
@@ -857,7 +857,7 @@
 // See https://gerrit-review.googlesource.com/Documentation/rest-api-projects.html#get-branch.
 func (c *Client) GetBranch(ctx context.Context, project, branch string) (BranchInfo, error) {
 	var res BranchInfo
-	err := c.do(ctx, &res, "GET", fmt.Sprintf("/projects/%s/branches/%s", project, branch))
+	err := c.do(ctx, &res, "GET", fmt.Sprintf("/projects/%s/branches/%s", url.PathEscape(project), branch))
 	return res, err
 }
 
@@ -866,7 +866,7 @@
 // See https://gerrit-review.googlesource.com/Documentation/rest-api-projects.html#get-content-from-commit.
 func (c *Client) GetFileContent(ctx context.Context, project, commit, path string) (io.ReadCloser, error) {
 	var body io.ReadCloser
-	err := c.do(ctx, nil, "GET", fmt.Sprintf("/projects/%s/commits/%s/files/%s/content", project, commit, url.QueryEscape(path)), respBodyRaw{&body})
+	err := c.do(ctx, nil, "GET", fmt.Sprintf("/projects/%s/commits/%s/files/%s/content", url.PathEscape(project), commit, url.PathEscape(path)), respBodyRaw{&body})
 	if err != nil {
 		return nil, err
 	}
@@ -935,7 +935,7 @@
 // reference.
 func (c *Client) GetProjectTags(ctx context.Context, name string) (map[string]TagInfo, error) {
 	var res []TagInfo
-	err := c.do(ctx, &res, "GET", fmt.Sprintf("/projects/%s/tags/", name))
+	err := c.do(ctx, &res, "GET", fmt.Sprintf("/projects/%s/tags/", url.PathEscape(name)))
 	if err != nil {
 		return nil, err
 	}
@@ -951,7 +951,7 @@
 // See https://gerrit-review.googlesource.com/Documentation/rest-api-projects.html#get-tag.
 func (c *Client) GetTag(ctx context.Context, project, tag string) (TagInfo, error) {
 	var res TagInfo
-	err := c.do(ctx, &res, "GET", fmt.Sprintf("/projects/%s/tags/%s", project, url.PathEscape(tag)))
+	err := c.do(ctx, &res, "GET", fmt.Sprintf("/projects/%s/tags/%s", url.PathEscape(project), url.PathEscape(tag)))
 	return res, err
 }
 
@@ -1168,6 +1168,6 @@
 	vals := url.Values{}
 	vals["commit"] = commits
 	vals["ref"] = refs
-	err := c.do(ctx, &result, "GET", "/projects/"+project+"/commits:in", urlValues(vals))
+	err := c.do(ctx, &result, "GET", "/projects/"+url.PathEscape(project)+"/commits:in", urlValues(vals))
 	return result, err
 }