UrlEscape Function Arguments used in UrlPath #273

Merged
zeripath merged 18 commits from 6543/go-sdk:EscapeUserRepoNames into master 1 month ago
6543 commented 1 year ago
Collaborator

close #271

  • add check if user/repo-name is empty
  • use net/url for links to encode
close #271 * [x] add check if user/repo-name is empty * [x] use net/url for links to encode
6543 added this to the v0.12.0 milestone 1 year ago
6543 added the
kind/enhancement
label 1 year ago
6543 commented 1 year ago
Poster
Collaborator

extracted fix to #276

extracted fix to #276
zeripath reviewed 1 year ago
Dismissed
func (c *Client) GetRepoGitHook(user, repo, id string) (*GitHook, error) {
h := new(GitHook)
return h, c.getParsedResponse("GET", fmt.Sprintf("/repos/%s/%s/hooks/git/%s", user, repo, id), nil, nil, h)
return h, c.getParsedResponse("GET", fmt.Sprintf("/repos/%s/%s/hooks/git/%s", url.PathEscape(user), url.PathEscape(repo), id), nil, nil, h)
Poster
Collaborator

If id is passed in to the library as an option it also needs to be PathEscape'd

If `id` is passed in to the library as an option it also needs to be `PathEscape`'d
6543 marked this conversation as resolved
zeripath reviewed 1 year ago
Dismissed
Poster
Collaborator

If id is passed in to the library as an option it also needs to be PathEscape'd

If `id` is passed in to the library as an option it also needs to be `PathEscape`'d
6543 marked this conversation as resolved
zeripath reviewed 1 year ago
Dismissed
return err
}
status, err := c.getStatusCode("PUT", fmt.Sprintf("/repos/%s/%s/issues/%d/subscriptions/%s", owner, repo, index, user), nil, nil)
status, err := c.getStatusCode("PUT", fmt.Sprintf("/repos/%s/%s/issues/%d/subscriptions/%s", url.PathEscape(owner), url.PathEscape(repo), index, user), nil, nil)
Poster
Collaborator

user here probably needs PathEscape too.

