mirror of
https://github.com/rustfs/rustfs.git
synced 2026-07-27 08:38:58 +00:00
fix(s3): normalize root double-slash ListBuckets requests (#4336)
test(s3): MinIO parity for double-slash ListBuckets, invalid copy-source date, region-aware CreateBucket - Add DoubleSlashListBucketsCompatLayer: rewrite `GET //` to `GET /` so it routes to ListBuckets, matching MinIO browser-client behavior (#601) - Add e2e regression coverage for invalid copy-source conditional date headers (#618) and region-aware MakeBucket(us-east-1) (#629) Refs rustfs/backlog#601 #618 #629
This commit is contained in:
@@ -0,0 +1,142 @@
|
|||||||
|
// Copyright 2024 RustFS Team
|
||||||
|
//
|
||||||
|
// Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
|
// you may not use this file except in compliance with the License.
|
||||||
|
// You may obtain a copy of the License at
|
||||||
|
//
|
||||||
|
// http://www.apache.org/licenses/LICENSE-2.0
|
||||||
|
//
|
||||||
|
// Unless required by applicable law or agreed to in writing, software
|
||||||
|
// distributed under the License is distributed on an "AS IS" BASIS,
|
||||||
|
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||||
|
// See the License for the specific language governing permissions and
|
||||||
|
// limitations under the License.
|
||||||
|
|
||||||
|
//! Regression coverage for rustfs/backlog#618 item 8: `CopyObject` with an
|
||||||
|
//! unparsable `x-amz-copy-source-if-unmodified-since` value.
|
||||||
|
//!
|
||||||
|
//! `minio-js` `CopyConditions.setUnmodified(new Date(undefined))` serialises the
|
||||||
|
//! header value as the literal string `"Invalid Date"` (via `Date.toUTCString()`)
|
||||||
|
//! and signs it. MinIO parses this header from the raw request and *ignores* it
|
||||||
|
//! when it fails to parse (`amztime.ParseHeader` error is swallowed), so the copy
|
||||||
|
//! proceeds.
|
||||||
|
//!
|
||||||
|
//! RustFS routes S3 through the `s3s` crate, which models the copy-source
|
||||||
|
//! conditional-date headers as typed `Timestamp` fields and rejects an
|
||||||
|
//! unparsable value with `400 InvalidArgument` during request deserialization —
|
||||||
|
//! this happens *after* SigV4 verification but *before* the RustFS `CopyObject`
|
||||||
|
//! handler runs. Because the header is part of the client's `SignedHeaders`, the
|
||||||
|
//! value cannot be stripped or rewritten by a pre-`s3s` layer without breaking the
|
||||||
|
//! signature, and `s3s` exposes no hook between auth and typed deserialization.
|
||||||
|
//! Matching MinIO's tolerant behavior therefore requires a change in `s3s`
|
||||||
|
//! (lenient parsing of conditional-date headers). See the PR description for
|
||||||
|
//! details.
|
||||||
|
//!
|
||||||
|
//! This test pins the two ends of the current behavior:
|
||||||
|
//! * a *valid* HTTP-date copy-source conditional header is accepted and the copy
|
||||||
|
//! succeeds (the common, real-SDK path), and
|
||||||
|
//! * an *invalid* date value is rejected with `400 InvalidArgument` at the `s3s`
|
||||||
|
//! layer (the documented gap). When `s3s` gains lenient parsing, the second
|
||||||
|
//! assertion should be flipped to expect a successful copy.
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use crate::common::{RustFSTestEnvironment, init_logging, local_http_client};
|
||||||
|
use aws_sdk_s3::primitives::ByteStream;
|
||||||
|
use http::header::HOST;
|
||||||
|
use rustfs_signer::constants::UNSIGNED_PAYLOAD;
|
||||||
|
use rustfs_signer::sign_v4;
|
||||||
|
use s3s::Body;
|
||||||
|
use serial_test::serial;
|
||||||
|
use std::error::Error;
|
||||||
|
|
||||||
|
/// Signed raw `PUT` copy request with an explicit copy-source conditional
|
||||||
|
/// header. The header is included in the SigV4 signature (as real SDKs do).
|
||||||
|
async fn signed_copy(
|
||||||
|
base_url: &str,
|
||||||
|
dst_path: &str,
|
||||||
|
copy_source: &str,
|
||||||
|
if_unmodified_since: &str,
|
||||||
|
access_key: &str,
|
||||||
|
secret_key: &str,
|
||||||
|
) -> Result<(reqwest::StatusCode, String), Box<dyn Error + Send + Sync>> {
|
||||||
|
let url = format!("{base_url}{dst_path}");
|
||||||
|
let uri = url.parse::<http::Uri>()?;
|
||||||
|
let authority = uri.authority().ok_or("missing authority")?.to_string();
|
||||||
|
let request = http::Request::builder()
|
||||||
|
.method(http::Method::PUT)
|
||||||
|
.uri(uri)
|
||||||
|
.header(HOST, authority)
|
||||||
|
.header("x-amz-content-sha256", UNSIGNED_PAYLOAD)
|
||||||
|
.header("x-amz-copy-source", copy_source)
|
||||||
|
.header("x-amz-copy-source-if-unmodified-since", if_unmodified_since);
|
||||||
|
let signed = sign_v4(request.body(Body::empty())?, 0, access_key, secret_key, "", "us-east-1");
|
||||||
|
|
||||||
|
let client = local_http_client();
|
||||||
|
let mut rb = client.request(reqwest::Method::PUT, url);
|
||||||
|
for (name, value) in signed.headers() {
|
||||||
|
rb = rb.header(name, value);
|
||||||
|
}
|
||||||
|
let resp = rb.send().await?;
|
||||||
|
let status = resp.status();
|
||||||
|
let body = resp.text().await?;
|
||||||
|
Ok((status, body))
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
#[serial]
|
||||||
|
async fn test_copy_source_if_unmodified_since_valid_and_invalid() -> Result<(), Box<dyn Error + Send + Sync>> {
|
||||||
|
init_logging();
|
||||||
|
let mut env = RustFSTestEnvironment::new().await?;
|
||||||
|
env.start_rustfs_server(vec![]).await?;
|
||||||
|
|
||||||
|
let client = env.create_s3_client();
|
||||||
|
let bucket = "copy-cond-bucket";
|
||||||
|
client.create_bucket().bucket(bucket).send().await?;
|
||||||
|
client
|
||||||
|
.put_object()
|
||||||
|
.bucket(bucket)
|
||||||
|
.key("src.txt")
|
||||||
|
.body(ByteStream::from_static(b"hello"))
|
||||||
|
.send()
|
||||||
|
.await?;
|
||||||
|
|
||||||
|
let copy_source = format!("/{bucket}/src.txt");
|
||||||
|
|
||||||
|
// A valid future HTTP-date is accepted by s3s and the copy succeeds.
|
||||||
|
let (status, body) = signed_copy(
|
||||||
|
&env.url,
|
||||||
|
&format!("/{bucket}/dst-valid.txt"),
|
||||||
|
©_source,
|
||||||
|
"Wed, 01 Jan 2031 00:00:00 GMT",
|
||||||
|
&env.access_key,
|
||||||
|
&env.secret_key,
|
||||||
|
)
|
||||||
|
.await?;
|
||||||
|
assert_eq!(status, reqwest::StatusCode::OK, "valid copy-source date must succeed: {body}");
|
||||||
|
assert!(body.contains("CopyObjectResult"), "expected CopyObjectResult XML: {body}");
|
||||||
|
|
||||||
|
// An unparsable date ("Invalid Date") is currently rejected by s3s at the
|
||||||
|
// deserialization layer with 400 InvalidArgument. This documents the s3s
|
||||||
|
// limitation described in backlog#618 item 8. When s3s parses these headers
|
||||||
|
// leniently (matching MinIO), this should become a successful copy.
|
||||||
|
let (status, body) = signed_copy(
|
||||||
|
&env.url,
|
||||||
|
&format!("/{bucket}/dst-invalid.txt"),
|
||||||
|
©_source,
|
||||||
|
"Invalid Date",
|
||||||
|
&env.access_key,
|
||||||
|
&env.secret_key,
|
||||||
|
)
|
||||||
|
.await?;
|
||||||
|
assert_eq!(
|
||||||
|
status,
|
||||||
|
reqwest::StatusCode::BAD_REQUEST,
|
||||||
|
"invalid copy-source date currently rejected by s3s (backlog#618): {body}"
|
||||||
|
);
|
||||||
|
assert!(body.contains("InvalidArgument"), "expected InvalidArgument for invalid date: {body}");
|
||||||
|
|
||||||
|
env.stop_server();
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,82 @@
|
|||||||
|
// Copyright 2024 RustFS Team
|
||||||
|
//
|
||||||
|
// Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
|
// you may not use this file except in compliance with the License.
|
||||||
|
// You may obtain a copy of the License at
|
||||||
|
//
|
||||||
|
// http://www.apache.org/licenses/LICENSE-2.0
|
||||||
|
//
|
||||||
|
// Unless required by applicable law or agreed to in writing, software
|
||||||
|
// distributed under the License is distributed on an "AS IS" BASIS,
|
||||||
|
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||||
|
// See the License for the specific language governing permissions and
|
||||||
|
// limitations under the License.
|
||||||
|
|
||||||
|
//! Regression test for rustfs/backlog#629(b): region-aware `CreateBucket`.
|
||||||
|
//!
|
||||||
|
//! `minio-go`'s `MakeBucket(bucket, "us-east-1")` was reported to fail SigV4
|
||||||
|
//! validation. The SigV4 signature is verified by `s3s` over the raw request
|
||||||
|
//! body bytes (compared against `x-amz-content-sha256`); a
|
||||||
|
//! `CreateBucketConfiguration`/`LocationConstraint` body therefore does not
|
||||||
|
//! change signature canonicalization. This test proves that both a
|
||||||
|
//! region-body `CreateBucket` and a plain `CreateBucket` succeed under SigV4.
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use crate::common::{RustFSTestEnvironment, init_logging};
|
||||||
|
use aws_sdk_s3::types::{BucketLocationConstraint, CreateBucketConfiguration};
|
||||||
|
use serial_test::serial;
|
||||||
|
use std::error::Error;
|
||||||
|
|
||||||
|
/// `CreateBucket` with a `LocationConstraint` body must pass SigV4 validation
|
||||||
|
/// and create the bucket, mirroring `minio-go` `MakeBucket(bucket, "us-east-1")`.
|
||||||
|
#[tokio::test]
|
||||||
|
#[serial]
|
||||||
|
async fn test_create_bucket_with_us_east_1_location_constraint() -> Result<(), Box<dyn Error + Send + Sync>> {
|
||||||
|
init_logging();
|
||||||
|
let mut env = RustFSTestEnvironment::new().await?;
|
||||||
|
env.start_rustfs_server(vec![]).await?;
|
||||||
|
let client = env.create_s3_client();
|
||||||
|
|
||||||
|
client
|
||||||
|
.create_bucket()
|
||||||
|
.bucket("region-aware-bucket")
|
||||||
|
.create_bucket_configuration(
|
||||||
|
CreateBucketConfiguration::builder()
|
||||||
|
.location_constraint(BucketLocationConstraint::from("us-east-1"))
|
||||||
|
.build(),
|
||||||
|
)
|
||||||
|
.send()
|
||||||
|
.await
|
||||||
|
.expect("region-aware CreateBucket must pass SigV4 validation and succeed");
|
||||||
|
|
||||||
|
// The bucket exists and is listed.
|
||||||
|
let listed = client.list_buckets().send().await?;
|
||||||
|
let names: Vec<&str> = listed.buckets().iter().filter_map(|b| b.name()).collect();
|
||||||
|
assert!(names.contains(&"region-aware-bucket"), "created bucket must be listed: {names:?}");
|
||||||
|
|
||||||
|
env.stop_server();
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A plain `CreateBucket` (no body) must also succeed; guards against a
|
||||||
|
/// regression where an empty body would be hashed incorrectly during SigV4.
|
||||||
|
#[tokio::test]
|
||||||
|
#[serial]
|
||||||
|
async fn test_create_bucket_without_location_constraint() -> Result<(), Box<dyn Error + Send + Sync>> {
|
||||||
|
init_logging();
|
||||||
|
let mut env = RustFSTestEnvironment::new().await?;
|
||||||
|
env.start_rustfs_server(vec![]).await?;
|
||||||
|
let client = env.create_s3_client();
|
||||||
|
|
||||||
|
client
|
||||||
|
.create_bucket()
|
||||||
|
.bucket("plain-region-bucket")
|
||||||
|
.send()
|
||||||
|
.await
|
||||||
|
.expect("plain CreateBucket must succeed");
|
||||||
|
|
||||||
|
env.stop_server();
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -175,4 +175,16 @@ mod admin_timeout_regression_test;
|
|||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod overwrite_cleanup_regression_test;
|
mod overwrite_cleanup_regression_test;
|
||||||
|
|
||||||
|
// Regression test for backlog#601: `GET //` ListBuckets browser compatibility.
|
||||||
|
#[cfg(test)]
|
||||||
|
mod list_buckets_double_slash_test;
|
||||||
|
|
||||||
|
// Regression test for backlog#629(b): region-aware CreateBucket SigV4.
|
||||||
|
#[cfg(test)]
|
||||||
|
mod create_bucket_region_test;
|
||||||
|
|
||||||
|
// Regression coverage for backlog#618 item 8: copy-source invalid-date header.
|
||||||
|
#[cfg(test)]
|
||||||
|
mod copy_source_invalid_date_test;
|
||||||
|
|
||||||
pub mod tls_gen;
|
pub mod tls_gen;
|
||||||
|
|||||||
@@ -0,0 +1,137 @@
|
|||||||
|
// Copyright 2024 RustFS Team
|
||||||
|
//
|
||||||
|
// Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
|
// you may not use this file except in compliance with the License.
|
||||||
|
// You may obtain a copy of the License at
|
||||||
|
//
|
||||||
|
// http://www.apache.org/licenses/LICENSE-2.0
|
||||||
|
//
|
||||||
|
// Unless required by applicable law or agreed to in writing, software
|
||||||
|
// distributed under the License is distributed on an "AS IS" BASIS,
|
||||||
|
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||||
|
// See the License for the specific language governing permissions and
|
||||||
|
// limitations under the License.
|
||||||
|
|
||||||
|
//! Regression test for rustfs/backlog#601: `GET //` ListBuckets browser compat.
|
||||||
|
//!
|
||||||
|
//! The AWS S3 browser (SigV4) concatenates the endpoint with a leading slash and
|
||||||
|
//! emits `GET //` for `ListBuckets`. The `s3s` path parser rejects `//` with
|
||||||
|
//! `InvalidBucketName` (empty bucket) before routing. `DoubleSlashListBucketsCompatLayer`
|
||||||
|
//! rewrites `GET //` to `GET /` up front so it routes to `ListBuckets`, matching
|
||||||
|
//! MinIO's explicit `//` route.
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use crate::common::{RustFSTestEnvironment, init_logging, local_http_client};
|
||||||
|
use http::header::HOST;
|
||||||
|
use rustfs_signer::constants::UNSIGNED_PAYLOAD;
|
||||||
|
use rustfs_signer::sign_v4;
|
||||||
|
use s3s::Body;
|
||||||
|
use serial_test::serial;
|
||||||
|
use std::error::Error;
|
||||||
|
|
||||||
|
/// Sends a SigV4-signed `GET` where the signature is computed over `sign_path`
|
||||||
|
/// but the on-the-wire request target is `wire_path`. Returns (status, body).
|
||||||
|
async fn signed_get(
|
||||||
|
base_url: &str,
|
||||||
|
sign_path: &str,
|
||||||
|
wire_path: &str,
|
||||||
|
access_key: &str,
|
||||||
|
secret_key: &str,
|
||||||
|
) -> Result<(reqwest::StatusCode, String), Box<dyn Error + Send + Sync>> {
|
||||||
|
let sign_url = format!("{base_url}{sign_path}");
|
||||||
|
let uri = sign_url.parse::<http::Uri>()?;
|
||||||
|
let authority = uri.authority().ok_or("missing authority")?.to_string();
|
||||||
|
let request = http::Request::builder()
|
||||||
|
.method(http::Method::GET)
|
||||||
|
.uri(uri)
|
||||||
|
.header(HOST, authority)
|
||||||
|
.header("x-amz-content-sha256", UNSIGNED_PAYLOAD);
|
||||||
|
let signed = sign_v4(request.body(Body::empty())?, 0, access_key, secret_key, "", "us-east-1");
|
||||||
|
|
||||||
|
let client = local_http_client();
|
||||||
|
let mut rb = client.request(reqwest::Method::GET, format!("{base_url}{wire_path}"));
|
||||||
|
for (name, value) in signed.headers() {
|
||||||
|
rb = rb.header(name, value);
|
||||||
|
}
|
||||||
|
let resp = rb.send().await?;
|
||||||
|
let status = resp.status();
|
||||||
|
let body = resp.text().await?;
|
||||||
|
Ok((status, body))
|
||||||
|
}
|
||||||
|
|
||||||
|
fn assert_list_buckets_ok(status: reqwest::StatusCode, body: &str, label: &str) {
|
||||||
|
assert_eq!(status, reqwest::StatusCode::OK, "{label}: expected 200, body: {body}");
|
||||||
|
assert!(body.contains("ListAllMyBucketsResult"), "{label}: expected ListBuckets XML, body: {body}");
|
||||||
|
}
|
||||||
|
|
||||||
|
/// `GET /` (path-style service call) returns `ListBuckets`.
|
||||||
|
#[tokio::test]
|
||||||
|
#[serial]
|
||||||
|
async fn test_list_buckets_single_slash() -> Result<(), Box<dyn Error + Send + Sync>> {
|
||||||
|
init_logging();
|
||||||
|
let mut env = RustFSTestEnvironment::new().await?;
|
||||||
|
env.start_rustfs_server(vec![]).await?;
|
||||||
|
|
||||||
|
let (status, body) = signed_get(&env.url, "/", "/", &env.access_key, &env.secret_key).await?;
|
||||||
|
assert_list_buckets_ok(status, &body, "GET /");
|
||||||
|
|
||||||
|
env.stop_server();
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
/// `GET //` returns `ListBuckets` for the S3 browser SigV4 case: the client
|
||||||
|
/// concatenates the endpoint with a leading `/` so the request target becomes
|
||||||
|
/// `//`, while the signature was computed over the canonical `/` path. The
|
||||||
|
/// compat layer rewrites `//` to `/` before `s3s` parses/verifies the request,
|
||||||
|
/// so both routing and signature verification operate on `/`.
|
||||||
|
#[tokio::test]
|
||||||
|
#[serial]
|
||||||
|
async fn test_list_buckets_double_slash_browser_compat() -> Result<(), Box<dyn Error + Send + Sync>> {
|
||||||
|
init_logging();
|
||||||
|
let mut env = RustFSTestEnvironment::new().await?;
|
||||||
|
env.start_rustfs_server(vec![]).await?;
|
||||||
|
|
||||||
|
let (status, body) = signed_get(&env.url, "/", "//", &env.access_key, &env.secret_key).await?;
|
||||||
|
assert_list_buckets_ok(status, &body, "GET // (signature over /)");
|
||||||
|
|
||||||
|
env.stop_server();
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The rewrite is confined to a request target of exactly `//`. A normal
|
||||||
|
/// path-style bucket listing (`GET /bucket`) must keep working (no routing
|
||||||
|
/// regression), and a leading double slash before a bucket name
|
||||||
|
/// (`GET //bucket`) must be left untouched by the compat layer — it is not a
|
||||||
|
/// `ListBuckets` request and s3s continues to reject the empty bucket name.
|
||||||
|
#[tokio::test]
|
||||||
|
#[serial]
|
||||||
|
async fn test_double_slash_rewrite_is_narrowly_scoped() -> Result<(), Box<dyn Error + Send + Sync>> {
|
||||||
|
init_logging();
|
||||||
|
let mut env = RustFSTestEnvironment::new().await?;
|
||||||
|
env.start_rustfs_server(vec![]).await?;
|
||||||
|
|
||||||
|
let client = env.create_s3_client();
|
||||||
|
client.create_bucket().bucket("compat-bucket").send().await?;
|
||||||
|
|
||||||
|
// `GET /compat-bucket` still lists the bucket (routing unchanged).
|
||||||
|
let (status, body) = signed_get(&env.url, "/compat-bucket", "/compat-bucket", &env.access_key, &env.secret_key).await?;
|
||||||
|
assert_eq!(status, reqwest::StatusCode::OK, "GET /compat-bucket body: {body}");
|
||||||
|
assert!(
|
||||||
|
body.contains("ListBucketResult") && !body.contains("ListAllMyBucketsResult"),
|
||||||
|
"GET /compat-bucket must be a bucket listing: {body}"
|
||||||
|
);
|
||||||
|
|
||||||
|
// `GET //compat-bucket` is not rewritten (path is not exactly `//`); s3s
|
||||||
|
// still rejects the empty leading bucket segment with InvalidBucketName.
|
||||||
|
let (status, body) = signed_get(&env.url, "//compat-bucket", "//compat-bucket", &env.access_key, &env.secret_key).await?;
|
||||||
|
assert_eq!(status, reqwest::StatusCode::BAD_REQUEST, "GET //compat-bucket body: {body}");
|
||||||
|
assert!(
|
||||||
|
body.contains("InvalidBucketName"),
|
||||||
|
"GET //compat-bucket must remain InvalidBucketName (not rewritten): {body}"
|
||||||
|
);
|
||||||
|
|
||||||
|
env.stop_server();
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -22,9 +22,9 @@ use crate::server::{
|
|||||||
compress::{HttpCompressionConfig, PathAwareHttpCompressionPredicate, PathCategoryInjectionLayer},
|
compress::{HttpCompressionConfig, PathAwareHttpCompressionPredicate, PathCategoryInjectionLayer},
|
||||||
hybrid::hybrid,
|
hybrid::hybrid,
|
||||||
layer::{
|
layer::{
|
||||||
BodylessStatusFixLayer, ConditionalCorsLayer, EmptyBodyContentLengthCompatLayer, HeadRequestBodyFixLayer,
|
BodylessStatusFixLayer, ConditionalCorsLayer, DoubleSlashListBucketsCompatLayer, EmptyBodyContentLengthCompatLayer,
|
||||||
ObjectAttributesEtagFixLayer, PublicHealthEndpointLayer, RedirectLayer, RequestContextLayer, RequestLoggingLayer,
|
HeadRequestBodyFixLayer, ObjectAttributesEtagFixLayer, PublicHealthEndpointLayer, RedirectLayer, RequestContextLayer,
|
||||||
S3ErrorMessageCompatLayer, VirtualHostStyleHintLayer, redact_sensitive_uri_query,
|
RequestLoggingLayer, S3ErrorMessageCompatLayer, VirtualHostStyleHintLayer, redact_sensitive_uri_query,
|
||||||
},
|
},
|
||||||
tls_material::{
|
tls_material::{
|
||||||
TlsAcceptorHolder, TlsHandshakeFailureKind, build_acceptor_from_loaded, load_tls_material, spawn_reload_loop,
|
TlsAcceptorHolder, TlsHandshakeFailureKind, build_acceptor_from_loaded, load_tls_material, spawn_reload_loop,
|
||||||
@@ -1097,6 +1097,7 @@ fn process_connection(
|
|||||||
// 20. HeadRequestBodyFixLayer — strips actual body bytes from HEAD responses
|
// 20. HeadRequestBodyFixLayer — strips actual body bytes from HEAD responses
|
||||||
// 21. PublicHealthEndpointLayer — handles public health before s3s host parsing
|
// 21. PublicHealthEndpointLayer — handles public health before s3s host parsing
|
||||||
// 22. VirtualHostStyleHintLayer — actionable error for unroutable virtual-hosted-style (conditional)
|
// 22. VirtualHostStyleHintLayer — actionable error for unroutable virtual-hosted-style (conditional)
|
||||||
|
// 23. DoubleSlashListBucketsCompatLayer — rewrites `GET //` to `GET /` for ListBuckets (MinIO browser compat)
|
||||||
// ─────────────────────────────────────────────────────────────
|
// ─────────────────────────────────────────────────────────────
|
||||||
// Batch 1 intentionally keeps the external and internode stacks behaviorally
|
// Batch 1 intentionally keeps the external and internode stacks behaviorally
|
||||||
// identical while giving each path family a named construction boundary.
|
// identical while giving each path family a named construction boundary.
|
||||||
@@ -1265,6 +1266,7 @@ fn process_connection(
|
|||||||
.layer(HeadRequestBodyFixLayer)
|
.layer(HeadRequestBodyFixLayer)
|
||||||
.layer(PublicHealthEndpointLayer)
|
.layer(PublicHealthEndpointLayer)
|
||||||
.option_layer((!server_domains_configured && !is_console).then_some(VirtualHostStyleHintLayer))
|
.option_layer((!server_domains_configured && !is_console).then_some(VirtualHostStyleHintLayer))
|
||||||
|
.layer(DoubleSlashListBucketsCompatLayer)
|
||||||
.service(service)
|
.service(service)
|
||||||
};
|
};
|
||||||
let build_internode_stack = |service| {
|
let build_internode_stack = |service| {
|
||||||
@@ -1414,6 +1416,7 @@ fn process_connection(
|
|||||||
.layer(HeadRequestBodyFixLayer)
|
.layer(HeadRequestBodyFixLayer)
|
||||||
.layer(PublicHealthEndpointLayer)
|
.layer(PublicHealthEndpointLayer)
|
||||||
.option_layer((!server_domains_configured && !is_console).then_some(VirtualHostStyleHintLayer))
|
.option_layer((!server_domains_configured && !is_console).then_some(VirtualHostStyleHintLayer))
|
||||||
|
.layer(DoubleSlashListBucketsCompatLayer)
|
||||||
.service(service)
|
.service(service)
|
||||||
};
|
};
|
||||||
let external_stack_service = build_external_stack(external_service);
|
let external_stack_service = build_external_stack(external_service);
|
||||||
|
|||||||
@@ -1544,6 +1544,79 @@ where
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Rewrites the request-target of a root-level double-slash `GET //` to `GET /`
|
||||||
|
/// so it routes to `ListBuckets`, matching MinIO browser compatibility.
|
||||||
|
///
|
||||||
|
/// The AWS S3 browser client (SigV4) concatenates the endpoint with a leading
|
||||||
|
/// slash and emits `GET //` for `ListBuckets`. The `s3s` path parser strips the
|
||||||
|
/// single leading slash and then treats the remaining `/` as an empty bucket
|
||||||
|
/// name, rejecting the request with `InvalidBucketName` before it is routed.
|
||||||
|
/// MinIO's `api-router.go` registers an explicit `//` route to `ListBuckets`;
|
||||||
|
/// this layer reproduces that behavior by collapsing the path to `/` up front.
|
||||||
|
///
|
||||||
|
/// The rewrite is intentionally limited to a path that is exactly `//` (optionally
|
||||||
|
/// followed by a query string). It never touches bucket/object requests, so object
|
||||||
|
/// keys with embedded or leading slashes are unaffected (those are normalized by
|
||||||
|
/// `s3s` via `normalize_forward_slash_path`).
|
||||||
|
#[derive(Clone)]
|
||||||
|
pub struct DoubleSlashListBucketsCompatLayer;
|
||||||
|
|
||||||
|
impl<S> Layer<S> for DoubleSlashListBucketsCompatLayer {
|
||||||
|
type Service = DoubleSlashListBucketsCompatService<S>;
|
||||||
|
|
||||||
|
fn layer(&self, inner: S) -> Self::Service {
|
||||||
|
DoubleSlashListBucketsCompatService { inner }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[derive(Clone)]
|
||||||
|
pub struct DoubleSlashListBucketsCompatService<S> {
|
||||||
|
inner: S,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl<S, ReqBody, ResBody> Service<HttpRequest<ReqBody>> for DoubleSlashListBucketsCompatService<S>
|
||||||
|
where
|
||||||
|
S: Service<HttpRequest<ReqBody>, Response = Response<ResBody>> + Clone + Send + 'static,
|
||||||
|
S::Future: Send + 'static,
|
||||||
|
ReqBody: Send + 'static,
|
||||||
|
ResBody: Send + 'static,
|
||||||
|
{
|
||||||
|
type Response = Response<ResBody>;
|
||||||
|
type Error = S::Error;
|
||||||
|
type Future = S::Future;
|
||||||
|
|
||||||
|
fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
|
||||||
|
self.inner.poll_ready(cx)
|
||||||
|
}
|
||||||
|
|
||||||
|
fn call(&mut self, mut req: HttpRequest<ReqBody>) -> Self::Future {
|
||||||
|
if req.method() == Method::GET
|
||||||
|
&& let Some(rewritten) = rewrite_double_slash_root(req.uri())
|
||||||
|
{
|
||||||
|
*req.uri_mut() = rewritten;
|
||||||
|
}
|
||||||
|
self.inner.call(req)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Returns the `/`-rooted URI when `uri` has a request-target path of exactly `//`.
|
||||||
|
///
|
||||||
|
/// Returns `None` for every other path so the rewrite is confined to the
|
||||||
|
/// root-level double-slash case.
|
||||||
|
fn rewrite_double_slash_root(uri: &Uri) -> Option<Uri> {
|
||||||
|
if uri.path() != "//" {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
|
||||||
|
let mut parts = uri.clone().into_parts();
|
||||||
|
let path_and_query = match uri.query() {
|
||||||
|
Some(query) => format!("/?{query}"),
|
||||||
|
None => "/".to_string(),
|
||||||
|
};
|
||||||
|
parts.path_and_query = Some(path_and_query.parse().ok()?);
|
||||||
|
Uri::from_parts(parts).ok()
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
@@ -3347,4 +3420,82 @@ mod tests {
|
|||||||
assert!(output.contains("500"), "{output}");
|
assert!(output.contains("500"), "{output}");
|
||||||
assert!(output.contains("server_error"), "{output}");
|
assert!(output.contains("server_error"), "{output}");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn rewrite_double_slash_root_only_matches_exact_double_slash() {
|
||||||
|
// Exactly `//` is rewritten to `/` (query preserved).
|
||||||
|
assert_eq!(
|
||||||
|
rewrite_double_slash_root(&Uri::from_static("//")).map(|u| u.to_string()),
|
||||||
|
Some("/".to_string())
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
rewrite_double_slash_root(&Uri::from_static("//?x-id=ListBuckets")).map(|u| u.to_string()),
|
||||||
|
Some("/?x-id=ListBuckets".to_string())
|
||||||
|
);
|
||||||
|
|
||||||
|
// Everything else is left untouched.
|
||||||
|
assert_eq!(rewrite_double_slash_root(&Uri::from_static("/")), None);
|
||||||
|
assert_eq!(rewrite_double_slash_root(&Uri::from_static("//bucket")), None);
|
||||||
|
assert_eq!(rewrite_double_slash_root(&Uri::from_static("/bucket")), None);
|
||||||
|
assert_eq!(rewrite_double_slash_root(&Uri::from_static("/bucket//key")), None);
|
||||||
|
assert_eq!(rewrite_double_slash_root(&Uri::from_static("///")), None);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Inner service that records the request-target path it received and
|
||||||
|
/// returns an empty response, so the layer's URI rewrite can be observed.
|
||||||
|
#[derive(Clone, Default)]
|
||||||
|
struct UriCaptureService {
|
||||||
|
path: Arc<Mutex<Option<String>>>,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl<B> Service<Request<B>> for UriCaptureService {
|
||||||
|
type Response = Response<Full<Bytes>>;
|
||||||
|
type Error = Infallible;
|
||||||
|
type Future = Ready<Result<Self::Response, Self::Error>>;
|
||||||
|
|
||||||
|
fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
|
||||||
|
Poll::Ready(Ok(()))
|
||||||
|
}
|
||||||
|
|
||||||
|
fn call(&mut self, req: Request<B>) -> Self::Future {
|
||||||
|
*self.path.lock().expect("path lock") = Some(req.uri().path().to_string());
|
||||||
|
ready(Ok(Response::new(Full::from(Bytes::new()))))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn double_slash_compat_layer_rewrites_get_double_slash() {
|
||||||
|
let inner = UriCaptureService::default();
|
||||||
|
let seen = inner.path.clone();
|
||||||
|
let mut service = DoubleSlashListBucketsCompatLayer.layer(inner);
|
||||||
|
|
||||||
|
let observe = |seen: &Arc<Mutex<Option<String>>>| seen.lock().expect("path lock").clone();
|
||||||
|
|
||||||
|
// `GET //` becomes `GET /`.
|
||||||
|
let req = Request::builder()
|
||||||
|
.method(Method::GET)
|
||||||
|
.uri("//")
|
||||||
|
.body(Full::<Bytes>::from(Bytes::new()))
|
||||||
|
.expect("request");
|
||||||
|
service.call(req).await.expect("response");
|
||||||
|
assert_eq!(observe(&seen).as_deref(), Some("/"));
|
||||||
|
|
||||||
|
// Non-GET `//` is not rewritten.
|
||||||
|
let req = Request::builder()
|
||||||
|
.method(Method::PUT)
|
||||||
|
.uri("//")
|
||||||
|
.body(Full::<Bytes>::from(Bytes::new()))
|
||||||
|
.expect("request");
|
||||||
|
service.call(req).await.expect("response");
|
||||||
|
assert_eq!(observe(&seen).as_deref(), Some("//"));
|
||||||
|
|
||||||
|
// `GET //bucket` is not rewritten.
|
||||||
|
let req = Request::builder()
|
||||||
|
.method(Method::GET)
|
||||||
|
.uri("//bucket")
|
||||||
|
.body(Full::<Bytes>::from(Bytes::new()))
|
||||||
|
.expect("request");
|
||||||
|
service.call(req).await.expect("response");
|
||||||
|
assert_eq!(observe(&seen).as_deref(), Some("//bucket"));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user