Skip to content

fix(drivers/s3): sign content-type for direct uploads - #3152

Open
mumingluan wants to merge 2 commits into
OpenListTeam:mainfrom
mumingluan:codex/s3-direct-upload-content-type
Open

mumingluan wants to merge 2 commits into
OpenListTeam:mainfrom
mumingluan:codex/s3-direct-upload-content-type

Conversation

@mumingluan

@mumingluan mumingluan commented Sep 30, 2026 •

Copy link
Copy Markdown

Summary / 摘要

S3 browser uploads can fail with 403 AccessDenied on Ceph RGW when the request carries an unsigned Content-Type. The frontend sends file.type || "application/octet-stream" in a new optional content_type field on the direct-upload information request.

The backend validates the supplied media type, passes it to the driver through a typed context key, signs it in the S3 PutObject URL, and returns the same value in the existing upload headers. Missing types fall back to application/octet-stream, preserving compatibility with existing clients.

  • This PR has breaking changes.
  • This PR changes public API, config, storage format, or migration behavior.
    Adds an optional request field and populates the existing optional response headers.
  • This PR requires corresponding changes in related repositories.
    A companion frontend change supplies the browser's MIME type.

Related repository PRs / 关联仓库 PR:

Testing / 测试

  • go test ./drivers/s3 ./internal/conf -count=1 passed.
  • The handles package compiles; its full test run fails in TestIsSearchNodeAccessible/outside_base_path. The same failure was reproduced at the original branch HEAD.
  • Frontend Prettier and both repositories' git diff --check passed.
  • Frontend TypeScript checking reports seven errors in unrelated files. Running it at the original frontend HEAD produced identical diagnostics.
  • Executed the revised frontend upload function with an isolated Node File/XHR harness: a .apk carrying text/plain and a file with an empty type send the expected content_type and matching upload header.
  • Ran the revised S3 driver against Rainyun and Cloudflare R2. A .apk using the client-supplied text/plain and a .jpg using application/pdf uploaded successfully. Missing MIME type used application/octet-stream, and an 80 MiB Rainyun upload succeeded. Object sizes were verified and temporary objects were deleted.

Checklist / 检查清单

  • Read the contribution instructions and PR templates.
  • Formatted the changed Go and TypeScript code.
  • Human review of this revision is complete.

AI Disclosure / AI 使用声明

  • This revision includes AI-assisted content.

  • Tool: Codex.

  • Scope: implementation, validation, and PR wording.

  • The human collaborator requested this revision in response to maintainer feedback. Codex performed the checks described above; the human collaborator reviewed the proposed changes and approved submission.

  • AI-assisted commits include the required Co-authored-by attribution.

- Include the inferred MIME type in presigned PutObject requests
- Return the signed Content-Type header for frontend uploads

Co-authored-by: Codex <267193182+codex@users.noreply.github.com>
@xrgzs xrgzs changed the title fix(s3): sign content type for direct uploads fix(drivers/s3): sign content type for direct uploads Oct 3, 2026
@xrgzs xrgzs changed the title fix(drivers/s3): sign content type for direct uploads fix(drivers/s3): sign content-type for direct uploads Oct 3, 2026
@xrgzs

xrgzs commented Oct 3, 2026

Copy link
Copy Markdown
Member

@mumingluan Have you tested other S3 backends?

@mumingluan

Copy link
Copy Markdown
Author

@mumingluan Have you tested other S3 backends?

Only Ceph encountered this issue.
Other backends can work properly with and without the fix.

Comment thread drivers/s3/driver.go Outdated
return nil, errs.NotImplement
}
path := getKey(stdpath.Join(dstDir.GetPath(), fileName), false)
contentType := utils.GetMimeType(fileName)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think getting Content-Type from the filename is a good idea. The browser's implementation may differ from the Go backend.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for the feedback. Updated this PR to use the browser-provided MIME type, with the companion frontend change in OpenListTeam/OpenList-Frontend#692.

The frontend sends file.type || "application/octet-stream" as content_type. The backend validates and signs that value, then returns the matching upload header. Existing clients that omit the field use application/octet-stream.

I verified the revised driver against Rainyun/Ceph and Cloudflare R2 with supplied MIME types that differ from the filename extensions, including a successful 80 MiB Rainyun upload using the fallback. Object sizes were verified and the temporary objects were deleted.

- Accept and validate the client-provided direct upload media type
- Pass the type to the S3 signer through a typed context key
- Default missing types to application/octet-stream

Co-authored-by: Codex <267193182+codex@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants