[gh api] Escape package name (URL encoding) for packages endpoint by iamazeem · Pull Request #10384 · cli/cli

iamazeem · 2025-02-07T08:29:28Z

According to the REST API endpoints for packages, the following endpoints need to be handled (duplicates removed):

/orgs/{org}/packages/{package_type}/{package_name}
/orgs/{org}/packages/{package_type}/{package_name}/restore
/orgs/{org}/packages/{package_type}/{package_name}/versions
/orgs/{org}/packages/{package_type}/{package_name}/versions/{package_version_id}
/orgs/{org}/packages/{package_type}/{package_name}/versions/{package_version_id}/restore
/user/packages/{package_type}/{package_name}
/user/packages/{package_type}/{package_name}/restore
/user/packages/{package_type}/{package_name}/versions
/user/packages/{package_type}/{package_name}/versions/{package_version_id}
/user/packages/{package_type}/{package_name}/versions/{package_version_id}/restore
/users/{username}/packages/{package_type}/{package_name}
/users/{username}/packages/{package_type}/{package_name}/restore
/users/{username}/packages/{package_type}/{package_name}/versions
/users/{username}/packages/{package_type}/{package_name}/versions/{package_version_id}
/users/{username}/packages/{package_type}/{package_name}/versions/{package_version_id}/restore

where package_type can be one of:

npm, maven, rubygems, docker, nuget, container

Usually, docker and container packages may have such naming conventions.
However, this PR handles all these package types, just to be sure.

Used regex to extract the package name:

regex101: https://regex101.com/r/ftredu/1
go playground: https://go.dev/play/p/6xK_iva7r-h

Tests

Packages for testing: https://github.com/Skarlso?ecosystem=container&tab=packages

# FAILURE
$ gh api /users/Skarlso/packages/container/helm/crd-bootstrap
{
  "message": "Not Found",
  "documentation_url": "https://docs.github.com/rest",
  "status": "404"
}
gh: Not Found (HTTP 404)

# SUCCESS
$ bin/gh api /users/Skarlso/packages/container/helm/crd-bootstrap
{
  "id": 6080213,
  "name": "helm/crd-bootstrap",
  "package_type": "container",
  ...
}

With /versions and /versions/{package_version_id}:

# FAILURE
$ gh api /users/Skarlso/packages/container/helm/crd-bootstrap/versions
{
  "message": "Not Found",
  "documentation_url": "https://docs.github.com/rest",
  "status": "404"
}
gh: Not Found (HTTP 404)

# SUCCESS
$ bin/gh api /users/Skarlso/packages/container/helm/crd-bootstrap/versions
[
  {
    "id": 331534419,
    "name": "sha256:b19cca6a3c790aa1a7df132112a778a41fff887fcdd08639276c2955f4b0e123",
    "url": "https://api.github.com/users/Skarlso/packages/container/helm%2Fcrd-bootstrap/versions/331534419",
    "package_html_url": "https://github.com/users/Skarlso/packages/container/package/helm%2Fcrd-bootstrap",
    "created_at": "2025-01-06T06:12:17Z",
    "updated_at": "2025-01-06T06:12:17Z",
    "description": "This is the official Helm chart for the Project crd-bootstrap.",
    "html_url": "https://github.com/users/Skarlso/packages/container/helm%2Fcrd-bootstrap/331534419",
    "metadata": {
      "package_type": "container",
      "container": {
        "tags": [
          "v0.9.9"
        ]
      }
    }
  },
  ...
]


# FAILURE
$ gh api /users/Skarlso/packages/container/helm/crd-bootstrap/versions/331534419
{
  "message": "Not Found",
  "documentation_url": "https://docs.github.com/rest",
  "status": "404"
}
gh: Not Found (HTTP 404)

# SUCCESS
$ bin/gh api /users/Skarlso/packages/container/helm/crd-bootstrap/versions/331534419
{
  "id": 331534419,
  "name": "sha256:b19cca6a3c790aa1a7df132112a778a41fff887fcdd08639276c2955f4b0e123",
  "url": "https://api.github.com/users/Skarlso/packages/container/helm%2Fcrd-bootstrap/versions/331534419",
  "package_html_url": "https://github.com/users/Skarlso/packages/container/package/helm%2Fcrd-bootstrap",
  "created_at": "2025-01-06T06:12:17Z",
  "updated_at": "2025-01-06T06:12:17Z",
  "description": "This is the official Helm chart for the Project crd-bootstrap.",
  "html_url": "https://github.com/users/Skarlso/packages/container/helm%2Fcrd-bootstrap/331534419",
  "metadata": {
    "package_type": "container",
    "container": {
      "tags": [
        "v0.9.9"
      ]
    }
  }
}

jtmcg · 2025-02-21T18:01:19Z

Somehow I missed this PR. Sorry @iamazeem. Either way, I think we're calling a pause on gh api changes for at least a week (but probably longer). The team is meeting to discuss next week, so I'll update you then. To set expectations, I find it most likely that we close this and come back to it after we invest some time into this command as a team.

