Skip to content

Commit

Permalink
docs(gitlab): Update the comment about url encoding
Browse files Browse the repository at this point in the history
Other parts of the GitLab API use the user and repo names without URL
encoding, so there really is no need to encode *those*. Thus, the only
remaining part that needs encoding, is the `/` path separator, which we
manually url-encoded to `%2F`.

If we encoded the user and repo too, that might lead to surprising
results later down the line. To not hide problems, leaving them as-is is
the preferable way.

Update the comment on `Gitlab::get_api_releases_url` accordingly.

Fixes #12.

Signed-off-by: Gergely Nagy <me@gergo.csillger.hu>
  • Loading branch information
algernon authored and cafkafk committed Nov 3, 2023
1 parent 244eb17 commit 10a6066
Showing 1 changed file with 8 additions and 2 deletions.
10 changes: 8 additions & 2 deletions src/api/v1/forges/gitlab.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,14 @@ impl Forge for Gitlab {
repo: &str,
page_size: u8,
) -> Result<String, ForgeError> {
// TODO: The middle part, `{}%2F{}` is really `user/repo` URL-encoded. We
// should do proper URL encoding.
// NOTE: The middle part, `{}%2F{}` *is* correct that way. The only part of
// the path we need to URL-encode is the `/` separator. We use the user and
// repo without url-encoding for every other Gitlab endpoint, and none of
// those require url encoding, because the forge does not allow user- and
// repo names that would.
//
// Thus, only encoding the separator here is the correct approach, because
// that doesn't hide problems.
Ok(format!(
"https://{}/api/v4/projects/{}%2F{}/releases?per_page={}",
host, user, repo, page_size
Expand Down

0 comments on commit 10a6066

Please sign in to comment.