From f02b3f20a67d31d0f06d988e6ae145331e8e2df9 Mon Sep 17 00:00:00 2001 From: pucedoteth <119044801+pucedoteth@users.noreply.github.com> Date: Fri, 11 Sep 2026 14:53:10 +0200 Subject: [PATCH] fix: Escape package names in UsersService methods GetPackage escaped packageName but the other seven UsersService package methods interpolated it raw, so a name containing a slash produced a malformed path. Container and scoped npm package names routinely contain one. OrganizationsService already escapes it in all seven equivalent methods, so this brings the user methods in line and adds the same doc note. Co-Authored-By: Claude Opus 5 --- github/users_packages.go | 40 +++++++--- github/users_packages_test.go | 133 ++++++++++++++++++++++++++++++++++ 2 files changed, 161 insertions(+), 12 deletions(-) diff --git a/github/users_packages.go b/github/users_packages.go index 8a1fad3b648..c6065b595f4 100644 --- a/github/users_packages.go +++ b/github/users_packages.go @@ -49,6 +49,8 @@ func (s *UsersService) ListPackages(ctx context.Context, user string, opts *Pack // GetPackage gets a package by name for a user. Passing the empty string for "user" will // get the package for the authenticated user. // +// Note that packageName is escaped for the URL path so that you don't need to. +// // GitHub API docs: https://docs.github.com/rest/packages/packages?apiVersion=2022-11-28#get-a-package-for-a-user // // GitHub API docs: https://docs.github.com/rest/packages/packages?apiVersion=2022-11-28#get-a-package-for-the-authenticated-user @@ -80,6 +82,8 @@ func (s *UsersService) GetPackage(ctx context.Context, user, packageType, packag // DeletePackage deletes a package from a user. Passing the empty string for "user" will // delete the package for the authenticated user. // +// Note that packageName is escaped for the URL path so that you don't need to. +// // GitHub API docs: https://docs.github.com/rest/packages/packages?apiVersion=2022-11-28#delete-a-package-for-a-user // // GitHub API docs: https://docs.github.com/rest/packages/packages?apiVersion=2022-11-28#delete-a-package-for-the-authenticated-user @@ -89,9 +93,9 @@ func (s *UsersService) GetPackage(ctx context.Context, user, packageType, packag func (s *UsersService) DeletePackage(ctx context.Context, user, packageType, packageName string) (*Response, error) { var u string if user != "" { - u = fmt.Sprintf("users/%v/packages/%v/%v", user, packageType, packageName) + u = fmt.Sprintf("users/%v/packages/%v/%v", user, packageType, url.PathEscape(packageName)) } else { - u = fmt.Sprintf("user/packages/%v/%v", packageType, packageName) + u = fmt.Sprintf("user/packages/%v/%v", packageType, url.PathEscape(packageName)) } req, err := s.client.NewRequest(ctx, "DELETE", u, nil) @@ -105,6 +109,8 @@ func (s *UsersService) DeletePackage(ctx context.Context, user, packageType, pac // RestorePackage restores a package to a user. Passing the empty string for "user" will // restore the package for the authenticated user. // +// Note that packageName is escaped for the URL path so that you don't need to. +// // GitHub API docs: https://docs.github.com/rest/packages/packages?apiVersion=2022-11-28#restore-a-package-for-a-user // // GitHub API docs: https://docs.github.com/rest/packages/packages?apiVersion=2022-11-28#restore-a-package-for-the-authenticated-user @@ -114,9 +120,9 @@ func (s *UsersService) DeletePackage(ctx context.Context, user, packageType, pac func (s *UsersService) RestorePackage(ctx context.Context, user, packageType, packageName string) (*Response, error) { var u string if user != "" { - u = fmt.Sprintf("users/%v/packages/%v/%v/restore", user, packageType, packageName) + u = fmt.Sprintf("users/%v/packages/%v/%v/restore", user, packageType, url.PathEscape(packageName)) } else { - u = fmt.Sprintf("user/packages/%v/%v/restore", packageType, packageName) + u = fmt.Sprintf("user/packages/%v/%v/restore", packageType, url.PathEscape(packageName)) } req, err := s.client.NewRequest(ctx, "POST", u, nil) @@ -137,11 +143,13 @@ type ListPackageVersionsOptions struct { // ListPackageVersions gets all versions of a package for the authenticated user. // +// Note that packageName is escaped for the URL path so that you don't need to. +// // GitHub API docs: https://docs.github.com/rest/packages/packages?apiVersion=2022-11-28#list-package-versions-for-a-package-owned-by-the-authenticated-user // //meta:operation GET /user/packages/{package_type}/{package_name}/versions func (s *UsersService) ListPackageVersions(ctx context.Context, packageType, packageName string, opts *ListPackageVersionsOptions) ([]*PackageVersion, *Response, error) { - u := fmt.Sprintf("user/packages/%v/%v/versions", packageType, packageName) + u := fmt.Sprintf("user/packages/%v/%v/versions", packageType, url.PathEscape(packageName)) u, err := addOptions(u, opts) if err != nil { return nil, nil, err @@ -163,11 +171,13 @@ func (s *UsersService) ListPackageVersions(ctx context.Context, packageType, pac // ListUserPackageVersions returns package versions for a public package owned by a specified user. // +// Note that packageName is escaped for the URL path so that you don't need to. +// // GitHub API docs: https://docs.github.com/rest/packages/packages?apiVersion=2022-11-28#list-package-versions-for-a-package-owned-by-a-user // //meta:operation GET /users/{username}/packages/{package_type}/{package_name}/versions func (s *UsersService) ListUserPackageVersions(ctx context.Context, user, packageType, packageName string) ([]*PackageVersion, *Response, error) { - u := fmt.Sprintf("users/%v/packages/%v/%v/versions", user, packageType, packageName) + u := fmt.Sprintf("users/%v/packages/%v/%v/versions", user, packageType, url.PathEscape(packageName)) req, err := s.client.NewRequest(ctx, "GET", u, nil) if err != nil { @@ -186,6 +196,8 @@ func (s *UsersService) ListUserPackageVersions(ctx context.Context, user, packag // PackageGetVersion gets a specific version of a package for a user. Passing the empty string for "user" will // get the version for the authenticated user. // +// Note that packageName is escaped for the URL path so that you don't need to. +// // GitHub API docs: https://docs.github.com/rest/packages/packages?apiVersion=2022-11-28#get-a-package-version-for-a-user // // GitHub API docs: https://docs.github.com/rest/packages/packages?apiVersion=2022-11-28#get-a-package-version-for-the-authenticated-user @@ -195,9 +207,9 @@ func (s *UsersService) ListUserPackageVersions(ctx context.Context, user, packag func (s *UsersService) PackageGetVersion(ctx context.Context, user, packageType, packageName string, packageVersionID int64) (*PackageVersion, *Response, error) { var u string if user != "" { - u = fmt.Sprintf("users/%v/packages/%v/%v/versions/%v", user, packageType, packageName, packageVersionID) + u = fmt.Sprintf("users/%v/packages/%v/%v/versions/%v", user, packageType, url.PathEscape(packageName), packageVersionID) } else { - u = fmt.Sprintf("user/packages/%v/%v/versions/%v", packageType, packageName, packageVersionID) + u = fmt.Sprintf("user/packages/%v/%v/versions/%v", packageType, url.PathEscape(packageName), packageVersionID) } req, err := s.client.NewRequest(ctx, "GET", u, nil) @@ -217,6 +229,8 @@ func (s *UsersService) PackageGetVersion(ctx context.Context, user, packageType, // PackageDeleteVersion deletes a package version for a user. Passing the empty string for "user" will // delete the version for the authenticated user. // +// Note that packageName is escaped for the URL path so that you don't need to. +// // GitHub API docs: https://docs.github.com/rest/packages/packages?apiVersion=2022-11-28#delete-a-package-version-for-the-authenticated-user // // GitHub API docs: https://docs.github.com/rest/packages/packages?apiVersion=2022-11-28#delete-package-version-for-a-user @@ -226,9 +240,9 @@ func (s *UsersService) PackageGetVersion(ctx context.Context, user, packageType, func (s *UsersService) PackageDeleteVersion(ctx context.Context, user, packageType, packageName string, packageVersionID int64) (*Response, error) { var u string if user != "" { - u = fmt.Sprintf("users/%v/packages/%v/%v/versions/%v", user, packageType, packageName, packageVersionID) + u = fmt.Sprintf("users/%v/packages/%v/%v/versions/%v", user, packageType, url.PathEscape(packageName), packageVersionID) } else { - u = fmt.Sprintf("user/packages/%v/%v/versions/%v", packageType, packageName, packageVersionID) + u = fmt.Sprintf("user/packages/%v/%v/versions/%v", packageType, url.PathEscape(packageName), packageVersionID) } req, err := s.client.NewRequest(ctx, "DELETE", u, nil) @@ -242,6 +256,8 @@ func (s *UsersService) PackageDeleteVersion(ctx context.Context, user, packageTy // PackageRestoreVersion restores a package version to a user. Passing the empty string for "user" will // restore the version for the authenticated user. // +// Note that packageName is escaped for the URL path so that you don't need to. +// // GitHub API docs: https://docs.github.com/rest/packages/packages?apiVersion=2022-11-28#restore-a-package-version-for-the-authenticated-user // // GitHub API docs: https://docs.github.com/rest/packages/packages?apiVersion=2022-11-28#restore-package-version-for-a-user @@ -251,9 +267,9 @@ func (s *UsersService) PackageDeleteVersion(ctx context.Context, user, packageTy func (s *UsersService) PackageRestoreVersion(ctx context.Context, user, packageType, packageName string, packageVersionID int64) (*Response, error) { var u string if user != "" { - u = fmt.Sprintf("users/%v/packages/%v/%v/versions/%v/restore", user, packageType, packageName, packageVersionID) + u = fmt.Sprintf("users/%v/packages/%v/%v/versions/%v/restore", user, packageType, url.PathEscape(packageName), packageVersionID) } else { - u = fmt.Sprintf("user/packages/%v/%v/versions/%v/restore", packageType, packageName, packageVersionID) + u = fmt.Sprintf("user/packages/%v/%v/versions/%v/restore", packageType, url.PathEscape(packageName), packageVersionID) } req, err := s.client.NewRequest(ctx, "POST", u, nil) diff --git a/github/users_packages_test.go b/github/users_packages_test.go index 013a2485988..e4b3ca08fc4 100644 --- a/github/users_packages_test.go +++ b/github/users_packages_test.go @@ -6,6 +6,7 @@ package github import ( + "context" "encoding/json" "fmt" "io" @@ -693,3 +694,135 @@ func TestUsersService_specifiedUser_PackageRestoreVersion(t *testing.T) { return client.Users.PackageRestoreVersion(ctx, "", "", "", 45763) }) } + +func TestUsersService_packageName_isEscaped(t *testing.T) { + t.Parallel() + const packageName = "hello/hello_docker" + const escaped = "hello%2fhello_docker" + + for _, tt := range []struct { + name string + wantPath string + wantMethod string + body string + call func(ctx context.Context, client *Client) error + }{ + { + name: "DeletePackage", + wantPath: "/user/packages/container/" + escaped, + wantMethod: "DELETE", + call: func(ctx context.Context, client *Client) error { + _, err := client.Users.DeletePackage(ctx, "", "container", packageName) + return err + }, + }, + { + name: "DeletePackage_specifiedUser", + wantPath: "/users/u/packages/container/" + escaped, + wantMethod: "DELETE", + call: func(ctx context.Context, client *Client) error { + _, err := client.Users.DeletePackage(ctx, "u", "container", packageName) + return err + }, + }, + { + name: "RestorePackage", + wantPath: "/user/packages/container/" + escaped + "/restore", + wantMethod: "POST", + call: func(ctx context.Context, client *Client) error { + _, err := client.Users.RestorePackage(ctx, "", "container", packageName) + return err + }, + }, + { + name: "RestorePackage_specifiedUser", + wantPath: "/users/u/packages/container/" + escaped + "/restore", + wantMethod: "POST", + call: func(ctx context.Context, client *Client) error { + _, err := client.Users.RestorePackage(ctx, "u", "container", packageName) + return err + }, + }, + { + name: "ListPackageVersions", + wantPath: "/user/packages/container/" + escaped + "/versions", + wantMethod: "GET", + body: `[]`, + call: func(ctx context.Context, client *Client) error { + _, _, err := client.Users.ListPackageVersions(ctx, "container", packageName, nil) + return err + }, + }, + { + name: "ListUserPackageVersions", + wantPath: "/users/u/packages/container/" + escaped + "/versions", + wantMethod: "GET", + body: `[]`, + call: func(ctx context.Context, client *Client) error { + _, _, err := client.Users.ListUserPackageVersions(ctx, "u", "container", packageName) + return err + }, + }, + { + name: "PackageGetVersion", + wantPath: "/user/packages/container/" + escaped + "/versions/45763", + wantMethod: "GET", + body: `{}`, + call: func(ctx context.Context, client *Client) error { + _, _, err := client.Users.PackageGetVersion(ctx, "", "container", packageName, 45763) + return err + }, + }, + { + name: "PackageGetVersion_specifiedUser", + wantPath: "/users/u/packages/container/" + escaped + "/versions/45763", + wantMethod: "GET", + body: `{}`, + call: func(ctx context.Context, client *Client) error { + _, _, err := client.Users.PackageGetVersion(ctx, "u", "container", packageName, 45763) + return err + }, + }, + { + name: "PackageDeleteVersion", + wantPath: "/user/packages/container/" + escaped + "/versions/45763", + wantMethod: "DELETE", + call: func(ctx context.Context, client *Client) error { + _, err := client.Users.PackageDeleteVersion(ctx, "", "container", packageName, 45763) + return err + }, + }, + { + name: "PackageRestoreVersion", + wantPath: "/user/packages/container/" + escaped + "/versions/45763/restore", + wantMethod: "POST", + call: func(ctx context.Context, client *Client) error { + _, err := client.Users.PackageRestoreVersion(ctx, "", "container", packageName, 45763) + return err + }, + }, + } { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + client, mux, _ := setup(t) + + var called bool + mux.HandleFunc(tt.wantPath, func(w http.ResponseWriter, r *http.Request) { + called = true + testMethod(t, r, tt.wantMethod) + if tt.body != "" { + if _, err := io.WriteString(w, tt.body); err != nil { + t.Fatal("Failed to write test response: ", err) + } + } + }) + + if err := tt.call(t.Context(), client); err != nil { + t.Errorf("Users.%v returned error: %v", tt.name, err) + } + if !called { + t.Errorf("Users.%v did not request the escaped path %v", tt.name, tt.wantPath) + } + }) + } +}