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:
Zhengchao An
2026-07-07 04:36:16 +08:00
committed by GitHub
parent eb02486574
commit 511ad31ba0
6 changed files with 530 additions and 3 deletions
+6 -3
View File
@@ -22,9 +22,9 @@ use crate::server::{
compress::{HttpCompressionConfig, PathAwareHttpCompressionPredicate, PathCategoryInjectionLayer},
hybrid::hybrid,
layer::{
BodylessStatusFixLayer, ConditionalCorsLayer, EmptyBodyContentLengthCompatLayer, HeadRequestBodyFixLayer,
ObjectAttributesEtagFixLayer, PublicHealthEndpointLayer, RedirectLayer, RequestContextLayer, RequestLoggingLayer,
S3ErrorMessageCompatLayer, VirtualHostStyleHintLayer, redact_sensitive_uri_query,
BodylessStatusFixLayer, ConditionalCorsLayer, DoubleSlashListBucketsCompatLayer, EmptyBodyContentLengthCompatLayer,
HeadRequestBodyFixLayer, ObjectAttributesEtagFixLayer, PublicHealthEndpointLayer, RedirectLayer, RequestContextLayer,
RequestLoggingLayer, S3ErrorMessageCompatLayer, VirtualHostStyleHintLayer, redact_sensitive_uri_query,
},
tls_material::{
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
// 21. PublicHealthEndpointLayer — handles public health before s3s host parsing
// 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
// identical while giving each path family a named construction boundary.
@@ -1265,6 +1266,7 @@ fn process_connection(
.layer(HeadRequestBodyFixLayer)
.layer(PublicHealthEndpointLayer)
.option_layer((!server_domains_configured && !is_console).then_some(VirtualHostStyleHintLayer))
.layer(DoubleSlashListBucketsCompatLayer)
.service(service)
};
let build_internode_stack = |service| {
@@ -1414,6 +1416,7 @@ fn process_connection(
.layer(HeadRequestBodyFixLayer)
.layer(PublicHealthEndpointLayer)
.option_layer((!server_domains_configured && !is_console).then_some(VirtualHostStyleHintLayer))
.layer(DoubleSlashListBucketsCompatLayer)
.service(service)
};
let external_stack_service = build_external_stack(external_service);
+151
View File
@@ -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)]
mod tests {
use super::*;
@@ -3347,4 +3420,82 @@ mod tests {
assert!(output.contains("500"), "{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"));
}
}