From ed70edb87c2d7aaaadbae1131a61c7afb1486935 Mon Sep 17 00:00:00 2001 From: Rakesh S Date: Thu, 4 Dec 2025 10:41:23 +0530 Subject: [PATCH 1/4] repos: add UploadReleaseAssetFromRelease convenience helper that uses release.UploadURL (fixes #3849) --- github/repos_releases.go | 56 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/github/repos_releases.go b/github/repos_releases.go index b5cff73260b..750686bf158 100644 --- a/github/repos_releases.go +++ b/github/repos_releases.go @@ -488,3 +488,59 @@ func (s *RepositoriesService) UploadReleaseAsset(ctx context.Context, owner, rep } return asset, resp, nil } + +// UploadReleaseAssetFromRelease uploads an asset using the UploadURL that's embedded +// in a RepositoryRelease object. +// +// This is a convenience wrapper that extracts the release.UploadURL (which is usually +// templated like "https://uploads.github.com/.../assets{?name,label}") and uploads +// the provided file using the existing upload helpers. +// +// GitHub API docs: https://docs.github.com/rest/releases/assets#upload-a-release-asset +// +//meta:operation POST /repos/{owner}/{repo}/releases/{release_id}/assets +func (s *RepositoriesService) UploadReleaseAssetFromRelease(ctx context.Context, release *RepositoryRelease, opts *UploadOptions, file *os.File) (*ReleaseAsset, *Response, error) { + if release == nil || release.UploadURL == nil { + return nil, nil, errors.New("release UploadURL must be provided") + } + if file == nil { + return nil, nil, errors.New("file must be provided") + } + + // Extract upload URL and remove template part (e.g. "{?name,label}") if present. + uploadURL := *release.UploadURL + if idx := strings.Index(uploadURL, "{"); idx != -1 { + uploadURL = uploadURL[:idx] + } + + // addOptions will append query params for name/label (same as UploadReleaseAsset) + u, err := addOptions(uploadURL, opts) + if err != nil { + return nil, nil, err + } + + stat, err := file.Stat() + if err != nil { + return nil, nil, err + } + if stat.IsDir() { + return nil, nil, errors.New("the asset to upload can't be a directory") + } + + mediaType := mime.TypeByExtension(filepath.Ext(file.Name())) + if opts != nil && opts.MediaType != "" { + mediaType = opts.MediaType + } + + req, err := s.client.NewUploadRequest(u, file, stat.Size(), mediaType) + if err != nil { + return nil, nil, err + } + + asset := new(ReleaseAsset) + resp, err := s.client.Do(ctx, req, asset) + if err != nil { + return nil, resp, err + } + return asset, resp, nil +} From 69a2c8b0f55bedd79e920389268393c147999b43 Mon Sep 17 00:00:00 2001 From: Rakesh S Date: Thu, 4 Dec 2025 21:11:54 +0530 Subject: [PATCH 2/4] repos: add UploadReleaseAssetFromRelease helper and tests; normalize upload_url --- github/repos_releases.go | 15 ++++++- github/repos_releases_test.go | 77 +++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 1 deletion(-) diff --git a/github/repos_releases.go b/github/repos_releases.go index 750686bf158..7b6d1ac12a2 100644 --- a/github/repos_releases.go +++ b/github/repos_releases.go @@ -12,6 +12,7 @@ import ( "io" "mime" "net/http" + "net/url" "os" "path/filepath" "strings" @@ -507,12 +508,24 @@ func (s *RepositoriesService) UploadReleaseAssetFromRelease(ctx context.Context, return nil, nil, errors.New("file must be provided") } - // Extract upload URL and remove template part (e.g. "{?name,label}") if present. + // Extract upload URL. uploadURL := *release.UploadURL + + // If uploadURL contains a template, strip it (e.g. "{?name,label}"). if idx := strings.Index(uploadURL, "{"); idx != -1 { uploadURL = uploadURL[:idx] } + // If uploadURL is absolute (starts with http/https), parse and use only the path. + if strings.HasPrefix(uploadURL, "http://") || strings.HasPrefix(uploadURL, "https://") { + if uParsed, err := url.Parse(uploadURL); err == nil { + uploadURL = uParsed.Path + } + } + + // Defensive: always remove any leading '/' so client gets a relative path. + uploadURL = strings.TrimPrefix(uploadURL, "/") + // addOptions will append query params for name/label (same as UploadReleaseAsset) u, err := addOptions(uploadURL, opts) if err != nil { diff --git a/github/repos_releases_test.go b/github/repos_releases_test.go index 6d2d5fd265d..73899f664fe 100644 --- a/github/repos_releases_test.go +++ b/github/repos_releases_test.go @@ -781,6 +781,83 @@ func TestRepositoriesService_UploadReleaseAsset(t *testing.T) { } } +func TestRepositoriesService_UploadReleaseAssetFromRelease(t *testing.T) { + t.Parallel() + + var ( + defaultUploadOptions = &UploadOptions{Name: "n"} + defaultExpectedFormValue = values{"name": "n"} + mediaTypeTextPlain = "text/plain; charset=utf-8" + ) + + client, mux, _ := setup(t) + + // Use the same endpoint path used in other tests. + mux.HandleFunc("/repos/o/r/releases/1/assets", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "POST") + testHeader(t, r, "Content-Type", mediaTypeTextPlain) + testHeader(t, r, "Content-Length", "12") + testFormValues(t, r, defaultExpectedFormValue) + testBody(t, r, "Upload me !\n") + + fmt.Fprint(w, `{"id":1}`) + }) + + // Create a file using the test helper that existing tests use. + file := openTestFile(t, "upload.txt", "Upload me !\n") + + // Provide a templated upload URL like GitHub returns. + uploadURL := "/repos/o/r/releases/1/assets{?name,label}" + release := &RepositoryRelease{ + UploadURL: &uploadURL, + } + + ctx := t.Context() + asset, _, err := client.Repositories.UploadReleaseAssetFromRelease(ctx, release, defaultUploadOptions, file) + if err != nil { + t.Fatalf("Repositories.UploadReleaseAssetFromRelease returned error: %v", err) + } + want := &ReleaseAsset{ID: Ptr(int64(1))} + if !cmp.Equal(asset, want) { + t.Fatalf("Repositories.UploadReleaseAssetFromRelease returned %+v, want %+v", asset, want) + } + + const methodName = "UploadReleaseAssetFromRelease" + testBadOptions(t, methodName, func() (err error) { + _, _, err = client.Repositories.UploadReleaseAssetFromRelease(ctx, release, defaultUploadOptions, nil) + return err + }) +} + +func TestRepositoriesService_UploadReleaseAssetFromRelease_AbsoluteTemplate(t *testing.T) { + t.Parallel() + client, mux, _ := setup(t) + + mux.HandleFunc("/repos/o/r/releases/1/assets", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "POST") + // Expect name query param created by addOptions after trimming template. + if r.URL.Query().Get("name") != "abs.txt" { + t.Errorf("Expected name query param 'abs.txt', got %q", r.URL.Query().Get("name")) + } + fmt.Fprint(w, `{"id":1}`) + }) + + file := openTestFile(t, "upload.txt", "Upload me !\n") + + uploadURL := "https://uploads.github.com/repos/o/r/releases/1/assets{?name,label}" + release := &RepositoryRelease{UploadURL: &uploadURL} + + opts := &UploadOptions{Name: "abs.txt"} + asset, _, err := client.Repositories.UploadReleaseAssetFromRelease(t.Context(), release, opts, file) + if err != nil { + t.Fatalf("UploadReleaseAssetFromRelease returned error: %v", err) + } + want := &ReleaseAsset{ID: Ptr(int64(1))} + if !cmp.Equal(asset, want) { + t.Fatalf("UploadReleaseAssetFromRelease returned %+v, want %+v", asset, want) + } +} + func TestRepositoryReleaseRequest_Marshal(t *testing.T) { t.Parallel() testJSONMarshal(t, &repositoryReleaseRequest{}, "{}") From a7a6dcb5587a5519564526c60f0f4c4f745de509 Mon Sep 17 00:00:00 2001 From: Rakesh S Date: Sat, 6 Dec 2025 18:07:14 +0530 Subject: [PATCH 3/4] repos: add UploadReleaseAssetFromRelease helper and tests; preserve absolute upload URLs This adds a convenience helper that uploads a release asset using the UploadURL returned in RepositoryRelease. Changes included: - Accept io.Reader + size instead of *os.File (consistent with NewUploadRequest) - Strip only the URI-template portion - Preserve absolute upload URLs exactly as provided (no path rewriting) - Add comprehensive tests for all branches (nil release, nil reader, negative size, absolute URLs, media type override, bad options, and upload success cases) This behavior ensures correct handling of GitHub Enterprise upload endpoints and aligns with the intent described in #3849. Fixes #3849. --- github/repos_releases.go | 41 ++--- github/repos_releases_test.go | 280 ++++++++++++++++++++++++---------- 2 files changed, 215 insertions(+), 106 deletions(-) diff --git a/github/repos_releases.go b/github/repos_releases.go index 7b6d1ac12a2..5d30beaf224 100644 --- a/github/repos_releases.go +++ b/github/repos_releases.go @@ -12,7 +12,6 @@ import ( "io" "mime" "net/http" - "net/url" "os" "path/filepath" "strings" @@ -495,57 +494,41 @@ func (s *RepositoriesService) UploadReleaseAsset(ctx context.Context, owner, rep // // This is a convenience wrapper that extracts the release.UploadURL (which is usually // templated like "https://uploads.github.com/.../assets{?name,label}") and uploads -// the provided file using the existing upload helpers. +// the provided data (reader + size) using the existing upload helpers. // // GitHub API docs: https://docs.github.com/rest/releases/assets#upload-a-release-asset // //meta:operation POST /repos/{owner}/{repo}/releases/{release_id}/assets -func (s *RepositoriesService) UploadReleaseAssetFromRelease(ctx context.Context, release *RepositoryRelease, opts *UploadOptions, file *os.File) (*ReleaseAsset, *Response, error) { +func (s *RepositoriesService) UploadReleaseAssetFromRelease(ctx context.Context, release *RepositoryRelease, opts *UploadOptions, reader io.Reader, size int64) (*ReleaseAsset, *Response, error) { if release == nil || release.UploadURL == nil { return nil, nil, errors.New("release UploadURL must be provided") } - if file == nil { - return nil, nil, errors.New("file must be provided") + if reader == nil { + return nil, nil, errors.New("reader must be provided") + } + if size < 0 { + return nil, nil, errors.New("size must be >= 0") } - // Extract upload URL. + // Strip URI-template portion (e.g. "{?name,label}") if present. uploadURL := *release.UploadURL - - // If uploadURL contains a template, strip it (e.g. "{?name,label}"). if idx := strings.Index(uploadURL, "{"); idx != -1 { uploadURL = uploadURL[:idx] } - // If uploadURL is absolute (starts with http/https), parse and use only the path. - if strings.HasPrefix(uploadURL, "http://") || strings.HasPrefix(uploadURL, "https://") { - if uParsed, err := url.Parse(uploadURL); err == nil { - uploadURL = uParsed.Path - } - } - - // Defensive: always remove any leading '/' so client gets a relative path. - uploadURL = strings.TrimPrefix(uploadURL, "/") - - // addOptions will append query params for name/label (same as UploadReleaseAsset) + // IMPORTANT: preserve absolute upload URLs. Do NOT convert to relative paths. + // addOptions will append name/label query params (same behavior as UploadReleaseAsset). u, err := addOptions(uploadURL, opts) if err != nil { return nil, nil, err } - stat, err := file.Stat() - if err != nil { - return nil, nil, err - } - if stat.IsDir() { - return nil, nil, errors.New("the asset to upload can't be a directory") - } - - mediaType := mime.TypeByExtension(filepath.Ext(file.Name())) + mediaType := defaultMediaType if opts != nil && opts.MediaType != "" { mediaType = opts.MediaType } - req, err := s.client.NewUploadRequest(u, file, stat.Size(), mediaType) + req, err := s.client.NewUploadRequest(u, reader, size, mediaType) if err != nil { return nil, nil, err } diff --git a/github/repos_releases_test.go b/github/repos_releases_test.go index 73899f664fe..20b92357543 100644 --- a/github/repos_releases_test.go +++ b/github/repos_releases_test.go @@ -781,83 +781,6 @@ func TestRepositoriesService_UploadReleaseAsset(t *testing.T) { } } -func TestRepositoriesService_UploadReleaseAssetFromRelease(t *testing.T) { - t.Parallel() - - var ( - defaultUploadOptions = &UploadOptions{Name: "n"} - defaultExpectedFormValue = values{"name": "n"} - mediaTypeTextPlain = "text/plain; charset=utf-8" - ) - - client, mux, _ := setup(t) - - // Use the same endpoint path used in other tests. - mux.HandleFunc("/repos/o/r/releases/1/assets", func(w http.ResponseWriter, r *http.Request) { - testMethod(t, r, "POST") - testHeader(t, r, "Content-Type", mediaTypeTextPlain) - testHeader(t, r, "Content-Length", "12") - testFormValues(t, r, defaultExpectedFormValue) - testBody(t, r, "Upload me !\n") - - fmt.Fprint(w, `{"id":1}`) - }) - - // Create a file using the test helper that existing tests use. - file := openTestFile(t, "upload.txt", "Upload me !\n") - - // Provide a templated upload URL like GitHub returns. - uploadURL := "/repos/o/r/releases/1/assets{?name,label}" - release := &RepositoryRelease{ - UploadURL: &uploadURL, - } - - ctx := t.Context() - asset, _, err := client.Repositories.UploadReleaseAssetFromRelease(ctx, release, defaultUploadOptions, file) - if err != nil { - t.Fatalf("Repositories.UploadReleaseAssetFromRelease returned error: %v", err) - } - want := &ReleaseAsset{ID: Ptr(int64(1))} - if !cmp.Equal(asset, want) { - t.Fatalf("Repositories.UploadReleaseAssetFromRelease returned %+v, want %+v", asset, want) - } - - const methodName = "UploadReleaseAssetFromRelease" - testBadOptions(t, methodName, func() (err error) { - _, _, err = client.Repositories.UploadReleaseAssetFromRelease(ctx, release, defaultUploadOptions, nil) - return err - }) -} - -func TestRepositoriesService_UploadReleaseAssetFromRelease_AbsoluteTemplate(t *testing.T) { - t.Parallel() - client, mux, _ := setup(t) - - mux.HandleFunc("/repos/o/r/releases/1/assets", func(w http.ResponseWriter, r *http.Request) { - testMethod(t, r, "POST") - // Expect name query param created by addOptions after trimming template. - if r.URL.Query().Get("name") != "abs.txt" { - t.Errorf("Expected name query param 'abs.txt', got %q", r.URL.Query().Get("name")) - } - fmt.Fprint(w, `{"id":1}`) - }) - - file := openTestFile(t, "upload.txt", "Upload me !\n") - - uploadURL := "https://uploads.github.com/repos/o/r/releases/1/assets{?name,label}" - release := &RepositoryRelease{UploadURL: &uploadURL} - - opts := &UploadOptions{Name: "abs.txt"} - asset, _, err := client.Repositories.UploadReleaseAssetFromRelease(t.Context(), release, opts, file) - if err != nil { - t.Fatalf("UploadReleaseAssetFromRelease returned error: %v", err) - } - want := &ReleaseAsset{ID: Ptr(int64(1))} - if !cmp.Equal(asset, want) { - t.Fatalf("UploadReleaseAssetFromRelease returned %+v, want %+v", asset, want) - } -} - func TestRepositoryReleaseRequest_Marshal(t *testing.T) { t.Parallel() testJSONMarshal(t, &repositoryReleaseRequest{}, "{}") @@ -1007,3 +930,206 @@ func TestGenerateNotesOptions_Marshal(t *testing.T) { testJSONMarshal(t, u, want) } + +func TestRepositoriesService_UploadReleaseAssetFromRelease(t *testing.T) { + t.Parallel() + + var ( + defaultUploadOptions = &UploadOptions{Name: "n"} + defaultExpectedFormValue = values{"name": "n"} + mediaTypeTextPlain = "text/plain; charset=utf-8" + ) + + client, mux, _ := setup(t) + + // Use the same endpoint path used in other release asset tests. + mux.HandleFunc("/repos/o/r/releases/1/assets", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "POST") + testHeader(t, r, "Content-Type", mediaTypeTextPlain) + testHeader(t, r, "Content-Length", "12") + testFormValues(t, r, defaultExpectedFormValue) + testBody(t, r, "Upload me !\n") + + fmt.Fprint(w, `{"id":1}`) + }) + + body := []byte("Upload me !\n") + reader := bytes.NewReader(body) + size := int64(len(body)) + + // Provide a templated upload URL like GitHub returns. + uploadURL := "/repos/o/r/releases/1/assets{?name,label}" + release := &RepositoryRelease{ + UploadURL: &uploadURL, + } + + ctx := t.Context() + asset, _, err := client.Repositories.UploadReleaseAssetFromRelease(ctx, release, defaultUploadOptions, reader, size) + if err != nil { + t.Fatalf("Repositories.UploadReleaseAssetFromRelease returned error: %v", err) + } + want := &ReleaseAsset{ID: Ptr(int64(1))} + if !cmp.Equal(asset, want) { + t.Fatalf("Repositories.UploadReleaseAssetFromRelease returned %+v, want %+v", asset, want) + } +} + +func TestRepositoriesService_UploadReleaseAssetFromRelease_AbsoluteTemplate(t *testing.T) { + t.Parallel() + client, mux, _ := setup(t) + + mux.HandleFunc("/repos/o/r/releases/1/assets", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "POST") + // Expect name query param created by addOptions after trimming template. + if got := r.URL.Query().Get("name"); got != "abs.txt" { + t.Errorf("Expected name query param 'abs.txt', got %q", got) + } + fmt.Fprint(w, `{"id":1}`) + }) + + body := []byte("Upload me !\n") + reader := bytes.NewReader(body) + size := int64(len(body)) + + // Build an absolute URL using the test client's BaseURL. + absoluteUploadURL := client.BaseURL.String() + "repos/o/r/releases/1/assets{?name,label}" + release := &RepositoryRelease{UploadURL: &absoluteUploadURL} + + opts := &UploadOptions{Name: "abs.txt"} + ctx := t.Context() + asset, _, err := client.Repositories.UploadReleaseAssetFromRelease(ctx, release, opts, reader, size) + if err != nil { + t.Fatalf("UploadReleaseAssetFromRelease returned error: %v", err) + } + want := &ReleaseAsset{ID: Ptr(int64(1))} + if !cmp.Equal(asset, want) { + t.Fatalf("UploadReleaseAssetFromRelease returned %+v, want %+v", asset, want) + } +} + +func TestRepositoriesService_UploadReleaseAssetFromRelease_NilRelease(t *testing.T) { + t.Parallel() + client, _, _ := setup(t) + + body := []byte("Upload me !\n") + reader := bytes.NewReader(body) + size := int64(len(body)) + + ctx := t.Context() + _, _, err := client.Repositories.UploadReleaseAssetFromRelease(ctx, nil, &UploadOptions{Name: "n"}, reader, size) + if err == nil { + t.Fatal("expected error for nil release, got nil") + } + + const methodName = "UploadReleaseAssetFromRelease" + testBadOptions(t, methodName, func() (err error) { + _, _, err = client.Repositories.UploadReleaseAssetFromRelease(ctx, nil, &UploadOptions{Name: "n"}, reader, size) + return err + }) +} + +func TestRepositoriesService_UploadReleaseAssetFromRelease_NilReader(t *testing.T) { + t.Parallel() + client, _, _ := setup(t) + + uploadURL := "/repos/o/r/releases/1/assets{?name,label}" + release := &RepositoryRelease{UploadURL: &uploadURL} + + ctx := t.Context() + _, _, err := client.Repositories.UploadReleaseAssetFromRelease(ctx, release, &UploadOptions{Name: "n"}, nil, 12) + if err == nil { + t.Fatal("expected error when reader is nil") + } + + const methodName = "UploadReleaseAssetFromRelease" + testBadOptions(t, methodName, func() (err error) { + _, _, err = client.Repositories.UploadReleaseAssetFromRelease(ctx, release, &UploadOptions{Name: "n"}, nil, 12) + return err + }) +} + +func TestRepositoriesService_UploadReleaseAssetFromRelease_NegativeSize(t *testing.T) { + t.Parallel() + client, _, _ := setup(t) + + uploadURL := "/repos/o/r/releases/1/assets{?name,label}" + release := &RepositoryRelease{UploadURL: &uploadURL} + + body := []byte("Upload me !\n") + reader := bytes.NewReader(body) + + ctx := t.Context() + _, _, err := client.Repositories.UploadReleaseAssetFromRelease(ctx, release, &UploadOptions{Name: "n"}, reader, -1) + if err == nil { + t.Fatal("expected error when size is negative") + } +} + +func TestRepositoriesService_UploadReleaseAssetFromRelease_NoOpts(t *testing.T) { + t.Parallel() + client, mux, _ := setup(t) + + // No opts: we just assert that the handler is hit and body is as expected. + mux.HandleFunc("/repos/o/r/releases/1/assets", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "POST") + testBody(t, r, "Upload me !\n") + fmt.Fprint(w, `{"id":1}`) + }) + + body := []byte("Upload me !\n") + reader := bytes.NewReader(body) + size := int64(len(body)) + + uploadURL := "/repos/o/r/releases/1/assets{?name,label}" + release := &RepositoryRelease{UploadURL: &uploadURL} + + ctx := t.Context() + asset, _, err := client.Repositories.UploadReleaseAssetFromRelease(ctx, release, nil, reader, size) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + want := &ReleaseAsset{ID: Ptr(int64(1))} + if !cmp.Equal(asset, want) { + t.Fatalf("Repositories.UploadReleaseAssetFromRelease returned %+v, want %+v", asset, want) + } + + const methodName = "UploadReleaseAssetFromRelease" + testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { + got, resp, err := client.Repositories.UploadReleaseAssetFromRelease(ctx, release, nil, reader, size) + if got != nil { + t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) + } + return resp, err + }) +} + +func TestRepositoriesService_UploadReleaseAssetFromRelease_WithMediaType(t *testing.T) { + t.Parallel() + client, mux, _ := setup(t) + + // Expect explicit media type to be used. + mux.HandleFunc("/repos/o/r/releases/1/assets", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "POST") + testHeader(t, r, "Content-Type", "image/png") + fmt.Fprint(w, `{"id":1}`) + }) + + body := []byte("Binary!") + reader := bytes.NewReader(body) + size := int64(len(body)) + + uploadURL := "/repos/o/r/releases/1/assets{?name,label}" + release := &RepositoryRelease{UploadURL: &uploadURL} + + opts := &UploadOptions{Name: "n", MediaType: "image/png"} + + ctx := t.Context() + asset, _, err := client.Repositories.UploadReleaseAssetFromRelease(ctx, release, opts, reader, size) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + want := &ReleaseAsset{ID: Ptr(int64(1))} + if !cmp.Equal(asset, want) { + t.Fatalf("UploadReleaseAssetFromRelease returned %+v, want %+v", asset, want) + } +} From 501de8f8cfe1e54f1ea2b93173a7e9fc34391bcf Mon Sep 17 00:00:00 2001 From: Rakesh S Date: Sun, 7 Dec 2025 09:19:48 +0530 Subject: [PATCH 4/4] repos: finalize UploadReleaseAssetFromRelease helper and tests - Preserve absolute upload URLs exactly as returned by the API - Trim only the URI-template portion - Align media-type resolution with UploadReleaseAsset - Update tests to cover all branches and BaseURL-prefixed paths - Improve error coverage using existing test helpers --- github/repos_releases.go | 22 ++++++++++++++++++++-- github/repos_releases_test.go | 28 ++++++++++++++-------------- 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/github/repos_releases.go b/github/repos_releases.go index 5d30beaf224..fa526e8c0a9 100644 --- a/github/repos_releases.go +++ b/github/repos_releases.go @@ -499,7 +499,13 @@ func (s *RepositoriesService) UploadReleaseAsset(ctx context.Context, owner, rep // GitHub API docs: https://docs.github.com/rest/releases/assets#upload-a-release-asset // //meta:operation POST /repos/{owner}/{repo}/releases/{release_id}/assets -func (s *RepositoriesService) UploadReleaseAssetFromRelease(ctx context.Context, release *RepositoryRelease, opts *UploadOptions, reader io.Reader, size int64) (*ReleaseAsset, *Response, error) { +func (s *RepositoriesService) UploadReleaseAssetFromRelease( + ctx context.Context, + release *RepositoryRelease, + opts *UploadOptions, + reader io.Reader, + size int64, +) (*ReleaseAsset, *Response, error) { if release == nil || release.UploadURL == nil { return nil, nil, errors.New("release UploadURL must be provided") } @@ -516,16 +522,28 @@ func (s *RepositoriesService) UploadReleaseAssetFromRelease(ctx context.Context, uploadURL = uploadURL[:idx] } - // IMPORTANT: preserve absolute upload URLs. Do NOT convert to relative paths. + // If this is a *relative* URL (no scheme), normalize it by trimming a leading "/" + // so it works with Client.BaseURL path prefixes (e.g. "/api-v3/"). + if !strings.HasPrefix(uploadURL, "http://") && !strings.HasPrefix(uploadURL, "https://") { + uploadURL = strings.TrimPrefix(uploadURL, "/") + } + // addOptions will append name/label query params (same behavior as UploadReleaseAsset). u, err := addOptions(uploadURL, opts) if err != nil { return nil, nil, err } + // determine media type mediaType := defaultMediaType if opts != nil && opts.MediaType != "" { mediaType = opts.MediaType + } else if opts != nil && opts.Name != "" { + if ext := filepath.Ext(opts.Name); ext != "" { + if mt := mime.TypeByExtension(ext); mt != "" { + mediaType = mt + } + } } req, err := s.client.NewUploadRequest(u, reader, size, mediaType) diff --git a/github/repos_releases_test.go b/github/repos_releases_test.go index 20b92357543..a73ffd3083e 100644 --- a/github/repos_releases_test.go +++ b/github/repos_releases_test.go @@ -689,8 +689,8 @@ func TestRepositoriesService_DeleteReleaseAsset(t *testing.T) { func TestRepositoriesService_UploadReleaseAsset(t *testing.T) { t.Parallel() var ( - defaultUploadOptions = &UploadOptions{Name: "n"} - defaultExpectedFormValue = values{"name": "n"} + defaultUploadOptions = &UploadOptions{Name: "n.txt"} + defaultExpectedFormValue = values{"name": "n.txt"} mediaTypeTextPlain = "text/plain; charset=utf-8" ) uploadTests := []struct { @@ -715,23 +715,23 @@ func TestRepositoriesService_UploadReleaseAsset(t *testing.T) { }, // No file extension and explicit media type. { - &UploadOptions{Name: "n", MediaType: "image/png"}, + &UploadOptions{Name: "n.txt", MediaType: "image/png"}, "upload", defaultExpectedFormValue, "image/png", }, // File extension and explicit media type. { - &UploadOptions{Name: "n", MediaType: "image/png"}, + &UploadOptions{Name: "n.txt", MediaType: "image/png"}, "upload.png", defaultExpectedFormValue, "image/png", }, // Label provided. { - &UploadOptions{Name: "n", Label: "l"}, + &UploadOptions{Name: "n.txt", Label: "l"}, "upload.txt", - values{"name": "n", "label": "l"}, + values{"name": "n.txt", "label": "l"}, mediaTypeTextPlain, }, // No label provided. @@ -935,8 +935,8 @@ func TestRepositoriesService_UploadReleaseAssetFromRelease(t *testing.T) { t.Parallel() var ( - defaultUploadOptions = &UploadOptions{Name: "n"} - defaultExpectedFormValue = values{"name": "n"} + defaultUploadOptions = &UploadOptions{Name: "n.txt"} + defaultExpectedFormValue = values{"name": "n.txt"} mediaTypeTextPlain = "text/plain; charset=utf-8" ) @@ -1016,14 +1016,14 @@ func TestRepositoriesService_UploadReleaseAssetFromRelease_NilRelease(t *testing size := int64(len(body)) ctx := t.Context() - _, _, err := client.Repositories.UploadReleaseAssetFromRelease(ctx, nil, &UploadOptions{Name: "n"}, reader, size) + _, _, err := client.Repositories.UploadReleaseAssetFromRelease(ctx, nil, &UploadOptions{Name: "n.txt"}, reader, size) if err == nil { t.Fatal("expected error for nil release, got nil") } const methodName = "UploadReleaseAssetFromRelease" testBadOptions(t, methodName, func() (err error) { - _, _, err = client.Repositories.UploadReleaseAssetFromRelease(ctx, nil, &UploadOptions{Name: "n"}, reader, size) + _, _, err = client.Repositories.UploadReleaseAssetFromRelease(ctx, nil, &UploadOptions{Name: "n.txt"}, reader, size) return err }) } @@ -1036,14 +1036,14 @@ func TestRepositoriesService_UploadReleaseAssetFromRelease_NilReader(t *testing. release := &RepositoryRelease{UploadURL: &uploadURL} ctx := t.Context() - _, _, err := client.Repositories.UploadReleaseAssetFromRelease(ctx, release, &UploadOptions{Name: "n"}, nil, 12) + _, _, err := client.Repositories.UploadReleaseAssetFromRelease(ctx, release, &UploadOptions{Name: "n.txt"}, nil, 12) if err == nil { t.Fatal("expected error when reader is nil") } const methodName = "UploadReleaseAssetFromRelease" testBadOptions(t, methodName, func() (err error) { - _, _, err = client.Repositories.UploadReleaseAssetFromRelease(ctx, release, &UploadOptions{Name: "n"}, nil, 12) + _, _, err = client.Repositories.UploadReleaseAssetFromRelease(ctx, release, &UploadOptions{Name: "n.txt"}, nil, 12) return err }) } @@ -1059,7 +1059,7 @@ func TestRepositoriesService_UploadReleaseAssetFromRelease_NegativeSize(t *testi reader := bytes.NewReader(body) ctx := t.Context() - _, _, err := client.Repositories.UploadReleaseAssetFromRelease(ctx, release, &UploadOptions{Name: "n"}, reader, -1) + _, _, err := client.Repositories.UploadReleaseAssetFromRelease(ctx, release, &UploadOptions{Name: "n..txt"}, reader, -1) if err == nil { t.Fatal("expected error when size is negative") } @@ -1121,7 +1121,7 @@ func TestRepositoriesService_UploadReleaseAssetFromRelease_WithMediaType(t *test uploadURL := "/repos/o/r/releases/1/assets{?name,label}" release := &RepositoryRelease{UploadURL: &uploadURL} - opts := &UploadOptions{Name: "n", MediaType: "image/png"} + opts := &UploadOptions{Name: "n.txt", MediaType: "image/png"} ctx := t.Context() asset, _, err := client.Repositories.UploadReleaseAssetFromRelease(ctx, release, opts, reader, size)