`user` here probably needs `PathEscape` too.
6543 marked this conversation as resolved
zeripath reviewed 1 year ago
Dismissed
func (c *Client) GetUserTrackedTimes(owner, repo, user string) ([]*TrackedTime, error) {
times := make([]*TrackedTime, 0, 10)
return times, c.getParsedResponse("GET", fmt.Sprintf("/repos/%s/%s/times/%s", owner, repo, user), nil, nil, &times)
return times, c.getParsedResponse("GET", fmt.Sprintf("/repos/%s/%s/times/%s", url.PathEscape(owner), url.PathEscape(repo), user), nil, nil, &times)
Poster
Collaborator

user here probably needs PathEscape

`user` here probably needs `PathEscape`
6543 marked this conversation as resolved
zeripath reviewed 1 year ago
Dismissed
Poster
Collaborator

user here probably needs PathEscape

`user` here probably needs `PathEscape`
6543 marked this conversation as resolved
zeripath reviewed 1 year ago
Dismissed
func (c *Client) GetBlob(user, repo, sha string) (*GitBlobResponse, error) {
blob := new(GitBlobResponse)
return blob, c.getParsedResponse("GET", fmt.Sprintf("/repos/%s/%s/git/blobs/%s", user, repo, sha), nil, nil, blob)
return blob, c.getParsedResponse("GET", fmt.Sprintf("/repos/%s/%s/git/blobs/%s", url.PathEscape(user), url.PathEscape(repo), sha), nil, nil, blob)
Poster
Collaborator

sha should probably be PathEscaped too.

`sha` should probably be `PathEscape`d too.
6543 marked this conversation as resolved
zeripath reviewed 1 year ago
Dismissed
gitea/hook.go Outdated
func (c *Client) GetOrgHook(org string, id int64) (*Hook, error) {
h := new(Hook)
return h, c.getParsedResponse("GET", fmt.Sprintf("/orgs/%s/hooks/%d", org, id), nil, nil, h)
return h, c.getParsedResponse("GET", fmt.Sprintf("/orgs/%s/hooks/%d", url.PathEscape(org), id), nil, nil, h)
Poster
Collaborator

If id is passed in to the library as an option it also needs to be PathEscape'd

If `id` is passed in to the library as an option it also needs to be `PathEscape`'d
6543 commented 1 year ago
Poster
Collaborator

id is an integer

id is an integer
6543 marked this conversation as resolved
zeripath reviewed 1 year ago
Dismissed
func (c *Client) IsCollaborator(user, repo, collaborator string) (bool, error) {
status, err := c.getStatusCode("GET", fmt.Sprintf("/repos/%s/%s/collaborators/%s", user, repo, collaborator), nil, nil)
func (c *Client) IsCollaborator(owner, repo, collaborator string) (bool, error) {
status, err := c.getStatusCode("GET", fmt.Sprintf("/repos/%s/%s/collaborators/%s", url.PathEscape(owner), url.PathEscape(repo), collaborator), nil, nil)
Poster
Collaborator

collaborator should be PathEscape'd too.

`collaborator` should be `PathEscape`'d too.
6543 marked this conversation as resolved
Collaborator

@6543 thank you for doing this! There's a few more things that probably need escaping too.

@6543 thank you for doing this! There's a few more things that probably need escaping too.
6543 commented 1 year ago
Poster
Collaborator

I'll look into it tomorow :)

I'll look into it tomorow :)
6543 commented 1 year ago
Poster
Collaborator

@zeripath done

@zeripath done
zeripath reviewed 1 year ago
Dismissed
func (c *Client) GetRepoBranch(owner, repo, branch string) (*Branch, error) {
b := new(Branch)
return b, c.getParsedResponse("GET", fmt.Sprintf("/repos/%s/%s/branches/%s", user, repo, branch), nil, nil, &b)
return b, c.getParsedResponse("GET", fmt.Sprintf("/repos/%s/%s/branches/%s", url.PathEscape(owner), url.PathEscape(repo), branch), nil, nil, &b)
Poster
Collaborator

Now this one is a bit difficult. We can't use pathescape however it's perfectly valid to have some weirdish characters in branch.

In Gitea proper we have a PathEscapeSegments:

// PathEscapeSegments escapes segments of a path while not escaping forward slash
func PathEscapeSegments(path string) string {
	slice := strings.Split(path, "/")
	for index := range slice {
		slice[index] = url.PathEscape(slice[index])
	}
	escapedPath := strings.Join(slice, "/")
	return escapedPath
}
Now this one is a bit difficult. We can't use pathescape however it's perfectly valid to have some weirdish characters in branch. In Gitea proper we have a PathEscapeSegments: ```go // PathEscapeSegments escapes segments of a path while not escaping forward slash func PathEscapeSegments(path string) string { slice := strings.Split(path, "/") for index := range slice { slice[index] = url.PathEscape(slice[index]) } escapedPath := strings.Join(slice, "/") return escapedPath } ```
6543 marked this conversation as resolved
zeripath reviewed 1 year ago
Dismissed
Poster
Collaborator

Now this one is a bit difficult. We can't use pathescape however it's perfectly valid to have some weirdish characters in branch.

In Gitea proper we have a PathEscapeSegments:

// PathEscapeSegments escapes segments of a path while not escaping forward slash
func PathEscapeSegments(path string) string {
	slice := strings.Split(path, "/")
	for index := range slice {
		slice[index] = url.PathEscape(slice[index])
	}
	escapedPath := strings.Join(slice, "/")
	return escapedPath
}
Now this one is a bit difficult. We can't use pathescape however it's perfectly valid to have some weirdish characters in branch. In Gitea proper we have a PathEscapeSegments: ```go // PathEscapeSegments escapes segments of a path while not escaping forward slash func PathEscapeSegments(path string) string { slice := strings.Split(path, "/") for index := range slice { slice[index] = url.PathEscape(slice[index]) } escapedPath := strings.Join(slice, "/") return escapedPath } ```
6543 marked this conversation as resolved
zeripath reviewed 1 year ago
Dismissed
ref = strings.TrimPrefix(ref, "refs/")
r := new(Reference)
err := c.getParsedResponse("GET", fmt.Sprintf("/repos/%s/%s/git/refs/%s", user, repo, ref), nil, nil, &r)
err := c.getParsedResponse("GET", fmt.Sprintf("/repos/%s/%s/git/refs/%s", url.PathEscape(owner), url.PathEscape(repo), ref), nil, nil, &r)
Poster
Collaborator

Similarly here a ref can have non-urlsafe characters meaning we need PathEscapeSegments

Similarly here a ref can have non-urlsafe characters meaning we need PathEscapeSegments
6543 marked this conversation as resolved
6543 added the
status/wip
label 1 year ago
6543 modified the milestone from v0.12.0 to v0.12.1 11 months ago
Owner

Please resolve the conflicts.

Please resolve the conflicts.
6543 added 16 commits 11 months ago
9f10a2b902 Add Create/Get/Delete for oauth2 apps (#305)
2f920dbb01 Add missing JSON header to `Client.AddCollaborator` (#306)
50efd911c8 Tune CheckNotifications for api change (#308)
0cf676d9f9 Add Get/Update for oauth2 apps (#311)
79665cae15 Fix MergePullRequest & extend Tests (#278)
7dfa25bb30 Add Issue Subscription Check & Fix DeleteIssueSubscription (#318)
50560273b9 Add Branch Deletion (#317)
36d2964230 API split for: to get single commit via SHA and Ref (#319)
7ae928fbc2 [Frontport] Changelog for v0.11.3 & newline fix (#321)
fb7355a186 IssueUn-/Subscription take care of new 200 status (#325)
2883376503 Revert "API split for: to get single commit via SHA and Ref (#319)" (#324)
93087537ff Corect test Title: TestIssueSubscription (#326)
70863f4458 Support 2FA for basic auth & refactor Token functions (#335)
f224b4e50c ListIssues: add milestones filter (#327)
bb6248f50d Add paggination to ListNotification functions (#339)
2cc36f912f Check if gitea is able to squash-merge via API (#336)
6543 added
invalid
and removed
invalid
labels 11 months ago
6543 added 5 commits 11 months ago
bb9144e8d6 Add Migration Guide for v0.12.0 (#343)
2e81813c45 Add BranchProtection functions (#341)
057518ef80 Add PullReview functions (#338)
567d2f8bbd Changelog v0.12.0 (#344)
e7c56d8f50 fix ineffassign in Tests & set Version v0.13.0 (#345)
6543 modified the milestone from v0.12.1 to v0.13.0 10 months ago
6543 changed title from Escape user and repo names to Escape user, repo, refs 7 months ago
6543 modified the milestone from v0.13.0 to v0.14.0 7 months ago
6543 added 1 commit 5 months ago
c0dd753977
... adoption ...
6543 added 1 commit 5 months ago
06fbde5422
... Done
6543 changed title from Escape user, repo, refs to UrlEscape Function Arguments used in UrlPath 5 months ago
6543 removed the
status/wip
label 5 months ago
Poster
Collaborator

@zeripath finished :)

CI is failing is unrelated ...

@zeripath finished :) CI is failing is unrelated ...
6543 added the
status/needs-reviews
label 5 months ago
6543 added 1 commit 5 months ago
fe238ca9d6
Update TestData
6543 added 1 commit 5 months ago
2bfe1cd507
Merge branch 'master' into EscapeUserRepoNames
6543 added 1 commit 5 months ago
713aa7b7c3 Merge branch 'master' into EscapeUserRepoNames
6543 added 1 commit 5 months ago
6c8ca797a7 Merge branch 'master' into EscapeUserRepoNames
6543 reviewed 2 months ago
Dismissed
// GetFile downloads a file of repository, ref can be branch/tag/commit.
// e.g.: ref -> master, tree -> macaron.go(no leading slash)
func (c *Client) GetFile(user, repo, ref, tree string) ([]byte, *Response, error) {
if err := escapeValidatePathSegments(&user, &repo, &ref, &tree); err != nil {
6543 commented 2 months ago
Poster
Collaborator

... escapeValidatePathSegments is not right for tree in that function
or ref
Those are allowed to have unescaped '/' in them but have to have everything else escaped...

... escapeValidatePathSegments is not right for tree in that function or ref Those are allowed to have unescaped '/' in them but have to have everything else escaped...
6543 marked this conversation as resolved
6543 added 2 commits 2 months ago
6543 added 1 commit 1 month ago
0a1d265e17
Merge branch 'master' into EscapeUserRepoNames
6543 reviewed 1 month ago
Dismissed
gitea/status.go Outdated
}
status := new(CombinedStatus)
resp, err := c.getParsedResponse("GET", fmt.Sprintf("/repos/%s/%s/commits/%s/status", owner, repo, ref), jsonHeader, nil, status)
resp, err := c.getParsedResponse("GET", fmt.Sprintf("/repos/%s/%s/commits/%s/status", owner, url.QueryEscape(ref), ref), jsonHeader, nil, status)
6543 commented 1 month ago
Poster
Collaborator

false params!

false params!
6543 marked this conversation as resolved
6543 reviewed 1 month ago
Dismissed
6543 added 1 commit 1 month ago
0a9af0f1b0
fix issues
6543 added 1 commit 1 month ago
00e007e87f
finish
Collaborator

Looks like you missed func (c *Client) issueBackwardsCompatibility(issue *Issue) {

Looks like you missed `func (c *Client) issueBackwardsCompatibility(issue *Issue) {`
Poster
Collaborator

@zeripath issueBackwardsCompatibility dont need that

@zeripath **issueBackwardsCompatibility** dont need that
Poster
Collaborator

should we add some notes to README, that the sdk now escape things by itselve?

should we add some notes to README, that the sdk now escape things by itselve?
zeripath approved these changes 1 month ago
Dismissed
6543 added the
kind/breaking
label 1 month ago
6543 added 1 commit 1 month ago
7998124ed3
docs
6543 modified the milestone from v0.14.0 to v0.14.1 1 month ago
6543 modified the milestone from v0.14.1 to v0.15.0 1 month ago
techknowlogick approved these changes 1 month ago
Dismissed
zeripath merged commit 6b6fdd91ce into master 1 month ago
6543 deleted branch EscapeUserRepoNames 1 month ago
6543 removed the
status/needs-reviews
label 1 month ago
6543 modified the milestone from v0.15.0 to v0.14.0 1 month ago
continuous-integration/drone/pr Build is passing
The pull request has been merged as 6b6fdd91ce.
Sign in to join this conversation.
Loading…
There is no content yet.