fix: consistent url.PathEscape across all Gitea client endpoints #30

2026-05-02T09:42:04Z

rodin commented

2026-05-02 09:42:04 +00:00

Summary

Apply url.PathEscape to owner, repo, and sha path segments in all methods that were previously interpolating raw values directly.

Problem

Some endpoints (ListReviews, DeleteReview, GetTimelineReviewCommentID, EditComment) already used url.PathEscape for owner/repo, while others (GetPullRequest, GetPullRequestDiff, GetPullRequestFiles, GetCommitStatuses, GetFileContent, GetFileContentRef, PostReview, ListContents) interpolated raw values. This inconsistency was flagged in PRs #17, #20, and #22.

Fix

All methods now consistently escape owner, repo, and sha path segments. The filepath parameter was already handled by escapePath() (per-segment escaping).

Risk

Low — for typical owner/repo names this is a no-op. Only matters for names with special characters (spaces, slashes, unicode). But consistency eliminates an entire class of potential bugs.

Closes #24

## Summary Apply `url.PathEscape` to owner, repo, and sha path segments in all methods that were previously interpolating raw values directly. ## Problem Some endpoints (`ListReviews`, `DeleteReview`, `GetTimelineReviewCommentID`, `EditComment`) already used `url.PathEscape` for owner/repo, while others (`GetPullRequest`, `GetPullRequestDiff`, `GetPullRequestFiles`, `GetCommitStatuses`, `GetFileContent`, `GetFileContentRef`, `PostReview`, `ListContents`) interpolated raw values. This inconsistency was flagged in PRs #17, #20, and #22. ## Fix All methods now consistently escape owner, repo, and sha path segments. The `filepath` parameter was already handled by `escapePath()` (per-segment escaping). ## Risk Low — for typical owner/repo names this is a no-op. Only matters for names with special characters (spaces, slashes, unicode). But consistency eliminates an entire class of potential bugs. Closes #24

rodin added 1 commit 2026-05-02 09:42:04 +00:00

fix: consistent url.PathEscape across all Gitea client endpoints

CI / test (pull_request) Successful in 14s

Details

CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 21s

Details

CI / review (gpt-5, security, SECURITY_REVIEW.md, SONNET_REVIEW_TOKEN) (pull_request) Successful in 34s

Details

CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 35s

Details

c2595d0263

Apply url.PathEscape to owner, repo, and sha path segments in all
methods that were previously interpolating raw values. Methods already
using PathEscape (ListReviews, DeleteReview, GetTimelineReviewCommentID,
EditComment) are unchanged.

This eliminates an inconsistency flagged in PRs #17, #20, and #22 and
prevents potential path-injection bugs for names with special characters.

Closes #24

gpt-review-bot approved these changes 2026-05-02 09:42:39 +00:00

gpt-review-bot left a comment

First-time contributor

Gpt Review

Summary

Consistent use of url.PathEscape on all path segments (owner, repo, sha) across the client code improves correctness and eliminates ambiguity, with low risk and no observable API change for typical input.

Recommendation

APPROVE — This PR brings much-needed consistency and safety to path handling for all endpoints, ensuring that special characters in owner, repo, and sha do not break or misroute requests. All uses were already updated to apply url.PathEscape to these segments where needed, and there are no idiom, API, style, or logic violations. Well-documented, low risk, and CI passes -- merge as is.

Review by gpt

# Gpt Review ## Summary Consistent use of url.PathEscape on all path segments (owner, repo, sha) across the client code improves correctness and eliminates ambiguity, with low risk and no observable API change for typical input. ## Recommendation **APPROVE** — This PR brings much-needed consistency and safety to path handling for all endpoints, ensuring that special characters in owner, repo, and sha do not break or misroute requests. All uses were already updated to apply url.PathEscape to these segments where needed, and there are no idiom, API, style, or logic violations. Well-documented, low risk, and CI passes -- merge as is. --- *Review by gpt*

sonnet-review-bot approved these changes 2026-05-02 09:42:53 +00:00

Dismissed

sonnet-review-bot left a comment

First-time contributor

Security Review

Summary

This PR consistently applies url.PathEscape to owner, repo, and sha path segments across endpoints, improving robustness against path injection and encoding inconsistencies. CI has passed and the changes are security-hardening without altering behavior for typical inputs.

Recommendation

APPROVE — The changes reduce the risk of path traversal/injection by uniformly escaping path segments and are aligned with secure URL construction practices. No exploitable issues were introduced. As a future hardening consideration (not required for this PR), you could consider constructing URLs via net/url.URL and path.Join-style segment handling to centralize escaping and reduce string formatting errors. Approve as-is.

Review by security

# Security Review ## Summary This PR consistently applies url.PathEscape to owner, repo, and sha path segments across endpoints, improving robustness against path injection and encoding inconsistencies. CI has passed and the changes are security-hardening without altering behavior for typical inputs. ## Recommendation **APPROVE** — The changes reduce the risk of path traversal/injection by uniformly escaping path segments and are aligned with secure URL construction practices. No exploitable issues were introduced. As a future hardening consideration (not required for this PR), you could consider constructing URLs via net/url.URL and path.Join-style segment handling to centralize escaping and reduce string formatting errors. Approve as-is. --- *Review by security*

sonnet-review-bot approved these changes 2026-05-02 09:42:54 +00:00

sonnet-review-bot left a comment

First-time contributor

Sonnet Review

Summary

The changes consistently apply url.PathEscape to owner, repo, and sha path segments across endpoints, aligning with correct URL construction practices. CI passed and the implementation appears correct and low-risk.

Recommendation

APPROVE — This PR improves robustness by uniformly escaping path segments for all affected endpoints, matching existing behavior in other methods. The use of url.PathEscape for path segments and url.QueryEscape for the ref query parameter is appropriate, and the existing escapePath() continues to handle file paths per-segment. With CI passing and no functional regressions apparent, this is ready to merge. As a potential future improvement, consider centralizing URL construction (e.g., helper to build repo-scoped paths) to prevent drift and reduce duplication.

Review by sonnet

# Sonnet Review ## Summary The changes consistently apply url.PathEscape to owner, repo, and sha path segments across endpoints, aligning with correct URL construction practices. CI passed and the implementation appears correct and low-risk. ## Recommendation **APPROVE** — This PR improves robustness by uniformly escaping path segments for all affected endpoints, matching existing behavior in other methods. The use of url.PathEscape for path segments and url.QueryEscape for the ref query parameter is appropriate, and the existing escapePath() continues to handle file paths per-segment. With CI passing and no functional regressions apparent, this is ready to merge. As a potential future improvement, consider centralizing URL construction (e.g., helper to build repo-scoped paths) to prevent drift and reduce duplication. --- *Review by sonnet*

rodin merged commit 3c785c5502 into main

2026-05-02 14:01:53 +00:00

rodin referenced this issue from a commit

2026-05-02 14:01:54 +00:00

Merge pull request 'fix: consistent url.PathEscape across all Gitea client endpoints' (#30) from fix/consistent-path-escape into main

Sign in to join this conversation.

3 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: rodin/review-bot#30