Skip to content

Fix NuGet package upload error handling#37074

Merged
silverwind merged 8 commits intogo-gitea:mainfrom
silverwind:fix-nuget-upload
Apr 1, 2026
Merged

Fix NuGet package upload error handling#37074
silverwind merged 8 commits intogo-gitea:mainfrom
silverwind:fix-nuget-upload

Conversation

@silverwind
Copy link
Copy Markdown
Member

@silverwind silverwind commented Apr 1, 2026

Wrap zip.NewReader errors in NuGet ParsePackageMetaData and ExtractPortablePdb as ErrInvalidArgument so invalid packages return HTTP 400 (Bad Request) instead of 500 (Internal Server Error).

Add integration test for multipart/form-data NuGet upload path (used by dotnet nuget push) which was previously untested.

Note: I was unable to reproduce the reported error with dotnet nuget push (.NET 10) against a local Gitea instance — the upload succeeded. The reporter's issue may be environment-specific (reverse proxy configuration, .NET version, etc). These changes improve error reporting for invalid uploads regardless.

Ref #36932


This PR was written with the help of Claude Opus 4.6

Fix `UploadStream()` to guard against nil `MultipartForm` which can
occur when `ParseMultipartForm` silently returns nil on malformed
Content-Type boundaries (e.g. unquoted boundaries containing `=`).
Also remove the `application/x-www-form-urlencoded` branch which
would always fail for multipart parsing.

Wrap `zip.NewReader` errors in NuGet `ParsePackageMetaData` and
`ExtractPortablePdb` as `ErrInvalidArgument` so invalid packages
return HTTP 400 instead of 500.

Add integration test for multipart/form-data NuGet upload path
(used by `dotnet nuget push`) which was previously untested.

Fixes go-gitea#36932

Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 1, 2026
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Apr 1, 2026
Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Improves NuGet package upload robustness by preventing panics on malformed multipart requests and ensuring invalid ZIP uploads are treated as client errors (HTTP 400), plus adds an integration test covering multipart/form-data uploads.

Changes:

  • Guard Context.UploadStream() against a nil Req.MultipartForm after ParseMultipartForm, avoiding a potential nil dereference and removing a dead application/x-www-form-urlencoded branch.
  • Wrap zip.NewReader failures in NuGet metadata/symbol parsing as util.ErrInvalidArgument to return HTTP 400 for invalid packages instead of HTTP 500.
  • Add an integration test that uploads a NuGet package using multipart/form-data.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
tests/integration/api_packages_nuget_test.go Adds an integration test for multipart/form-data NuGet uploads.
services/context/context_request.go Hardens UploadStream() to avoid nil dereference on multipart parsing and removes dead code.
modules/packages/nuget/metadata.go Maps invalid ZIP reader errors to ErrInvalidArgument (HTTP 400) for .nupkg parsing.
modules/packages/nuget/symbol_extractor.go Maps invalid ZIP reader errors to ErrInvalidArgument (HTTP 400) for .snupkg parsing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>
@silverwind silverwind changed the title Fix NuGet package upload error handling and UploadStream nil pointer Clean up UploadStream and nuget response status codes Apr 1, 2026
ParseMultipartForm always returns an error when it cannot parse the
form, so MultipartForm is never nil after a nil-error return. The
nil guard and application/x-www-form-urlencoded removal were
defensive but unreachable.

Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>
@silverwind silverwind changed the title Clean up UploadStream and nuget response status codes Fix NuGet package upload error handling Apr 1, 2026
Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>
Signed-off-by: wxiaoguang <wxiaoguang@gmail.com>
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 1, 2026
@wxiaoguang
Copy link
Copy Markdown
Contributor

Updated to different messages to help distinguishing different failure cases

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 1, 2026
@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 1, 2026
@silverwind silverwind enabled auto-merge (squash) April 1, 2026 23:23
@silverwind silverwind disabled auto-merge April 1, 2026 23:24
@silverwind silverwind enabled auto-merge (squash) April 1, 2026 23:24
@silverwind silverwind merged commit 2158cf6 into go-gitea:main Apr 1, 2026
26 checks passed
@silverwind silverwind deleted the fix-nuget-upload branch April 1, 2026 23:54
@GiteaBot GiteaBot added this to the 1.26.0 milestone Apr 1, 2026
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 1, 2026
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 2, 2026
* main:
  Fix NuGet package upload error handling (go-gitea#37074)
  Desaturate dark theme background colors (go-gitea#37056)
  Update JS dependencies and misc tweaks (go-gitea#37064)
  Redirect to the only OAuth2 provider when no other login methods and fix various problems (go-gitea#36901)
  Show workflow link (go-gitea#37070)
  Remove leftover `webpackChunkName` comments from codeeditor (go-gitea#37062)
  Update Go dependencies (go-gitea#36781)
  Add webhook name field to improve webhook identification (go-gitea#37025) (go-gitea#37040)
  Upgrade `go-git` to v5.17.2 (go-gitea#37060)
  Replace Monaco with CodeMirror (go-gitea#36764)
  Update Combine method to treat warnings as failures and adjust tests (go-gitea#37048)
  Raise minimum Node.js version to 22.18.0 (go-gitea#37058)
  Update golangci-lint to v2.11.4 (go-gitea#37059)
  Upgrade `golang.org/x/image` to v0.38.0 (go-gitea#37054)
  Increase e2e test timeouts on CI to fix flaky tests (go-gitea#37053)
  Refactor "org teams" page and help new users to "add member" to an org (go-gitea#37051)
silverwind added a commit to silverwind/gitea that referenced this pull request Apr 2, 2026
* origin/main: (192 commits)
  Fix NuGet package upload error handling (go-gitea#37074)
  Desaturate dark theme background colors (go-gitea#37056)
  Update JS dependencies and misc tweaks (go-gitea#37064)
  Redirect to the only OAuth2 provider when no other login methods and fix various problems (go-gitea#36901)
  Show workflow link (go-gitea#37070)
  Remove leftover `webpackChunkName` comments from codeeditor (go-gitea#37062)
  Update Go dependencies (go-gitea#36781)
  Add webhook name field to improve webhook identification (go-gitea#37025) (go-gitea#37040)
  Upgrade `go-git` to v5.17.2 (go-gitea#37060)
  Replace Monaco with CodeMirror (go-gitea#36764)
  Update Combine method to treat warnings as failures and adjust tests (go-gitea#37048)
  Raise minimum Node.js version to 22.18.0 (go-gitea#37058)
  Update golangci-lint to v2.11.4 (go-gitea#37059)
  Upgrade `golang.org/x/image` to v0.38.0 (go-gitea#37054)
  Increase e2e test timeouts on CI to fix flaky tests (go-gitea#37053)
  Refactor "org teams" page and help new users to "add member" to an org (go-gitea#37051)
  Refactor issue sidebar and fix various problems (go-gitea#37045)
  Add tests for pull request's content_version in API (go-gitea#37044)
  Enable concurrent vitest execution (go-gitea#36998)
  Fix theme discovery and Vite dev server in dev mode (go-gitea#37033)
  ...

# Conflicts:
#	templates/user/dashboard/feeds.tmpl
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/go Pull requests that update Go code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants