Skip to content

fix(cp): avoid guessed content type for multipart uploads#203

Merged
overtrue merged 1 commit into
mainfrom
overtrue/fix-multipart-upload-191
May 16, 2026
Merged

fix(cp): avoid guessed content type for multipart uploads#203
overtrue merged 1 commit into
mainfrom
overtrue/fix-multipart-upload-191

Conversation

@overtrue
Copy link
Copy Markdown
Contributor

Related issue

Closes #191

Background

Large local-to-remote rc cp uploads use multipart upload. For files such as a generated .txt file, the CLI guessed text/plain from the extension and forwarded that value into the multipart create request. Some RustFS multipart create paths reject that inferred content type, while small single-part uploads still work.

Solution

Skip automatically guessed content types for multipart-sized uploads. Explicit --content-type values are still honored, and small single-part uploads keep the existing MIME guessing behavior.

Tests

  • cargo fmt --all --check
  • cargo test -p rustfs-cli commands::cp::tests::test_select_upload_content_type --lib
  • cargo clippy --workspace -- -D warnings
  • cargo test --workspace

Copilot AI review requested due to automatic review settings May 16, 2026 05:46
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

This PR addresses failures in rc cp multipart uploads caused by forwarding an automatically guessed Content-Type (e.g., text/plain) into the multipart create request, which some RustFS multipart paths reject. The change keeps MIME guessing for small single-part uploads, while ensuring multipart-sized uploads only send an explicit --content-type when provided.

Changes:

  • Move content-type selection to occur after file size is known, enabling size-based selection.
  • Add select_upload_content_type helper to skip guessed MIME types for multipart-sized uploads while honoring explicit --content-type.
  • Add unit tests covering the helper’s selection behavior.
Comments suppressed due to low confidence (2)

crates/cli/src/commands/cp.rs:351

  • select_upload_content_type uses file_size > MULTIPART_THRESHOLD to decide when to skip guessed MIME types, but MULTIPART_THRESHOLD is documented as “files at least this size use multipart upload” and upload_file logs/chooses the multipart progress bar on file_size >= MULTIPART_THRESHOLD. This boundary mismatch means a file exactly equal to the threshold will still forward the guessed content type even though it’s treated as multipart elsewhere. Align the comparison (and ideally share the same predicate as the actual multipart decision) so the behavior is consistent at the threshold.
fn select_upload_content_type<'a>(
    explicit_type: Option<&'a str>,
    guessed_type: Option<&'a str>,
    file_size: u64,
) -> Option<&'a str> {
    if file_size > MULTIPART_THRESHOLD {
        explicit_type
    } else {
        explicit_type.or(guessed_type)
    }

crates/cli/src/commands/cp.rs:895

  • The new unit tests cover < threshold and > threshold, but don’t assert behavior for file_size == MULTIPART_THRESHOLD, which is currently ambiguous/inconsistent with the constant’s doc comment and the progress-bar condition. Add a boundary test for the exact-threshold size once the intended semantics (>= vs >) are clarified so regressions at the cutoff are caught.
    #[test]
    fn test_select_upload_content_type_uses_guess_for_small_files() {
        let selected =
            select_upload_content_type(None, Some("text/plain"), MULTIPART_THRESHOLD - 1);

        assert_eq!(selected, Some("text/plain"));
    }

    #[test]
    fn test_select_upload_content_type_skips_guess_for_multipart_files() {
        let selected =
            select_upload_content_type(None, Some("text/plain"), MULTIPART_THRESHOLD + 1);

        assert_eq!(selected, None);
    }

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

Comment thread crates/cli/src/commands/cp.rs
@overtrue overtrue force-pushed the overtrue/fix-multipart-upload-191 branch from 92041ee to fa61632 Compare May 16, 2026 05:52
@overtrue overtrue force-pushed the overtrue/fix-multipart-upload-191 branch from fa61632 to c44a04d Compare May 16, 2026 05:53
@overtrue overtrue merged commit 1df8086 into main May 16, 2026
15 checks passed
@overtrue overtrue deleted the overtrue/fix-multipart-upload-191 branch May 16, 2026 13:17
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.

Network error: create multipart upload: service error

2 participants