mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-20 11:32:19 +00:00
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
This commit is contained in:
@@ -277,6 +277,64 @@ mod integration_tests {
|
|||||||
Ok(())
|
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<dyn std::error::Error + Send + Sync>> {
|
||||||
|
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]
|
#[tokio::test]
|
||||||
#[serial]
|
#[serial]
|
||||||
async fn test_quota_update_and_clear() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
|
async fn test_quota_update_and_clear() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
|
||||||
|
|||||||
@@ -133,13 +133,13 @@ use rustfs_utils::http::{
|
|||||||
AMZ_BUCKET_REPLICATION_STATUS, AMZ_CHECKSUM_MODE, AMZ_CHECKSUM_TYPE, AMZ_WEBSITE_REDIRECT_LOCATION, CONTENT_TYPE,
|
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,
|
SUFFIX_ACTUAL_SIZE, SUFFIX_COMPRESSION, SUFFIX_COMPRESSION_SIZE, SUFFIX_REPLICATION_STATUS, SUFFIX_REPLICATION_TIMESTAMP,
|
||||||
headers::{
|
headers::{
|
||||||
AMZ_DECODED_CONTENT_LENGTH, AMZ_MINIO_SNOWBALL_IGNORE_DIRS, AMZ_MINIO_SNOWBALL_IGNORE_ERRORS, AMZ_MINIO_SNOWBALL_PREFIX,
|
AMZ_CONTENT_SHA256, AMZ_DECODED_CONTENT_LENGTH, AMZ_MINIO_SNOWBALL_IGNORE_DIRS, AMZ_MINIO_SNOWBALL_IGNORE_ERRORS,
|
||||||
AMZ_OBJECT_LOCK_LEGAL_HOLD, AMZ_OBJECT_LOCK_LEGAL_HOLD_LOWER, AMZ_OBJECT_LOCK_MODE, AMZ_OBJECT_LOCK_MODE_LOWER,
|
AMZ_MINIO_SNOWBALL_PREFIX, AMZ_OBJECT_LOCK_LEGAL_HOLD, AMZ_OBJECT_LOCK_LEGAL_HOLD_LOWER, AMZ_OBJECT_LOCK_MODE,
|
||||||
AMZ_OBJECT_LOCK_RETAIN_UNTIL_DATE, AMZ_OBJECT_LOCK_RETAIN_UNTIL_DATE_LOWER, AMZ_OBJECT_TAGGING, AMZ_RESTORE_EXPIRY_DAYS,
|
AMZ_OBJECT_LOCK_MODE_LOWER, AMZ_OBJECT_LOCK_RETAIN_UNTIL_DATE, AMZ_OBJECT_LOCK_RETAIN_UNTIL_DATE_LOWER,
|
||||||
AMZ_RESTORE_REQUEST_DATE, AMZ_RUSTFS_SNOWBALL_IGNORE_DIRS, AMZ_RUSTFS_SNOWBALL_IGNORE_ERRORS, AMZ_RUSTFS_SNOWBALL_PREFIX,
|
AMZ_OBJECT_TAGGING, AMZ_RESTORE_EXPIRY_DAYS, AMZ_RESTORE_REQUEST_DATE, AMZ_RUSTFS_SNOWBALL_IGNORE_DIRS,
|
||||||
AMZ_SERVER_SIDE_ENCRYPTION, AMZ_SERVER_SIDE_ENCRYPTION_CUSTOMER_ALGORITHM, AMZ_SERVER_SIDE_ENCRYPTION_KMS_ID,
|
AMZ_RUSTFS_SNOWBALL_IGNORE_ERRORS, AMZ_RUSTFS_SNOWBALL_PREFIX, AMZ_SERVER_SIDE_ENCRYPTION,
|
||||||
AMZ_SNOWBALL_EXTRACT, AMZ_SNOWBALL_IGNORE_DIRS, AMZ_SNOWBALL_IGNORE_ERRORS, AMZ_SNOWBALL_PREFIX, AMZ_STORAGE_CLASS,
|
AMZ_SERVER_SIDE_ENCRYPTION_CUSTOMER_ALGORITHM, AMZ_SERVER_SIDE_ENCRYPTION_KMS_ID, AMZ_SNOWBALL_EXTRACT,
|
||||||
AMZ_TAG_COUNT,
|
AMZ_SNOWBALL_IGNORE_DIRS, AMZ_SNOWBALL_IGNORE_ERRORS, AMZ_SNOWBALL_PREFIX, AMZ_STORAGE_CLASS, AMZ_TAG_COUNT,
|
||||||
},
|
},
|
||||||
insert_str, remove_str,
|
insert_str, remove_str,
|
||||||
};
|
};
|
||||||
@@ -304,11 +304,16 @@ fn range_to_http_range_spec(range: Range) -> S3Result<HTTPRangeSpec> {
|
|||||||
|
|
||||||
/// Resolve the authoritative object length that bucket-quota admission (and downstream sizing) must use.
|
/// 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<i64>) -> S3Result<i64> {
|
fn resolve_put_object_authoritative_size(headers: &HeaderMap, content_length: Option<i64>) -> S3Result<i64> {
|
||||||
let decoded_content_length = decoded_content_length_from_headers(headers)?;
|
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,
|
(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)),
|
(true, None, _) => return Err(s3_error!(UnexpectedContent)),
|
||||||
(false, _, Some(raw)) => raw,
|
(false, _, Some(raw)) => raw,
|
||||||
(false, Some(decoded), None) => decoded,
|
(false, Some(decoded), None) => decoded,
|
||||||
@@ -322,6 +327,17 @@ fn resolve_put_object_authoritative_size(headers: &HeaderMap, content_length: Op
|
|||||||
Ok(size)
|
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.
|
/// 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.
|
/// 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
|
// 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.
|
// 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 {
|
fn aws_chunked_headers(decoded_len: Option<&str>) -> HeaderMap {
|
||||||
let mut headers = HeaderMap::new();
|
let mut headers = HeaderMap::new();
|
||||||
headers.insert(http::header::CONTENT_ENCODING, HeaderValue::from_static("aws-chunked"));
|
headers.insert(http::header::CONTENT_ENCODING, HeaderValue::from_static("aws-chunked"));
|
||||||
@@ -10637,23 +10655,86 @@ mod tests {
|
|||||||
headers
|
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]
|
#[test]
|
||||||
fn authoritative_size_prefers_aws_chunked_decoded_over_wire_content_length() {
|
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.
|
// 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");
|
let size = resolve_put_object_authoritative_size(&headers, Some(1088)).expect("decoded length is authoritative");
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
size, 1000,
|
size, 1000,
|
||||||
"aws-chunked admission must use the decoded object length, not the framed wire length"
|
"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]
|
#[test]
|
||||||
fn authoritative_size_rejects_aws_chunked_without_decoded_length() {
|
fn authoritative_size_streaming_without_content_encoding_uses_decoded_length() {
|
||||||
// A chunked upload without x-amz-decoded-content-length has no authoritative size; the wire length must NOT be a fallback.
|
// A streaming payload signals framing via x-amz-content-sha256 alone; Content-Encoding is optional.
|
||||||
let headers = aws_chunked_headers(None);
|
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))
|
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);
|
assert_eq!(err.code(), &S3ErrorCode::UnexpectedContent);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user