Also, I'm curious as to why this isn't just URL encoding everything. Admittedly, I haven't dug deeply into this yet, but I would have just expected we could get away with universally encoding all paths we're given in this command.

iamazeem · 2025-02-21T18:07:49Z

Somehow I missed this PR. Sorry @iamazeem. Either way, I think we're calling a pause on gh api changes for at least a week (but probably longer). The team is meeting to discuss next week, so I'll update you then. To set expectations, I find it most likely that we close this and come back to it after we invest some time into this command as a team.

No worries. Take your time. 👍

Also, I'm curious as to why this isn't just URL encoding everything. Admittedly, I haven't dug deeply into this yet, but I would have just expected we could get away with universally encoding all paths we're given in this command.

That doesn't work.
Already tested and verified that.
GitHub API has other similar instances where the slashes need to be URL encoded.
I saw one recently but can't remember which one was it. I'll share it later when I find it.

iamazeem · 2025-02-24T11:32:36Z

@jtmcg: Here are other such instances where url.QueryEscape is being used in a similar scenario:
https://github.com/search?q=repo%3Acli%2Fcli%20%22url.QueryEscape%22&type=code

jtmcg

Looks good. A few minor comments. I'll test this locally and give this comment a 👍 if everything checks out

pkg/cmd/api/api.go

