fix: path-escape file paths and eliminate url package shadowing #17

Merged
rodin merged 4 commits from fix/url-escaping-and-shadow into main 2026-05-01 21:55:02 +00:00
4 changed files with 65 additions and 17 deletions
+38 -17
View File
@@ -1,3 +1,6 @@
// Package gitea provides a client for the Gitea API.
// It supports pull request operations, file content retrieval,
// and review submission.
package gitea package gitea
import ( import (
@@ -56,8 +59,8 @@ type ChangedFile struct {
// GetPullRequest fetches PR metadata. // GetPullRequest fetches PR metadata.
func (c *Client) GetPullRequest(ctx context.Context, owner, repo string, number int) (*PullRequest, error) { func (c *Client) GetPullRequest(ctx context.Context, owner, repo string, number int) (*PullRequest, error) {
url := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d", c.baseURL, owner, repo, number) reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d", c.baseURL, owner, repo, number)
body, err := c.doGet(ctx, url) body, err := c.doGet(ctx, reqURL)
if err != nil { if err != nil {
return nil, fmt.Errorf("fetch PR: %w", err) return nil, fmt.Errorf("fetch PR: %w", err)
} }
@@ -70,8 +73,8 @@ func (c *Client) GetPullRequest(ctx context.Context, owner, repo string, number
// GetPullRequestDiff fetches the unified diff for a PR. // GetPullRequestDiff fetches the unified diff for a PR.
func (c *Client) GetPullRequestDiff(ctx context.Context, owner, repo string, number int) (string, error) { func (c *Client) GetPullRequestDiff(ctx context.Context, owner, repo string, number int) (string, error) {
url := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d.diff", c.baseURL, owner, repo, number) reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d.diff", c.baseURL, owner, repo, number)
body, err := c.doGet(ctx, url) body, err := c.doGet(ctx, reqURL)
if err != nil { if err != nil {
return "", fmt.Errorf("fetch diff: %w", err) return "", fmt.Errorf("fetch diff: %w", err)
} }
@@ -80,8 +83,8 @@ func (c *Client) GetPullRequestDiff(ctx context.Context, owner, repo string, num
// GetPullRequestFiles fetches the list of files changed in a PR. // GetPullRequestFiles fetches the list of files changed in a PR.
func (c *Client) GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]ChangedFile, error) { func (c *Client) GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]ChangedFile, error) {
url := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/files", c.baseURL, owner, repo, number) reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/files", c.baseURL, owner, repo, number)
body, err := c.doGet(ctx, url) body, err := c.doGet(ctx, reqURL)
if err != nil { if err != nil {
return nil, fmt.Errorf("fetch PR files: %w", err) return nil, fmt.Errorf("fetch PR files: %w", err)
} }
@@ -94,8 +97,8 @@ func (c *Client) GetPullRequestFiles(ctx context.Context, owner, repo string, nu
// GetCommitStatuses fetches CI statuses for a commit SHA. // GetCommitStatuses fetches CI statuses for a commit SHA.
func (c *Client) GetCommitStatuses(ctx context.Context, owner, repo, sha string) ([]CommitStatus, error) { func (c *Client) GetCommitStatuses(ctx context.Context, owner, repo, sha string) ([]CommitStatus, error) {
url := fmt.Sprintf("%s/api/v1/repos/%s/%s/commits/%s/statuses", c.baseURL, owner, repo, sha) reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/commits/%s/statuses", c.baseURL, owner, repo, sha)
body, err := c.doGet(ctx, url) body, err := c.doGet(ctx, reqURL)
if err != nil { if err != nil {
return nil, fmt.Errorf("fetch commit statuses: %w", err) return nil, fmt.Errorf("fetch commit statuses: %w", err)
} }
@@ -108,8 +111,8 @@ func (c *Client) GetCommitStatuses(ctx context.Context, owner, repo, sha string)
// GetFileContent fetches a file from the default branch of a repo. // GetFileContent fetches a file from the default branch of a repo.
func (c *Client) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) { func (c *Client) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) {
url := fmt.Sprintf("%s/api/v1/repos/%s/%s/raw/%s", c.baseURL, owner, repo, filepath) reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/raw/%s", c.baseURL, owner, repo, escapePath(filepath))
body, err := c.doGet(ctx, url) body, err := c.doGet(ctx, reqURL)
if err != nil { if err != nil {
return "", fmt.Errorf("fetch file %s: %w", filepath, err) return "", fmt.Errorf("fetch file %s: %w", filepath, err)
} }
@@ -118,7 +121,7 @@ func (c *Client) GetFileContent(ctx context.Context, owner, repo, filepath strin
// GetFileContentRef fetches a file from a specific ref (branch/tag/sha) in a repo. // GetFileContentRef fetches a file from a specific ref (branch/tag/sha) in a repo.
func (c *Client) GetFileContentRef(ctx context.Context, owner, repo, filepath, ref string) (string, error) { func (c *Client) GetFileContentRef(ctx context.Context, owner, repo, filepath, ref string) (string, error) {
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/raw/%s?ref=%s", c.baseURL, owner, repo, filepath, url.QueryEscape(ref)) reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/raw/%s?ref=%s", c.baseURL, owner, repo, escapePath(filepath), url.QueryEscape(ref))
body, err := c.doGet(ctx, reqURL) body, err := c.doGet(ctx, reqURL)
if err != nil { if err != nil {
return "", fmt.Errorf("fetch file %s@%s: %w", filepath, ref, err) return "", fmt.Errorf("fetch file %s@%s: %w", filepath, ref, err)
@@ -129,7 +132,7 @@ func (c *Client) GetFileContentRef(ctx context.Context, owner, repo, filepath, r
// PostReview submits a review to a PR. // PostReview submits a review to a PR.
// event should be "APPROVED" or "REQUEST_CHANGES". // event should be "APPROVED" or "REQUEST_CHANGES".
func (c *Client) PostReview(ctx context.Context, owner, repo string, number int, event, body string) error { func (c *Client) PostReview(ctx context.Context, owner, repo string, number int, event, body string) error {
url := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews", c.baseURL, owner, repo, number) reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews", c.baseURL, owner, repo, number)
payload := struct { payload := struct {
Body string `json:"body"` Body string `json:"body"`
@@ -144,7 +147,7 @@ func (c *Client) PostReview(ctx context.Context, owner, repo string, number int,
return fmt.Errorf("marshal review payload: %w", err) return fmt.Errorf("marshal review payload: %w", err)
} }
req, err := http.NewRequestWithContext(ctx, "POST", url, bytes.NewReader(data)) req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, bytes.NewReader(data))
if err != nil { if err != nil {
return fmt.Errorf("create review request: %w", err) return fmt.Errorf("create review request: %w", err)
} }
@@ -164,8 +167,8 @@ func (c *Client) PostReview(ctx context.Context, owner, repo string, number int,
return nil return nil
} }
func (c *Client) doGet(ctx context.Context, url string) ([]byte, error) { func (c *Client) doGet(ctx context.Context, reqURL string) ([]byte, error) {
req, err := http.NewRequestWithContext(ctx, "GET", url, nil) req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@@ -184,6 +187,18 @@ func (c *Client) doGet(ctx context.Context, url string) ([]byte, error) {
return io.ReadAll(resp.Body) return io.ReadAll(resp.Body)
} }
// escapePath escapes each segment of a relative file path for use in URLs.
// Slashes are preserved as path separators; other special characters are escaped.
// Input should be a relative path (no leading slash). Already-encoded segments
// will be double-encoded, which is the desired behavior for user-provided paths.
func escapePath(p string) string {
parts := strings.Split(p, "/")
for i, part := range parts {
parts[i] = url.PathEscape(part)
}
return strings.Join(parts, "/")
}
// ContentEntry represents a file or directory entry from the contents API. // ContentEntry represents a file or directory entry from the contents API.
type ContentEntry struct { type ContentEntry struct {
Name string `json:"name"` Name string `json:"name"`
@@ -192,9 +207,15 @@ type ContentEntry struct {
} }
// ListContents lists files and directories at a given path in a repo. // ListContents lists files and directories at a given path in a repo.
// Pass an empty path to list the repository root.
func (c *Client) ListContents(ctx context.Context, owner, repo, path string) ([]ContentEntry, error) { func (c *Client) ListContents(ctx context.Context, owner, repo, path string) ([]ContentEntry, error) {
url := fmt.Sprintf("%s/api/v1/repos/%s/%s/contents/%s", c.baseURL, owner, repo, path) var reqURL string
body, err := c.doGet(ctx, url) if path == "" {
reqURL = fmt.Sprintf("%s/api/v1/repos/%s/%s/contents", c.baseURL, owner, repo)
} else {
reqURL = fmt.Sprintf("%s/api/v1/repos/%s/%s/contents/%s", c.baseURL, owner, repo, escapePath(path))
}
body, err := c.doGet(ctx, reqURL)
if err != nil { if err != nil {
return nil, fmt.Errorf("list contents %s: %w", path, err) return nil, fmt.Errorf("list contents %s: %w", path, err)
} }
+24
View File
@@ -294,3 +294,27 @@ func TestGetAllFilesInPath_File(t *testing.T) {
t.Errorf("unexpected content: %q", files["README.md"]) t.Errorf("unexpected content: %q", files["README.md"])
} }
} }
func TestEscapePath(t *testing.T) {
tests := []struct {
name string
input string
want string
}{
{"simple", "src/main.go", "src/main.go"},
{"spaces", "my dir/my file.go", "my%20dir/my%20file.go"},
{"special chars", "path/file#1.txt", "path/file%231.txt"},
{"empty", "", ""},
{"single segment", "README.md", "README.md"},
{"nested deep", "a/b/c/d.md", "a/b/c/d.md"},
{"already encoded", "path/file%20name.go", "path/file%2520name.go"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := escapePath(tt.input)
if got != tt.want {
t.Errorf("escapePath(%q) = %q, want %q", tt.input, got, tt.want)
}
})
}
}
+1
View File
@@ -1,3 +1,4 @@
// Package llm provides a client for OpenAI-compatible chat completion APIs.
package llm package llm
import ( import (
+2
View File
@@ -1,3 +1,5 @@
// Package review builds prompts for AI code review and parses LLM responses
// into structured review results.
package review package review
import ( import (