fix: path-escape file paths and eliminate url package shadowing
- Add escapePath() helper: escapes each path segment individually (preserves slashes as separators, escapes spaces/#/? etc) - Apply to GetFileContent, GetFileContentRef, ListContents - Rename doGet parameter from url to reqURL (avoids shadowing net/url) - Rename local variables in GetFileContent/ListContents for consistency Addresses remaining findings from PR #16 review.
This commit is contained in:
+17
-7
@@ -108,8 +108,8 @@ func (c *Client) GetCommitStatuses(ctx context.Context, owner, repo, sha string)
|
||||
|
||||
// GetFileContent fetches a file from the default branch of a repo.
|
||||
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)
|
||||
body, err := c.doGet(ctx, url)
|
||||
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/raw/%s", c.baseURL, owner, repo, escapePath(filepath))
|
||||
body, err := c.doGet(ctx, reqURL)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("fetch file %s: %w", filepath, err)
|
||||
}
|
||||
@@ -118,7 +118,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.
|
||||
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)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("fetch file %s@%s: %w", filepath, ref, err)
|
||||
@@ -164,8 +164,8 @@ func (c *Client) PostReview(ctx context.Context, owner, repo string, number int,
|
||||
return nil
|
||||
}
|
||||
|
||||
func (c *Client) doGet(ctx context.Context, url string) ([]byte, error) {
|
||||
req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
|
||||
func (c *Client) doGet(ctx context.Context, reqURL string) ([]byte, error) {
|
||||
req, err := http.NewRequestWithContext(ctx, "GET", reqURL, nil)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
@@ -184,6 +184,16 @@ func (c *Client) doGet(ctx context.Context, url string) ([]byte, error) {
|
||||
return io.ReadAll(resp.Body)
|
||||
}
|
||||
|
||||
// escapePath escapes each segment of a file path for use in URLs.
|
||||
// Slashes are preserved as path separators; other special characters are escaped.
|
||||
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.
|
||||
type ContentEntry struct {
|
||||
Name string `json:"name"`
|
||||
@@ -193,8 +203,8 @@ type ContentEntry struct {
|
||||
|
||||
// ListContents lists files and directories at a given path in a repo.
|
||||
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)
|
||||
body, err := c.doGet(ctx, url)
|
||||
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 {
|
||||
return nil, fmt.Errorf("list contents %s: %w", path, err)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user