Skip to content

Commit

Permalink
Escape package names to support names which include a slash (#3002)
Browse files Browse the repository at this point in the history
  • Loading branch information
@bn4t
bn4t committed Dec 16, 2023
1 parent 37f1826 commit ee55955
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 45 deletions.
29 changes: 22 additions & 7 deletions github/orgs_packages.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package github
import (
"context"
"fmt"
"net/url"
)

// ListPackages lists the packages for an organization.
Expand Down Expand Up @@ -38,11 +39,13 @@ func (s *OrganizationsService) ListPackages(ctx context.Context, org string, opt

// GetPackage gets a package by name from an organization.
//
// 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#get-a-package-for-an-organization
//
//meta:operation GET /orgs/{org}/packages/{package_type}/{package_name}
func (s *OrganizationsService) GetPackage(ctx context.Context, org, packageType, packageName string) (*Package, *Response, error) {
u := fmt.Sprintf("orgs/%v/packages/%v/%v", org, packageType, packageName)
u := fmt.Sprintf("orgs/%v/packages/%v/%v", org, packageType, url.PathEscape(packageName))
req, err := s.client.NewRequest("GET", u, nil)
if err != nil {
return nil, nil, err
Expand All @@ -59,11 +62,13 @@ func (s *OrganizationsService) GetPackage(ctx context.Context, org, packageType,

// DeletePackage deletes a package from an organization.
//
// 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#delete-a-package-for-an-organization
//
//meta:operation DELETE /orgs/{org}/packages/{package_type}/{package_name}
func (s *OrganizationsService) DeletePackage(ctx context.Context, org, packageType, packageName string) (*Response, error) {
u := fmt.Sprintf("orgs/%v/packages/%v/%v", org, packageType, packageName)
u := fmt.Sprintf("orgs/%v/packages/%v/%v", org, packageType, url.PathEscape(packageName))
req, err := s.client.NewRequest("DELETE", u, nil)
if err != nil {
return nil, err
Expand All @@ -74,11 +79,13 @@ func (s *OrganizationsService) DeletePackage(ctx context.Context, org, packageTy

// RestorePackage restores a package to an organization.
//
// 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#restore-a-package-for-an-organization
//
//meta:operation POST /orgs/{org}/packages/{package_type}/{package_name}/restore
func (s *OrganizationsService) RestorePackage(ctx context.Context, org, packageType, packageName string) (*Response, error) {
u := fmt.Sprintf("orgs/%v/packages/%v/%v/restore", org, packageType, packageName)
u := fmt.Sprintf("orgs/%v/packages/%v/%v/restore", org, packageType, url.PathEscape(packageName))
req, err := s.client.NewRequest("POST", u, nil)
if err != nil {
return nil, err
Expand All @@ -89,11 +96,13 @@ func (s *OrganizationsService) RestorePackage(ctx context.Context, org, packageT

// PackageGetAllVersions gets all versions of a package in an organization.
//
// 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#list-package-versions-for-a-package-owned-by-an-organization
//
//meta:operation GET /orgs/{org}/packages/{package_type}/{package_name}/versions
func (s *OrganizationsService) PackageGetAllVersions(ctx context.Context, org, packageType, packageName string, opts *PackageListOptions) ([]*PackageVersion, *Response, error) {
u := fmt.Sprintf("orgs/%v/packages/%v/%v/versions", org, packageType, packageName)
u := fmt.Sprintf("orgs/%v/packages/%v/%v/versions", org, packageType, url.PathEscape(packageName))
u, err := addOptions(u, opts)
if err != nil {
return nil, nil, err
Expand All @@ -115,11 +124,13 @@ func (s *OrganizationsService) PackageGetAllVersions(ctx context.Context, org, p

// PackageGetVersion gets a specific version of a package in an organization.
//
// 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#get-a-package-version-for-an-organization
//
//meta:operation GET /orgs/{org}/packages/{package_type}/{package_name}/versions/{package_version_id}
func (s *OrganizationsService) PackageGetVersion(ctx context.Context, org, packageType, packageName string, packageVersionID int64) (*PackageVersion, *Response, error) {
u := fmt.Sprintf("orgs/%v/packages/%v/%v/versions/%v", org, packageType, packageName, packageVersionID)
u := fmt.Sprintf("orgs/%v/packages/%v/%v/versions/%v", org, packageType, url.PathEscape(packageName), packageVersionID)
req, err := s.client.NewRequest("GET", u, nil)
if err != nil {
return nil, nil, err
Expand All @@ -136,11 +147,13 @@ func (s *OrganizationsService) PackageGetVersion(ctx context.Context, org, packa

// PackageDeleteVersion deletes a package version from an organization.
//
// 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#delete-package-version-for-an-organization
//
//meta:operation DELETE /orgs/{org}/packages/{package_type}/{package_name}/versions/{package_version_id}
func (s *OrganizationsService) PackageDeleteVersion(ctx context.Context, org, packageType, packageName string, packageVersionID int64) (*Response, error) {
u := fmt.Sprintf("orgs/%v/packages/%v/%v/versions/%v", org, packageType, packageName, packageVersionID)
u := fmt.Sprintf("orgs/%v/packages/%v/%v/versions/%v", org, packageType, url.PathEscape(packageName), packageVersionID)
req, err := s.client.NewRequest("DELETE", u, nil)
if err != nil {
return nil, err
Expand All @@ -151,11 +164,13 @@ func (s *OrganizationsService) PackageDeleteVersion(ctx context.Context, org, pa

// PackageRestoreVersion restores a package version to an organization.
//
// 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#restore-package-version-for-an-organization
//
//meta:operation POST /orgs/{org}/packages/{package_type}/{package_name}/versions/{package_version_id}/restore
func (s *OrganizationsService) PackageRestoreVersion(ctx context.Context, org, packageType, packageName string, packageVersionID int64) (*Response, error) {
u := fmt.Sprintf("orgs/%v/packages/%v/%v/versions/%v/restore", org, packageType, packageName, packageVersionID)
u := fmt.Sprintf("orgs/%v/packages/%v/%v/versions/%v/restore", org, packageType, url.PathEscape(packageName), packageVersionID)
req, err := s.client.NewRequest("POST", u, nil)
if err != nil {
return nil, err
Expand Down
95 changes: 57 additions & 38 deletions github/orgs_packages_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ package github

import (
"context"
"fmt"
"io"
"net/http"
"testing"

Expand All @@ -20,7 +20,7 @@ func TestOrganizationsService_ListPackages(t *testing.T) {

mux.HandleFunc("/orgs/o/packages", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")
fmt.Fprint(w, `[{
_, err := io.WriteString(w, `[{
"id": 197,
"name": "hello_docker",
"package_type": "container",
Expand Down Expand Up @@ -52,6 +52,9 @@ func TestOrganizationsService_ListPackages(t *testing.T) {
"html_url": "https://github.com/orgs/github/packages/container/package/hello_docker"
}
]`)
if err != nil {
t.Fatal("Failed to write test response: ", err)
}
})

ctx := context.Background()
Expand Down Expand Up @@ -114,35 +117,39 @@ func TestOrganizationsService_GetPackage(t *testing.T) {
client, mux, _, teardown := setup()
defer teardown()

mux.HandleFunc("/orgs/o/packages/container/hello_docker", func(w http.ResponseWriter, r *http.Request) {
// don't url escape the package name here since mux will convert it to a slash automatically
mux.HandleFunc("/orgs/o/packages/container/hello/hello_docker", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")
fmt.Fprint(w, `{
_, err := io.WriteString(w, `{
"id": 197,
"name": "hello_docker",
"name": "hello/hello_docker",
"package_type": "container",
"version_count": 1,
"visibility": "private",
"url": "https://api.github.com/orgs/github/packages/container/hello_docker",
"url": "https://api.github.com/orgs/github/packages/container/hello%2Fhello_docker",
"created_at": `+referenceTimeStr+`,
"updated_at": `+referenceTimeStr+`,
"html_url": "https://github.com/orgs/github/packages/container/package/hello_docker"
"html_url": "https://github.com/orgs/github/packages/container/package/hello%2Fhello_docker"
}`)
if err != nil {
t.Fatal("Failed to write test response: ", err)
}
})

ctx := context.Background()
packages, _, err := client.Organizations.GetPackage(ctx, "o", "container", "hello_docker")
packages, _, err := client.Organizations.GetPackage(ctx, "o", "container", "hello/hello_docker")
if err != nil {
t.Errorf("Organizations.GetPackage returned error: %v", err)
}

want := &Package{
ID: Int64(197),
Name: String("hello_docker"),
Name: String("hello/hello_docker"),
PackageType: String("container"),
VersionCount: Int64(1),
Visibility: String("private"),
URL: String("https://api.github.com/orgs/github/packages/container/hello_docker"),
HTMLURL: String("https://github.com/orgs/github/packages/container/package/hello_docker"),
URL: String("https://api.github.com/orgs/github/packages/container/hello%2Fhello_docker"),
HTMLURL: String("https://github.com/orgs/github/packages/container/package/hello%2Fhello_docker"),
CreatedAt: &Timestamp{referenceTime},
UpdatedAt: &Timestamp{referenceTime},
}
Expand All @@ -169,12 +176,13 @@ func TestOrganizationsService_DeletePackage(t *testing.T) {
client, mux, _, teardown := setup()
defer teardown()

mux.HandleFunc("/orgs/o/packages/container/hello_docker", func(w http.ResponseWriter, r *http.Request) {
// don't url escape the package name here since mux will convert it to a slash automatically
mux.HandleFunc("/orgs/o/packages/container/hello/hello_docker", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "DELETE")
})

ctx := context.Background()
_, err := client.Organizations.DeletePackage(ctx, "o", "container", "hello_docker")
_, err := client.Organizations.DeletePackage(ctx, "o", "container", "hello/hello_docker")
if err != nil {
t.Errorf("Organizations.DeletePackage returned error: %v", err)
}
Expand All @@ -198,12 +206,13 @@ func TestOrganizationsService_RestorePackage(t *testing.T) {
client, mux, _, teardown := setup()
defer teardown()

mux.HandleFunc("/orgs/o/packages/container/hello_docker/restore", func(w http.ResponseWriter, r *http.Request) {
// don't url escape the package name here since mux will convert it to a slash automatically
mux.HandleFunc("/orgs/o/packages/container/hello/hello_docker/restore", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "POST")
})

ctx := context.Background()
_, err := client.Organizations.RestorePackage(ctx, "o", "container", "hello_docker")
_, err := client.Organizations.RestorePackage(ctx, "o", "container", "hello/hello_docker")
if err != nil {
t.Errorf("Organizations.RestorePackage returned error: %v", err)
}
Expand All @@ -215,26 +224,27 @@ func TestOrganizationsService_RestorePackage(t *testing.T) {
})

testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) {
return client.Organizations.RestorePackage(ctx, "", "container", "hello_docker")
return client.Organizations.RestorePackage(ctx, "", "container", "hello/hello_docker")
})
}

func TestOrganizationsService_ListPackagesVersions(t *testing.T) {
client, mux, _, teardown := setup()
defer teardown()

mux.HandleFunc("/orgs/o/packages/container/hello_docker/versions", func(w http.ResponseWriter, r *http.Request) {
// don't url escape the package name here since mux will convert it to a slash automatically
mux.HandleFunc("/orgs/o/packages/container/hello/hello_docker/versions", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")
testFormValues(t, r, values{"per_page": "2", "page": "1", "state": "deleted", "visibility": "internal", "package_type": "container"})
fmt.Fprint(w, `[
_, err := io.WriteString(w, `[
{
"id": 45763,
"name": "sha256:08a44bab0bddaddd8837a8b381aebc2e4b933768b981685a9e088360af0d3dd9",
"url": "https://api.github.com/users/octocat/packages/container/hello_docker/versions/45763",
"package_html_url": "https://github.com/users/octocat/packages/container/package/hello_docker",
"url": "https://api.github.com/users/octocat/packages/container/hello%2Fhello_docker/versions/45763",
"package_html_url": "https://github.com/users/octocat/packages/container/package/hello%2Fhello_docker",
"created_at": `+referenceTimeStr+`,
"updated_at": `+referenceTimeStr+`,
"html_url": "https://github.com/users/octocat/packages/container/hello_docker/45763",
"html_url": "https://github.com/users/octocat/packages/container/hello%2Fhello_docker/45763",
"metadata": {
"package_type": "container",
"container": {
Expand All @@ -244,25 +254,28 @@ func TestOrganizationsService_ListPackagesVersions(t *testing.T) {
}
}
}]`)
if err != nil {
t.Fatal("Failed to write test response: ", err)
}
})

ctx := context.Background()
opts := &PackageListOptions{
String("internal"), String("container"), String("deleted"), ListOptions{Page: 1, PerPage: 2},
}
packages, _, err := client.Organizations.PackageGetAllVersions(ctx, "o", "container", "hello_docker", opts)
packages, _, err := client.Organizations.PackageGetAllVersions(ctx, "o", "container", "hello/hello_docker", opts)
if err != nil {
t.Errorf("Organizations.PackageGetAllVersions returned error: %v", err)
}

want := []*PackageVersion{{
ID: Int64(45763),
Name: String("sha256:08a44bab0bddaddd8837a8b381aebc2e4b933768b981685a9e088360af0d3dd9"),
URL: String("https://api.github.com/users/octocat/packages/container/hello_docker/versions/45763"),
PackageHTMLURL: String("https://github.com/users/octocat/packages/container/package/hello_docker"),
URL: String("https://api.github.com/users/octocat/packages/container/hello%2Fhello_docker/versions/45763"),
PackageHTMLURL: String("https://github.com/users/octocat/packages/container/package/hello%2Fhello_docker"),
CreatedAt: &Timestamp{referenceTime},
UpdatedAt: &Timestamp{referenceTime},
HTMLURL: String("https://github.com/users/octocat/packages/container/hello_docker/45763"),
HTMLURL: String("https://github.com/users/octocat/packages/container/hello%2Fhello_docker/45763"),
Metadata: &PackageMetadata{
PackageType: String("container"),
Container: &PackageContainerMetadata{
Expand Down Expand Up @@ -293,17 +306,18 @@ func TestOrganizationsService_PackageGetVersion(t *testing.T) {
client, mux, _, teardown := setup()
defer teardown()

mux.HandleFunc("/orgs/o/packages/container/hello_docker/versions/45763", func(w http.ResponseWriter, r *http.Request) {
// don't url escape the package name here since mux will convert it to a slash automatically
mux.HandleFunc("/orgs/o/packages/container/hello/hello_docker/versions/45763", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")
fmt.Fprint(w, `
_, err := io.WriteString(w, `
{
"id": 45763,
"name": "sha256:08a44bab0bddaddd8837a8b381aebc2e4b933768b981685a9e088360af0d3dd9",
"url": "https://api.github.com/users/octocat/packages/container/hello_docker/versions/45763",
"package_html_url": "https://github.com/users/octocat/packages/container/package/hello_docker",
"url": "https://api.github.com/users/octocat/packages/container/hello%2Fhello_docker/versions/45763",
"package_html_url": "https://github.com/users/octocat/packages/container/package/hello%2Fhello_docker",
"created_at": `+referenceTimeStr+`,
"updated_at": `+referenceTimeStr+`,
"html_url": "https://github.com/users/octocat/packages/container/hello_docker/45763",
"html_url": "https://github.com/users/octocat/packages/container/hello%2Fhello_docker/45763",
"metadata": {
"package_type": "container",
"container": {
Expand All @@ -313,22 +327,25 @@ func TestOrganizationsService_PackageGetVersion(t *testing.T) {
}
}
}`)
if err != nil {
t.Fatal("Failed to write test response: ", err)
}
})

ctx := context.Background()
packages, _, err := client.Organizations.PackageGetVersion(ctx, "o", "container", "hello_docker", 45763)
packages, _, err := client.Organizations.PackageGetVersion(ctx, "o", "container", "hello/hello_docker", 45763)
if err != nil {
t.Errorf("Organizations.PackageGetVersion returned error: %v", err)
}

want := &PackageVersion{
ID: Int64(45763),
Name: String("sha256:08a44bab0bddaddd8837a8b381aebc2e4b933768b981685a9e088360af0d3dd9"),
URL: String("https://api.github.com/users/octocat/packages/container/hello_docker/versions/45763"),
PackageHTMLURL: String("https://github.com/users/octocat/packages/container/package/hello_docker"),
URL: String("https://api.github.com/users/octocat/packages/container/hello%2Fhello_docker/versions/45763"),
PackageHTMLURL: String("https://github.com/users/octocat/packages/container/package/hello%2Fhello_docker"),
CreatedAt: &Timestamp{referenceTime},
UpdatedAt: &Timestamp{referenceTime},
HTMLURL: String("https://github.com/users/octocat/packages/container/hello_docker/45763"),
HTMLURL: String("https://github.com/users/octocat/packages/container/hello%2Fhello_docker/45763"),
Metadata: &PackageMetadata{
PackageType: String("container"),
Container: &PackageContainerMetadata{
Expand Down Expand Up @@ -359,12 +376,13 @@ func TestOrganizationsService_PackageDeleteVersion(t *testing.T) {
client, mux, _, teardown := setup()
defer teardown()

mux.HandleFunc("/orgs/o/packages/container/hello_docker/versions/45763", func(w http.ResponseWriter, r *http.Request) {
// don't url escape the package name here since mux will convert it to a slash automatically
mux.HandleFunc("/orgs/o/packages/container/hello/hello_docker/versions/45763", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "DELETE")
})

ctx := context.Background()
_, err := client.Organizations.PackageDeleteVersion(ctx, "o", "container", "hello_docker", 45763)
_, err := client.Organizations.PackageDeleteVersion(ctx, "o", "container", "hello/hello_docker", 45763)
if err != nil {
t.Errorf("Organizations.PackageDeleteVersion returned error: %v", err)
}
Expand All @@ -384,12 +402,13 @@ func TestOrganizationsService_PackageRestoreVersion(t *testing.T) {
client, mux, _, teardown := setup()
defer teardown()

mux.HandleFunc("/orgs/o/packages/container/hello_docker/versions/45763/restore", func(w http.ResponseWriter, r *http.Request) {
// don't url escape the package name here since mux will convert it to a slash automatically
mux.HandleFunc("/orgs/o/packages/container/hello/hello_docker/versions/45763/restore", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "POST")
})

ctx := context.Background()
_, err := client.Organizations.PackageRestoreVersion(ctx, "o", "container", "hello_docker", 45763)
_, err := client.Organizations.PackageRestoreVersion(ctx, "o", "container", "hello/hello_docker", 45763)
if err != nil {
t.Errorf("Organizations.PackageRestoreVersion returned error: %v", err)
}
Expand Down

0 comments on commit ee55955

Please sign in to comment.