From a9e3613cfd623ed02fd2fab0eba7e79006bb7cc1 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Fri, 17 Jul 2026 16:23:00 +0800 Subject: [PATCH] fix(quota): only require decoded length for actually framed aws-chunked bodies (#4968) PUT admission since #4928 rejected any request whose Content-Encoding declares aws-chunked but lacks x-amz-decoded-content-length with 400 UnexpectedContent. Whether the body is actually chunk-framed is signalled by a STREAMING-* x-amz-content-sha256, not by the declared encoding: the s3s auth layer only de-frames streaming payloads and already requires the decoded length for them, so a declared-only aws-chunked request (issue #1857 clients) carries an unframed body whose wire Content-Length is the authoritative object size. Admit it against that length; keep failing closed for genuinely framed bodies without a decoded length, and use the decoded length for streaming payloads even when Content-Encoding is absent. Refs: https://github.com/rustfs/backlog/issues/1336 --- crates/e2e_test/src/quota_test.rs | 58 ++++++++++++++++ rustfs/src/app/object_usecase.rs | 109 ++++++++++++++++++++++++++---- 2 files changed, 153 insertions(+), 14 deletions(-) diff --git a/crates/e2e_test/src/quota_test.rs b/crates/e2e_test/src/quota_test.rs index 8f3cb0018..f8f1ced05 100644 --- a/crates/e2e_test/src/quota_test.rs +++ b/crates/e2e_test/src/quota_test.rs @@ -277,6 +277,64 @@ mod integration_tests { Ok(()) } + /// backlog#1336 regression: a PUT that merely declares `Content-Encoding: aws-chunked` + /// (no SigV4 streaming payload, so no `x-amz-decoded-content-length`) carries an unframed + /// body whose wire Content-Length is the real object size. Quota admission must use that + /// length — with and without a hard quota configured — instead of rejecting the request + /// with 400 UnexpectedContent, and an over-quota aws-chunked PUT must still get the quota + /// rejection. + #[tokio::test] + #[serial] + async fn test_quota_admission_aws_chunked_declared_encoding() -> Result<(), Box> { + init_logging(); + if skip_without_awscurl() { + return Ok(()); + } + let env = QuotaTestEnv::new().await?; + env.create_bucket().await?; + + let put_aws_chunked = |key: &'static str, size_bytes: usize| { + env.client + .put_object() + .bucket(&env.bucket_name) + .key(key) + .content_encoding("aws-chunked") + .body(aws_sdk_s3::primitives::ByteStream::from(vec![0u8; size_bytes])) + .send() + }; + + // No quota configured: the declared aws-chunked PUT must be admitted. + put_aws_chunked("no-quota.bin", 512) + .await + .expect("declared aws-chunked PUT without quota must succeed"); + assert!(env.object_exists("no-quota.bin").await?); + + // Hard quota configured: a within-quota declared aws-chunked PUT is admitted + // against its wire Content-Length. + env.set_bucket_quota(4 * 1024).await?; + put_aws_chunked("within-quota.bin", 1024) + .await + .expect("declared aws-chunked PUT within quota must succeed"); + assert!(env.object_exists("within-quota.bin").await?); + + // An over-quota declared aws-chunked PUT is rejected by quota admission — + // not with UnexpectedContent. + let err = put_aws_chunked("over-quota.bin", 16 * 1024) + .await + .expect_err("declared aws-chunked PUT over quota must be rejected"); + let err_debug = format!("{err:?}"); + assert!( + !err_debug.contains("UnexpectedContent"), + "over-quota rejection must be the quota error, not UnexpectedContent: {err_debug}" + ); + assert!(!env.object_exists("over-quota.bin").await?); + + env.clear_bucket_quota().await?; + env.cleanup_bucket().await?; + + Ok(()) + } + #[tokio::test] #[serial] async fn test_quota_update_and_clear() -> Result<(), Box> { diff --git a/rustfs/src/app/object_usecase.rs b/rustfs/src/app/object_usecase.rs index 0e55fba5a..d1ffb505d 100644 --- a/rustfs/src/app/object_usecase.rs +++ b/rustfs/src/app/object_usecase.rs @@ -133,13 +133,13 @@ use rustfs_utils::http::{ AMZ_BUCKET_REPLICATION_STATUS, AMZ_CHECKSUM_MODE, AMZ_CHECKSUM_TYPE, AMZ_WEBSITE_REDIRECT_LOCATION, CONTENT_TYPE, SUFFIX_ACTUAL_SIZE, SUFFIX_COMPRESSION, SUFFIX_COMPRESSION_SIZE, SUFFIX_REPLICATION_STATUS, SUFFIX_REPLICATION_TIMESTAMP, headers::{ - AMZ_DECODED_CONTENT_LENGTH, AMZ_MINIO_SNOWBALL_IGNORE_DIRS, AMZ_MINIO_SNOWBALL_IGNORE_ERRORS, AMZ_MINIO_SNOWBALL_PREFIX, - AMZ_OBJECT_LOCK_LEGAL_HOLD, AMZ_OBJECT_LOCK_LEGAL_HOLD_LOWER, AMZ_OBJECT_LOCK_MODE, AMZ_OBJECT_LOCK_MODE_LOWER, - AMZ_OBJECT_LOCK_RETAIN_UNTIL_DATE, AMZ_OBJECT_LOCK_RETAIN_UNTIL_DATE_LOWER, AMZ_OBJECT_TAGGING, AMZ_RESTORE_EXPIRY_DAYS, - AMZ_RESTORE_REQUEST_DATE, AMZ_RUSTFS_SNOWBALL_IGNORE_DIRS, AMZ_RUSTFS_SNOWBALL_IGNORE_ERRORS, AMZ_RUSTFS_SNOWBALL_PREFIX, - AMZ_SERVER_SIDE_ENCRYPTION, AMZ_SERVER_SIDE_ENCRYPTION_CUSTOMER_ALGORITHM, AMZ_SERVER_SIDE_ENCRYPTION_KMS_ID, - AMZ_SNOWBALL_EXTRACT, AMZ_SNOWBALL_IGNORE_DIRS, AMZ_SNOWBALL_IGNORE_ERRORS, AMZ_SNOWBALL_PREFIX, AMZ_STORAGE_CLASS, - AMZ_TAG_COUNT, + AMZ_CONTENT_SHA256, AMZ_DECODED_CONTENT_LENGTH, AMZ_MINIO_SNOWBALL_IGNORE_DIRS, AMZ_MINIO_SNOWBALL_IGNORE_ERRORS, + AMZ_MINIO_SNOWBALL_PREFIX, AMZ_OBJECT_LOCK_LEGAL_HOLD, AMZ_OBJECT_LOCK_LEGAL_HOLD_LOWER, AMZ_OBJECT_LOCK_MODE, + AMZ_OBJECT_LOCK_MODE_LOWER, AMZ_OBJECT_LOCK_RETAIN_UNTIL_DATE, AMZ_OBJECT_LOCK_RETAIN_UNTIL_DATE_LOWER, + AMZ_OBJECT_TAGGING, AMZ_RESTORE_EXPIRY_DAYS, AMZ_RESTORE_REQUEST_DATE, AMZ_RUSTFS_SNOWBALL_IGNORE_DIRS, + AMZ_RUSTFS_SNOWBALL_IGNORE_ERRORS, AMZ_RUSTFS_SNOWBALL_PREFIX, AMZ_SERVER_SIDE_ENCRYPTION, + AMZ_SERVER_SIDE_ENCRYPTION_CUSTOMER_ALGORITHM, AMZ_SERVER_SIDE_ENCRYPTION_KMS_ID, AMZ_SNOWBALL_EXTRACT, + AMZ_SNOWBALL_IGNORE_DIRS, AMZ_SNOWBALL_IGNORE_ERRORS, AMZ_SNOWBALL_PREFIX, AMZ_STORAGE_CLASS, AMZ_TAG_COUNT, }, insert_str, remove_str, }; @@ -304,11 +304,16 @@ fn range_to_http_range_spec(range: Range) -> S3Result { /// Resolve the authoritative object length that bucket-quota admission (and downstream sizing) must use. /// -/// For an `aws-chunked` upload the request body is wrapped in chunk framing, so the wire `Content-Length` overcounts the actual object and must never be admitted; the decoded length (`x-amz-decoded-content-length`) is the only authoritative size and is therefore required — a chunked request without it is rejected rather than silently falling back to the framed wire length. For a non-chunked request the raw `Content-Length` is authoritative, with an explicit decoded length as the fallback when the S3 layer only surfaced the latter. A negative or otherwise unknown length is rejected so it can never be reinterpreted as an enormous unsigned size downstream. +/// `Content-Encoding: aws-chunked` alone only *declares* the encoding; whether the body actually arrived chunk-framed is signalled by a `STREAMING-*` `x-amz-content-sha256`, and the S3 auth layer both requires `x-amz-decoded-content-length` for those requests and hands the body down already de-framed. So when a decoded length is present it is authoritative (the wire `Content-Length` counts chunk framing and would overcount); a framed body without a decoded length is rejected rather than falling back to the framed wire length. A declared-only aws-chunked request (issue #1857 clients) carries an unframed body, so its wire `Content-Length` is the authoritative size, exactly as for a plain PUT. A negative or otherwise unknown length is rejected so it can never be reinterpreted as an enormous unsigned size downstream. fn resolve_put_object_authoritative_size(headers: &HeaderMap, content_length: Option) -> S3Result { let decoded_content_length = decoded_content_length_from_headers(headers)?; - let size = match (request_uses_aws_chunked(headers), decoded_content_length, content_length) { + let aws_chunked = request_uses_aws_chunked(headers) || request_body_is_aws_chunked_framed(headers); + let size = match (aws_chunked, decoded_content_length, content_length) { (true, Some(decoded), _) => decoded, + // Declared aws-chunked without a streaming payload: the body is not framed (the auth + // layer only de-frames STREAMING-* payloads, which always carry a decoded length), so + // the wire Content-Length is the real object size. + (true, None, Some(raw)) if !request_body_is_aws_chunked_framed(headers) => raw, (true, None, _) => return Err(s3_error!(UnexpectedContent)), (false, _, Some(raw)) => raw, (false, Some(decoded), None) => decoded, @@ -322,6 +327,17 @@ fn resolve_put_object_authoritative_size(headers: &HeaderMap, content_length: Op Ok(size) } +/// True when the request body actually arrived chunk-framed on the wire, i.e. the payload was +/// signed as a SigV4 streaming upload (`x-amz-content-sha256: STREAMING-*`). This is the only +/// case in which the auth layer de-frames the body; `Content-Encoding: aws-chunked` without a +/// streaming payload is just a declared encoding over an unframed body. +fn request_body_is_aws_chunked_framed(headers: &HeaderMap) -> bool { + headers + .get(AMZ_CONTENT_SHA256) + .and_then(|value| value.to_str().ok()) + .is_some_and(|value| value.len() >= 10 && value[..10].eq_ignore_ascii_case("STREAMING-")) +} + /// Map a bucket-quota checker outcome onto the S3 admission result. /// /// Hard is the only supported quota type, so a checker fault (bucket-config read, config parse, or usage lookup) must fail closed rather than admit the write: allowing it would silently bypass a configured hard quota. The no-quota happy path never reaches the error arm — `QuotaChecker::check_quota` returns `Ok(allowed)` via the zero-extra-I/O fast path when no quota is configured, so failing closed here cannot penalise buckets without a quota. A fault surfaces as a retryable `ServiceUnavailable` and is counted; the client-facing message stays generic so internal config/usage details are not leaked. @@ -10625,6 +10641,8 @@ mod tests { // https://github.com/rustfs/backlog/issues/1311 — bucket-quota admission must run against the authoritative // decoded/plain object length, never the aws-chunked wire Content-Length, and must reject negative/unknown lengths. + // https://github.com/rustfs/backlog/issues/1336 — but Content-Encoding: aws-chunked alone is only a declared + // encoding: without a STREAMING-* payload the body is unframed and the wire Content-Length is authoritative. fn aws_chunked_headers(decoded_len: Option<&str>) -> HeaderMap { let mut headers = HeaderMap::new(); headers.insert(http::header::CONTENT_ENCODING, HeaderValue::from_static("aws-chunked")); @@ -10637,23 +10655,86 @@ mod tests { headers } + fn streaming_headers(decoded_len: Option<&str>) -> HeaderMap { + let mut headers = aws_chunked_headers(decoded_len); + headers.insert( + HeaderName::from_bytes(AMZ_CONTENT_SHA256.as_bytes()).unwrap(), + HeaderValue::from_static("STREAMING-AWS4-HMAC-SHA256-PAYLOAD"), + ); + headers + } + #[test] fn authoritative_size_prefers_aws_chunked_decoded_over_wire_content_length() { // Wire Content-Length (chunk framing) differs from the decoded object length; the decoded length wins. - let headers = aws_chunked_headers(Some("1000")); + let headers = streaming_headers(Some("1000")); let size = resolve_put_object_authoritative_size(&headers, Some(1088)).expect("decoded length is authoritative"); assert_eq!( size, 1000, "aws-chunked admission must use the decoded object length, not the framed wire length" ); + + // A declared-only aws-chunked request that still carries a decoded length behaves the same. + let headers = aws_chunked_headers(Some("1000")); + let size = resolve_put_object_authoritative_size(&headers, Some(1088)).expect("decoded length is authoritative"); + assert_eq!(size, 1000); } #[test] - fn authoritative_size_rejects_aws_chunked_without_decoded_length() { - // A chunked upload without x-amz-decoded-content-length has no authoritative size; the wire length must NOT be a fallback. - let headers = aws_chunked_headers(None); + fn authoritative_size_streaming_without_content_encoding_uses_decoded_length() { + // A streaming payload signals framing via x-amz-content-sha256 alone; Content-Encoding is optional. + let mut headers = HeaderMap::new(); + headers.insert( + HeaderName::from_bytes(AMZ_CONTENT_SHA256.as_bytes()).unwrap(), + HeaderValue::from_static("STREAMING-UNSIGNED-PAYLOAD-TRAILER"), + ); + headers.insert( + HeaderName::from_bytes(AMZ_DECODED_CONTENT_LENGTH.as_bytes()).unwrap(), + HeaderValue::from_static("1000"), + ); + let size = resolve_put_object_authoritative_size(&headers, Some(1088)).expect("decoded length is authoritative"); + assert_eq!( + size, 1000, + "a streaming payload without Content-Encoding must still use the decoded length" + ); + } + + #[test] + fn authoritative_size_rejects_framed_body_without_decoded_length() { + // A genuinely framed upload without x-amz-decoded-content-length has no authoritative size; + // the framed wire length must NOT be a fallback. + let headers = streaming_headers(None); let err = resolve_put_object_authoritative_size(&headers, Some(1088)) - .expect_err("chunked upload without decoded length must be rejected"); + .expect_err("framed upload without decoded length must be rejected"); + assert_eq!(err.code(), &S3ErrorCode::UnexpectedContent); + + // ... even when the wire Content-Length is also absent. + let err = + resolve_put_object_authoritative_size(&headers, None).expect_err("framed upload without any length must be rejected"); + assert_eq!(err.code(), &S3ErrorCode::UnexpectedContent); + } + + #[test] + fn authoritative_size_declared_aws_chunked_without_streaming_uses_wire_content_length() { + // backlog#1336: an SDK PUT that merely declares Content-Encoding: aws-chunked (issue #1857 + // clients) has an unframed body and no decoded length; the wire Content-Length is the real + // object size and the request must be admitted, not rejected with UnexpectedContent. + let headers = aws_chunked_headers(None); + let size = resolve_put_object_authoritative_size(&headers, Some(1088)) + .expect("declared-only aws-chunked must fall back to the wire Content-Length"); + assert_eq!(size, 1088); + + // Same for a combined declared encoding (aws-chunked,gzip). + let mut headers = HeaderMap::new(); + headers.insert(http::header::CONTENT_ENCODING, HeaderValue::from_static("aws-chunked,gzip")); + let size = resolve_put_object_authoritative_size(&headers, Some(2048)) + .expect("declared-only aws-chunked,gzip must fall back to the wire Content-Length"); + assert_eq!(size, 2048); + + // Without any length information it is still rejected. + let headers = aws_chunked_headers(None); + let err = resolve_put_object_authoritative_size(&headers, None) + .expect_err("declared-only aws-chunked with no length at all must be rejected"); assert_eq!(err.code(), &S3ErrorCode::UnexpectedContent); }