jtmcg

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [cli/cli](https://github.com/cli/cli) | minor | `v2.67.0` -> `v2.68.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>cli/cli (cli/cli)</summary> ### [`v2.68.0`](https://github.com/cli/cli/releases/tag/v2.68.0): GitHub CLI 2.68.0 [Compare Source](cli/cli@v2.67.0...v2.68.0) #### What's Changed ##### ✨ Features - \[gh repo view] Improve error message for forked repo by [@iamazeem](https://github.com/iamazeem) in cli/cli#10334 - Add signer-digest, source-ref, and source-digest options for `gh attestation verify` by [@malancas](https://github.com/malancas) in cli/cli#10308 - \[gh pr checkout] Add --no-tags option to git fetch commands in checkout by [@latzskim](https://github.com/latzskim) in cli/cli#10479 - \[`gh issue/pr comment`] Add `--create-if-none` and prompts to create a comment if no comment already exists by [@latzskim](https://github.com/latzskim) in cli/cli#10427 - \[gh cache delete --all] Add `--succeed-on-no-caches` flag to return exit code 0 by [@iamazeem](https://github.com/iamazeem) in cli/cli#10327 - \[gh release create] Fail when there are no new commits since the last release by [@iamazeem](https://github.com/iamazeem) in cli/cli#10398 - update default upstream when forking repo during MR creation by [@daviddl9](https://github.com/daviddl9) in cli/cli#10458 ##### 🐛 Fixes - Refactor `GetLocalAttestations` and clean up custom registry transport by [@malancas](https://github.com/malancas) in cli/cli#10382 - Check `GH_REPO` too in addition to `--repo` for disambiguation by [@williammartin](https://github.com/williammartin) in cli/cli#10539 - (Fixes `gh secret` subcommands not working outside of a repository) - Fix unhandled panic in FindWorkflow and add tests by [@jtmcg](https://github.com/jtmcg) in cli/cli#10521 - Fix checkout when URL arg is from fork and cwd is upstream by [@williammartin](https://github.com/williammartin) in cli/cli#10512 - \[gh api] Escape package name (URL encoding) for packages endpoint by [@iamazeem](https://github.com/iamazeem) in cli/cli#10384 - Fix `remoteResolver` caching issue by [@iamazeem](https://github.com/iamazeem) in cli/cli#10456 - Fix gh project item-edit to allow --number 0 as a valid value by [@aryanbhosale](https://github.com/aryanbhosale) in cli/cli#10417 - Add mutex to fix race in attestation test client by [@codysoyland](https://github.com/codysoyland) in cli/cli#10439 - Base64 decode GPG passphrase in deployment workflow by [@BagToad](https://github.com/BagToad) in cli/cli#10546 ##### 📚 Docs & Chores - Deep Dive Document Release Process by [@williammartin](https://github.com/williammartin) in cli/cli#10503 - Inconsistent format of examples in help text by [@iamazeem](https://github.com/iamazeem) in cli/cli#10508 - Inconsistent format of description of flags (starting with lowercase letter) by [@iamazeem](https://github.com/iamazeem) in cli/cli#10507 - Update Go version to 1.23 in CONTRIBUTING.md by [@williammartin](https://github.com/williammartin) in cli/cli#10504 - Fix minor auth login help typo by [@williammartin](https://github.com/williammartin) in cli/cli#10501 - docs: document how to revoke `gh` OAuth tokens in `auth logout`'s help by [@BagToad](https://github.com/BagToad) in cli/cli#10490 - chore: update codespaces Go version by [@BagToad](https://github.com/BagToad) in cli/cli#10491 - Allow injection of TUFMetadataDir in tests by [@williammartin](https://github.com/williammartin) in cli/cli#10478 - refactor: use a more straightforward return value by [@beforetech](https://github.com/beforetech) in cli/cli#10489 - Use subtests in attestation verification integration tests by [@williammartin](https://github.com/williammartin) in cli/cli#10463 - Fix typo in README by [@iamazeem](https://github.com/iamazeem) in cli/cli#10445 - Update usage to lower-kebab-case by [@iamazeem](https://github.com/iamazeem) in cli/cli#10447 - Standardize URLs by [@iamazeem](https://github.com/iamazeem) in cli/cli#10429 - Remove trailing whitespace by [@iamazeem](https://github.com/iamazeem) in cli/cli#10430 #####

Dependencies - Bump actions/attest-build-provenance from 2.2.0 to 2.2.2 by [@dependabot](https://github.com/dependabot) in cli/cli#10518 - Bump github.com/go-jose/go-jose/v4 from 4.0.2 to 4.0.5 by [@dependabot](https://github.com/dependabot) in cli/cli#10499 - Bump github.com/spf13/pflag from 1.0.5 to 1.0.6 by [@dependabot](https://github.com/dependabot) in cli/cli#10338 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

…ckage-name" This reverts commit ed2c322, reversing changes made to f019cf7.

Revert "Merge pull request #10384 from iamazeem/9798-gh-api-encode-package-name"

williammartin · 2025-03-18T12:28:28Z

I've reverted this PR: #10630

…-package-name" This reverts commit ed2c322, reversing changes made to f019cf7.

[gh api] Escape package name (URL encoding) for packages endpoint

4107944

iamazeem requested a review from a team as a code owner February 7, 2025 08:29

iamazeem requested a review from jtmcg February 7, 2025 08:29

cliAutomation added the external pull request originating outside of the CLI core team label Feb 7, 2025

Erick643-sudo mentioned this pull request Feb 20, 2025

Erick Reyes González #10470

Closed

jtmcg reviewed Feb 24, 2025

View reviewed changes

pkg/cmd/api/api.go Show resolved Hide resolved

pkg/cmd/api/api.go Outdated Show resolved Hide resolved

Add docs; rename function name

9c87069

iamazeem requested a review from jtmcg February 25, 2025 05:38

jtmcg reviewed Feb 25, 2025

View reviewed changes

pkg/cmd/api/api.go Show resolved Hide resolved

jtmcg approved these changes Feb 25, 2025

View reviewed changes

jtmcg merged commit ed2c322 into cli:trunk Feb 25, 2025
9 checks passed

iamazeem deleted the 9798-gh-api-encode-package-name branch February 26, 2025 04:57

drewzemke-rss mentioned this pull request Mar 12, 2025

gh api URL encoding change not backwards compatible #10592

Closed

digimangos mentioned this pull request Mar 18, 2025

gh api URL encoding change is incomplete #10628

Closed

williammartin added a commit that referenced this pull request Mar 18, 2025

Revert "Merge pull request #10384 from iamazeem/9798-gh-api-encode-pa…

f4382d1

…ckage-name" This reverts commit ed2c322, reversing changes made to f019cf7.

williammartin mentioned this pull request Mar 18, 2025

Revert "Merge pull request #10384 from iamazeem/9798-gh-api-encode-package-name" #10630

Merged

williammartin added a commit that referenced this pull request Mar 18, 2025

Merge pull request #10630 from cli/wm/revert-api-encoding

8f0a465

Revert "Merge pull request #10384 from iamazeem/9798-gh-api-encode-package-name"

rancorm pushed a commit to rancorm/cli that referenced this pull request May 3, 2025

Revert "Merge pull request cli#10384 from iamazeem/9798-gh-api-encode…

f0605a5

…-package-name" This reverts commit ed2c322, reversing changes made to f019cf7.

jeroen mentioned this pull request Jul 24, 2025

htt2 >= 1.2.0 replaces '%2f' with '/' in the URL r-lib/httr2#786

Closed

This was referenced Mar 26, 2026

feat: add /risk and /reviewers commands, closes #65 warestack/watchflow#67

Open

feat(lsp): complete inlay hint resolver with label location details (#2097) EffortlessMetrics/perl-lsp#2985

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[gh api] Escape package name (URL encoding) for packages endpoint#10384

[gh api] Escape package name (URL encoding) for packages endpoint#10384
jtmcg merged 2 commits intocli:trunkfrom
iamazeem:9798-gh-api-encode-package-name

iamazeem commented Feb 7, 2025 •

edited

Loading

Uh oh!

jtmcg commented Feb 21, 2025

Uh oh!

iamazeem commented Feb 21, 2025

Uh oh!

iamazeem commented Feb 24, 2025

Uh oh!

jtmcg left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jtmcg left a comment

Uh oh!

Uh oh!

williammartin commented Mar 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

iamazeem commented Feb 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tests

Uh oh!

jtmcg commented Feb 21, 2025

Uh oh!

iamazeem commented Feb 21, 2025

Uh oh!

iamazeem commented Feb 24, 2025

Uh oh!

jtmcg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jtmcg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

williammartin commented Mar 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

iamazeem commented Feb 7, 2025 •

edited

Loading