Skip to content

Commit ee55955

Browse files
authored
Escape package names to support names which include a slash (#3002)
1 parent 37f1826 commit ee55955

File tree

2 files changed

+79
-45
lines changed

2 files changed

+79
-45
lines changed

github/orgs_packages.go

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ package github
88
import (
99
"context"
1010
"fmt"
11+
"net/url"
1112
)
1213

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

3940
// GetPackage gets a package by name from an organization.
4041
//
42+
// Note that packageName is escaped for the URL path so that you don't need to.
43+
//
4144
// GitHub API docs: https://docs.github.com/rest/packages/packages#get-a-package-for-an-organization
4245
//
4346
//meta:operation GET /orgs/{org}/packages/{package_type}/{package_name}
4447
func (s *OrganizationsService) GetPackage(ctx context.Context, org, packageType, packageName string) (*Package, *Response, error) {
45-
u := fmt.Sprintf("orgs/%v/packages/%v/%v", org, packageType, packageName)
48+
u := fmt.Sprintf("orgs/%v/packages/%v/%v", org, packageType, url.PathEscape(packageName))
4649
req, err := s.client.NewRequest("GET", u, nil)
4750
if err != nil {
4851
return nil, nil, err
@@ -59,11 +62,13 @@ func (s *OrganizationsService) GetPackage(ctx context.Context, org, packageType,
5962

6063
// DeletePackage deletes a package from an organization.
6164
//
65+
// Note that packageName is escaped for the URL path so that you don't need to.
66+
//
6267
// GitHub API docs: https://docs.github.com/rest/packages/packages#delete-a-package-for-an-organization
6368
//
6469
//meta:operation DELETE /orgs/{org}/packages/{package_type}/{package_name}
6570
func (s *OrganizationsService) DeletePackage(ctx context.Context, org, packageType, packageName string) (*Response, error) {
66-
u := fmt.Sprintf("orgs/%v/packages/%v/%v", org, packageType, packageName)
71+
u := fmt.Sprintf("orgs/%v/packages/%v/%v", org, packageType, url.PathEscape(packageName))
6772
req, err := s.client.NewRequest("DELETE", u, nil)
6873
if err != nil {
6974
return nil, err
@@ -74,11 +79,13 @@ func (s *OrganizationsService) DeletePackage(ctx context.Context, org, packageTy
7479

7580
// RestorePackage restores a package to an organization.
7681
//
82+
// Note that packageName is escaped for the URL path so that you don't need to.
83+
//
7784
// GitHub API docs: https://docs.github.com/rest/packages/packages#restore-a-package-for-an-organization
7885
//
7986
//meta:operation POST /orgs/{org}/packages/{package_type}/{package_name}/restore
8087
func (s *OrganizationsService) RestorePackage(ctx context.Context, org, packageType, packageName string) (*Response, error) {
81-
u := fmt.Sprintf("orgs/%v/packages/%v/%v/restore", org, packageType, packageName)
88+
u := fmt.Sprintf("orgs/%v/packages/%v/%v/restore", org, packageType, url.PathEscape(packageName))
8289
req, err := s.client.NewRequest("POST", u, nil)
8390
if err != nil {
8491
return nil, err
@@ -89,11 +96,13 @@ func (s *OrganizationsService) RestorePackage(ctx context.Context, org, packageT
8996

9097
// PackageGetAllVersions gets all versions of a package in an organization.
9198
//
99+
// Note that packageName is escaped for the URL path so that you don't need to.
100+
//
92101
// GitHub API docs: https://docs.github.com/rest/packages/packages#list-package-versions-for-a-package-owned-by-an-organization
93102
//
94103
//meta:operation GET /orgs/{org}/packages/{package_type}/{package_name}/versions
95104
func (s *OrganizationsService) PackageGetAllVersions(ctx context.Context, org, packageType, packageName string, opts *PackageListOptions) ([]*PackageVersion, *Response, error) {
96-
u := fmt.Sprintf("orgs/%v/packages/%v/%v/versions", org, packageType, packageName)
105+
u := fmt.Sprintf("orgs/%v/packages/%v/%v/versions", org, packageType, url.PathEscape(packageName))
97106
u, err := addOptions(u, opts)
98107
if err != nil {
99108
return nil, nil, err
@@ -115,11 +124,13 @@ func (s *OrganizationsService) PackageGetAllVersions(ctx context.Context, org, p
115124

116125
// PackageGetVersion gets a specific version of a package in an organization.
117126
//
127+
// Note that packageName is escaped for the URL path so that you don't need to.
128+
//
118129
// GitHub API docs: https://docs.github.com/rest/packages/packages#get-a-package-version-for-an-organization
119130
//
120131
//meta:operation GET /orgs/{org}/packages/{package_type}/{package_name}/versions/{package_version_id}
121132
func (s *OrganizationsService) PackageGetVersion(ctx context.Context, org, packageType, packageName string, packageVersionID int64) (*PackageVersion, *Response, error) {
122-
u := fmt.Sprintf("orgs/%v/packages/%v/%v/versions/%v", org, packageType, packageName, packageVersionID)
133+
u := fmt.Sprintf("orgs/%v/packages/%v/%v/versions/%v", org, packageType, url.PathEscape(packageName), packageVersionID)
123134
req, err := s.client.NewRequest("GET", u, nil)
124135
if err != nil {
125136
return nil, nil, err
@@ -136,11 +147,13 @@ func (s *OrganizationsService) PackageGetVersion(ctx context.Context, org, packa
136147

137148
// PackageDeleteVersion deletes a package version from an organization.
138149
//
150+
// Note that packageName is escaped for the URL path so that you don't need to.
151+
//
139152
// GitHub API docs: https://docs.github.com/rest/packages/packages#delete-package-version-for-an-organization
140153
//
141154
//meta:operation DELETE /orgs/{org}/packages/{package_type}/{package_name}/versions/{package_version_id}
142155
func (s *OrganizationsService) PackageDeleteVersion(ctx context.Context, org, packageType, packageName string, packageVersionID int64) (*Response, error) {
143-
u := fmt.Sprintf("orgs/%v/packages/%v/%v/versions/%v", org, packageType, packageName, packageVersionID)
156+
u := fmt.Sprintf("orgs/%v/packages/%v/%v/versions/%v", org, packageType, url.PathEscape(packageName), packageVersionID)
144157
req, err := s.client.NewRequest("DELETE", u, nil)
145158
if err != nil {
146159
return nil, err
@@ -151,11 +164,13 @@ func (s *OrganizationsService) PackageDeleteVersion(ctx context.Context, org, pa
151164

152165
// PackageRestoreVersion restores a package version to an organization.
153166
//
167+
// Note that packageName is escaped for the URL path so that you don't need to.
168+
//
154169
// GitHub API docs: https://docs.github.com/rest/packages/packages#restore-package-version-for-an-organization
155170
//
156171
//meta:operation POST /orgs/{org}/packages/{package_type}/{package_name}/versions/{package_version_id}/restore
157172
func (s *OrganizationsService) PackageRestoreVersion(ctx context.Context, org, packageType, packageName string, packageVersionID int64) (*Response, error) {
158-
u := fmt.Sprintf("orgs/%v/packages/%v/%v/versions/%v/restore", org, packageType, packageName, packageVersionID)
173+
u := fmt.Sprintf("orgs/%v/packages/%v/%v/versions/%v/restore", org, packageType, url.PathEscape(packageName), packageVersionID)
159174
req, err := s.client.NewRequest("POST", u, nil)
160175
if err != nil {
161176
return nil, err

github/orgs_packages_test.go

Lines changed: 57 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ package github
77

88
import (
99
"context"
10-
"fmt"
10+
"io"
1111
"net/http"
1212
"testing"
1313

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

2121
mux.HandleFunc("/orgs/o/packages", func(w http.ResponseWriter, r *http.Request) {
2222
testMethod(t, r, "GET")
23-
fmt.Fprint(w, `[{
23+
_, err := io.WriteString(w, `[{
2424
"id": 197,
2525
"name": "hello_docker",
2626
"package_type": "container",
@@ -52,6 +52,9 @@ func TestOrganizationsService_ListPackages(t *testing.T) {
5252
"html_url": "https://github.com/orgs/github/packages/container/package/hello_docker"
5353
}
5454
]`)
55+
if err != nil {
56+
t.Fatal("Failed to write test response: ", err)
57+
}
5558
})
5659

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

117-
mux.HandleFunc("/orgs/o/packages/container/hello_docker", func(w http.ResponseWriter, r *http.Request) {
120+
// don't url escape the package name here since mux will convert it to a slash automatically
121+
mux.HandleFunc("/orgs/o/packages/container/hello/hello_docker", func(w http.ResponseWriter, r *http.Request) {
118122
testMethod(t, r, "GET")
119-
fmt.Fprint(w, `{
123+
_, err := io.WriteString(w, `{
120124
"id": 197,
121-
"name": "hello_docker",
125+
"name": "hello/hello_docker",
122126
"package_type": "container",
123127
"version_count": 1,
124128
"visibility": "private",
125-
"url": "https://api.github.com/orgs/github/packages/container/hello_docker",
129+
"url": "https://api.github.com/orgs/github/packages/container/hello%2Fhello_docker",
126130
"created_at": `+referenceTimeStr+`,
127131
"updated_at": `+referenceTimeStr+`,
128-
"html_url": "https://github.com/orgs/github/packages/container/package/hello_docker"
132+
"html_url": "https://github.com/orgs/github/packages/container/package/hello%2Fhello_docker"
129133
}`)
134+
if err != nil {
135+
t.Fatal("Failed to write test response: ", err)
136+
}
130137
})
131138

132139
ctx := context.Background()
133-
packages, _, err := client.Organizations.GetPackage(ctx, "o", "container", "hello_docker")
140+
packages, _, err := client.Organizations.GetPackage(ctx, "o", "container", "hello/hello_docker")
134141
if err != nil {
135142
t.Errorf("Organizations.GetPackage returned error: %v", err)
136143
}
137144

138145
want := &Package{
139146
ID: Int64(197),
140-
Name: String("hello_docker"),
147+
Name: String("hello/hello_docker"),
141148
PackageType: String("container"),
142149
VersionCount: Int64(1),
143150
Visibility: String("private"),
144-
URL: String("https://api.github.com/orgs/github/packages/container/hello_docker"),
145-
HTMLURL: String("https://github.com/orgs/github/packages/container/package/hello_docker"),
151+
URL: String("https://api.github.com/orgs/github/packages/container/hello%2Fhello_docker"),
152+
HTMLURL: String("https://github.com/orgs/github/packages/container/package/hello%2Fhello_docker"),
146153
CreatedAt: &Timestamp{referenceTime},
147154
UpdatedAt: &Timestamp{referenceTime},
148155
}
@@ -169,12 +176,13 @@ func TestOrganizationsService_DeletePackage(t *testing.T) {
169176
client, mux, _, teardown := setup()
170177
defer teardown()
171178

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

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

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

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

217226
testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) {
218-
return client.Organizations.RestorePackage(ctx, "", "container", "hello_docker")
227+
return client.Organizations.RestorePackage(ctx, "", "container", "hello/hello_docker")
219228
})
220229
}
221230

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

226-
mux.HandleFunc("/orgs/o/packages/container/hello_docker/versions", func(w http.ResponseWriter, r *http.Request) {
235+
// don't url escape the package name here since mux will convert it to a slash automatically
236+
mux.HandleFunc("/orgs/o/packages/container/hello/hello_docker/versions", func(w http.ResponseWriter, r *http.Request) {
227237
testMethod(t, r, "GET")
228238
testFormValues(t, r, values{"per_page": "2", "page": "1", "state": "deleted", "visibility": "internal", "package_type": "container"})
229-
fmt.Fprint(w, `[
239+
_, err := io.WriteString(w, `[
230240
{
231241
"id": 45763,
232242
"name": "sha256:08a44bab0bddaddd8837a8b381aebc2e4b933768b981685a9e088360af0d3dd9",
233-
"url": "https://api.github.com/users/octocat/packages/container/hello_docker/versions/45763",
234-
"package_html_url": "https://github.com/users/octocat/packages/container/package/hello_docker",
243+
"url": "https://api.github.com/users/octocat/packages/container/hello%2Fhello_docker/versions/45763",
244+
"package_html_url": "https://github.com/users/octocat/packages/container/package/hello%2Fhello_docker",
235245
"created_at": `+referenceTimeStr+`,
236246
"updated_at": `+referenceTimeStr+`,
237-
"html_url": "https://github.com/users/octocat/packages/container/hello_docker/45763",
247+
"html_url": "https://github.com/users/octocat/packages/container/hello%2Fhello_docker/45763",
238248
"metadata": {
239249
"package_type": "container",
240250
"container": {
@@ -244,25 +254,28 @@ func TestOrganizationsService_ListPackagesVersions(t *testing.T) {
244254
}
245255
}
246256
}]`)
257+
if err != nil {
258+
t.Fatal("Failed to write test response: ", err)
259+
}
247260
})
248261

249262
ctx := context.Background()
250263
opts := &PackageListOptions{
251264
String("internal"), String("container"), String("deleted"), ListOptions{Page: 1, PerPage: 2},
252265
}
253-
packages, _, err := client.Organizations.PackageGetAllVersions(ctx, "o", "container", "hello_docker", opts)
266+
packages, _, err := client.Organizations.PackageGetAllVersions(ctx, "o", "container", "hello/hello_docker", opts)
254267
if err != nil {
255268
t.Errorf("Organizations.PackageGetAllVersions returned error: %v", err)
256269
}
257270

258271
want := []*PackageVersion{{
259272
ID: Int64(45763),
260273
Name: String("sha256:08a44bab0bddaddd8837a8b381aebc2e4b933768b981685a9e088360af0d3dd9"),
261-
URL: String("https://api.github.com/users/octocat/packages/container/hello_docker/versions/45763"),
262-
PackageHTMLURL: String("https://github.com/users/octocat/packages/container/package/hello_docker"),
274+
URL: String("https://api.github.com/users/octocat/packages/container/hello%2Fhello_docker/versions/45763"),
275+
PackageHTMLURL: String("https://github.com/users/octocat/packages/container/package/hello%2Fhello_docker"),
263276
CreatedAt: &Timestamp{referenceTime},
264277
UpdatedAt: &Timestamp{referenceTime},
265-
HTMLURL: String("https://github.com/users/octocat/packages/container/hello_docker/45763"),
278+
HTMLURL: String("https://github.com/users/octocat/packages/container/hello%2Fhello_docker/45763"),
266279
Metadata: &PackageMetadata{
267280
PackageType: String("container"),
268281
Container: &PackageContainerMetadata{
@@ -293,17 +306,18 @@ func TestOrganizationsService_PackageGetVersion(t *testing.T) {
293306
client, mux, _, teardown := setup()
294307
defer teardown()
295308

296-
mux.HandleFunc("/orgs/o/packages/container/hello_docker/versions/45763", func(w http.ResponseWriter, r *http.Request) {
309+
// don't url escape the package name here since mux will convert it to a slash automatically
310+
mux.HandleFunc("/orgs/o/packages/container/hello/hello_docker/versions/45763", func(w http.ResponseWriter, r *http.Request) {
297311
testMethod(t, r, "GET")
298-
fmt.Fprint(w, `
312+
_, err := io.WriteString(w, `
299313
{
300314
"id": 45763,
301315
"name": "sha256:08a44bab0bddaddd8837a8b381aebc2e4b933768b981685a9e088360af0d3dd9",
302-
"url": "https://api.github.com/users/octocat/packages/container/hello_docker/versions/45763",
303-
"package_html_url": "https://github.com/users/octocat/packages/container/package/hello_docker",
316+
"url": "https://api.github.com/users/octocat/packages/container/hello%2Fhello_docker/versions/45763",
317+
"package_html_url": "https://github.com/users/octocat/packages/container/package/hello%2Fhello_docker",
304318
"created_at": `+referenceTimeStr+`,
305319
"updated_at": `+referenceTimeStr+`,
306-
"html_url": "https://github.com/users/octocat/packages/container/hello_docker/45763",
320+
"html_url": "https://github.com/users/octocat/packages/container/hello%2Fhello_docker/45763",
307321
"metadata": {
308322
"package_type": "container",
309323
"container": {
@@ -313,22 +327,25 @@ func TestOrganizationsService_PackageGetVersion(t *testing.T) {
313327
}
314328
}
315329
}`)
330+
if err != nil {
331+
t.Fatal("Failed to write test response: ", err)
332+
}
316333
})
317334

318335
ctx := context.Background()
319-
packages, _, err := client.Organizations.PackageGetVersion(ctx, "o", "container", "hello_docker", 45763)
336+
packages, _, err := client.Organizations.PackageGetVersion(ctx, "o", "container", "hello/hello_docker", 45763)
320337
if err != nil {
321338
t.Errorf("Organizations.PackageGetVersion returned error: %v", err)
322339
}
323340

324341
want := &PackageVersion{
325342
ID: Int64(45763),
326343
Name: String("sha256:08a44bab0bddaddd8837a8b381aebc2e4b933768b981685a9e088360af0d3dd9"),
327-
URL: String("https://api.github.com/users/octocat/packages/container/hello_docker/versions/45763"),
328-
PackageHTMLURL: String("https://github.com/users/octocat/packages/container/package/hello_docker"),
344+
URL: String("https://api.github.com/users/octocat/packages/container/hello%2Fhello_docker/versions/45763"),
345+
PackageHTMLURL: String("https://github.com/users/octocat/packages/container/package/hello%2Fhello_docker"),
329346
CreatedAt: &Timestamp{referenceTime},
330347
UpdatedAt: &Timestamp{referenceTime},
331-
HTMLURL: String("https://github.com/users/octocat/packages/container/hello_docker/45763"),
348+
HTMLURL: String("https://github.com/users/octocat/packages/container/hello%2Fhello_docker/45763"),
332349
Metadata: &PackageMetadata{
333350
PackageType: String("container"),
334351
Container: &PackageContainerMetadata{
@@ -359,12 +376,13 @@ func TestOrganizationsService_PackageDeleteVersion(t *testing.T) {
359376
client, mux, _, teardown := setup()
360377
defer teardown()
361378

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

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

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

391410
ctx := context.Background()
392-
_, err := client.Organizations.PackageRestoreVersion(ctx, "o", "container", "hello_docker", 45763)
411+
_, err := client.Organizations.PackageRestoreVersion(ctx, "o", "container", "hello/hello_docker", 45763)
393412
if err != nil {
394413
t.Errorf("Organizations.PackageRestoreVersion returned error: %v", err)
395414
}

0 commit comments

Comments
 (0)