Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bug: we can't deal with branches with slashes / #19

Closed
cafkafk opened this issue Oct 30, 2023 · 1 comment · Fixed by #21
Closed

bug: we can't deal with branches with slashes / #19

cafkafk opened this issue Oct 30, 2023 · 1 comment · Fixed by #21

Comments

@cafkafk
Copy link
Owner

cafkafk commented Oct 30, 2023 •
edited
Loading

Discovered in #18, honestly not something I'm personally super interested in trying to solve, but here's an issue so we don't forget.
@algernon
Copy link
Collaborator 

❯ curl -i 'http://127.0.0.1:3000/v1/forgejo/git.madhouse-project.org/algernon/riemann-c-client/b/test%2fbranch.tar.gz' 
HTTP/1.1 303 See Other
location: https://git.madhouse-project.org/algernon/riemann-c-client/archive/test/branch.tar.gz
content-length: 0
date: Mon, 30 Oct 2023 19:24:42 GMT

Just need to url encode! :)

But we can do better: just replace /:branch in the routes with /*branch, so it captures the rest of the path. Since branch is always at the end, I believe this can be done safely. It will only captures paths, not query strings. Since the /b/ or /branch/ part of the URI will route the request there, we won't have problems accidentally routing the request elsewhere, either.

@algernon algernon mentioned this issue Oct 30, 2023
Merged
cafkafk pushed a commit that referenced this issue Nov 1, 2023
@algernon @cafkafk
bug(api): handle branch names with slashes in them
236277f 
When dealing with branches, don't capture just one slice of the path as
the branch name, but every part of the path from that point on. This
lets us easily deal with branches that have slashes in their names.

Since the branch is always the last part of the URI, and the `/b/` or
`/branch/` part routes things here anyway, capturing the rest of the
path is safe. Only the path is captured, query strings are not, those
aren't part of the routing.

Fixes #19.

Signed-off-by: Gergely Nagy <me@gergo.csillger.hu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

bug(api): handle branch names with slashes in them
2 participants
@algernon @cafkafk