Skip to content

Commit 364b803

Browse files
committed
fix: preserve standard ports in presigned URLs for signature validation
When custom endpoints explicitly specify standard ports (80 for HTTP, 443 for HTTPS), these ports must be preserved in presigned URLs. The URL crate's to_string() method omits standard ports by design, but this causes signature validation failures when servers expect the port in the Host header. This fix rebuilds the presigned URL string with the original host (including port) when a custom region explicitly specifies a standard port. Non-standard ports continue to work as before since they are naturally preserved by URL parsing. Added comprehensive tests to verify correct behavior for both standard and non-standard ports. Closes #419
1 parent 7bbb1bf commit 364b803

File tree

2 files changed

+129
-3
lines changed

2 files changed

+129
-3
lines changed

s3/src/bucket.rs

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3685,6 +3685,76 @@ mod test {
36853685
assert!(url.contains("/test/test.file?"))
36863686
}
36873687

3688+
#[maybe_async::test(
3689+
feature = "sync",
3690+
async(all(not(feature = "sync"), feature = "with-tokio"), tokio::test),
3691+
async(
3692+
all(not(feature = "sync"), feature = "with-async-std"),
3693+
async_std::test
3694+
)
3695+
)]
3696+
async fn test_presign_url_standard_ports() {
3697+
// Test that presigned URLs preserve standard ports in the host header
3698+
// This is crucial for signature validation
3699+
3700+
// Test with HTTP standard port 80
3701+
let region_http_80 = Region::Custom {
3702+
region: "eu-central-1".to_owned(),
3703+
endpoint: "http://minio:80".to_owned(),
3704+
};
3705+
let credentials = Credentials::new(
3706+
Some("test_access_key"),
3707+
Some("test_secret_key"),
3708+
None,
3709+
None,
3710+
None,
3711+
).unwrap();
3712+
let bucket_http_80 = Bucket::new("test-bucket", region_http_80, credentials.clone())
3713+
.unwrap()
3714+
.with_path_style();
3715+
3716+
let presigned_url_80 = bucket_http_80.presign_get("/test.file", 3600, None).await.unwrap();
3717+
println!("Presigned URL with port 80: {}", presigned_url_80);
3718+
3719+
// Port 80 MUST be preserved in the URL for signature validation
3720+
assert!(
3721+
presigned_url_80.starts_with("http://minio:80/"),
3722+
"URL must preserve port 80, got: {}",
3723+
presigned_url_80
3724+
);
3725+
3726+
// Test with HTTPS standard port 443
3727+
let region_https_443 = Region::Custom {
3728+
region: "eu-central-1".to_owned(),
3729+
endpoint: "https://minio:443".to_owned(),
3730+
};
3731+
let bucket_https_443 = Bucket::new("test-bucket", region_https_443, credentials.clone())
3732+
.unwrap()
3733+
.with_path_style();
3734+
3735+
let presigned_url_443 = bucket_https_443.presign_get("/test.file", 3600, None).await.unwrap();
3736+
println!("Presigned URL with port 443: {}", presigned_url_443);
3737+
3738+
// Port 443 MUST be preserved in the URL for signature validation
3739+
assert!(
3740+
presigned_url_443.starts_with("https://minio:443/"),
3741+
"URL must preserve port 443, got: {}",
3742+
presigned_url_443
3743+
);
3744+
3745+
// Test with non-standard port (should always include port)
3746+
let region_http_9000 = Region::Custom {
3747+
region: "eu-central-1".to_owned(),
3748+
endpoint: "http://minio:9000".to_owned(),
3749+
};
3750+
let bucket_http_9000 = Bucket::new("test-bucket", region_http_9000, credentials)
3751+
.unwrap()
3752+
.with_path_style();
3753+
3754+
let presigned_url_9000 = bucket_http_9000.presign_get("/test.file", 3600, None).await.unwrap();
3755+
assert!(presigned_url_9000.contains("minio:9000"), "Non-standard port should be preserved in URL");
3756+
}
3757+
36883758
#[maybe_async::test(
36893759
feature = "sync",
36903760
async(all(not(feature = "sync"), feature = "with-tokio"), tokio::test),

s3/src/request/request_trait.rs

Lines changed: 59 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -283,10 +283,38 @@ pub trait Request {
283283
_ => unreachable!(),
284284
};
285285

286+
let url = self
287+
.presigned_url_no_sig(expiry, custom_headers.as_ref(), custom_queries.as_ref())
288+
.await?;
289+
290+
// Build the URL string preserving the original host (including standard ports)
291+
// The Url type drops standard ports when converting to string, but we need them
292+
// for signature validation
293+
let url_str = if let awsregion::Region::Custom { ref endpoint, .. } = self.bucket().region()
294+
{
295+
// Check if we need to preserve a standard port
296+
if (endpoint.contains(":80") && url.scheme() == "http" && url.port().is_none())
297+
|| (endpoint.contains(":443") && url.scheme() == "https" && url.port().is_none())
298+
{
299+
// Rebuild the URL with the original host from the endpoint
300+
let host = self.bucket().host();
301+
format!(
302+
"{}://{}{}{}",
303+
url.scheme(),
304+
host,
305+
url.path(),
306+
url.query().map(|q| format!("?{}", q)).unwrap_or_default()
307+
)
308+
} else {
309+
url.to_string()
310+
}
311+
} else {
312+
url.to_string()
313+
};
314+
286315
Ok(format!(
287316
"{}&X-Amz-Signature={}",
288-
self.presigned_url_no_sig(expiry, custom_headers.as_ref(), custom_queries.as_ref())
289-
.await?,
317+
url_str,
290318
self.presigned_authorization(custom_headers.as_ref())
291319
.await?
292320
))
@@ -308,9 +336,37 @@ pub trait Request {
308336
_ => unreachable!(),
309337
};
310338

339+
let url =
340+
self.presigned_url_no_sig(expiry, custom_headers.as_ref(), custom_queries.as_ref())?;
341+
342+
// Build the URL string preserving the original host (including standard ports)
343+
// The Url type drops standard ports when converting to string, but we need them
344+
// for signature validation
345+
let url_str = if let awsregion::Region::Custom { ref endpoint, .. } = self.bucket().region()
346+
{
347+
// Check if we need to preserve a standard port
348+
if (endpoint.contains(":80") && url.scheme() == "http" && url.port().is_none())
349+
|| (endpoint.contains(":443") && url.scheme() == "https" && url.port().is_none())
350+
{
351+
// Rebuild the URL with the original host from the endpoint
352+
let host = self.bucket().host();
353+
format!(
354+
"{}://{}{}{}",
355+
url.scheme(),
356+
host,
357+
url.path(),
358+
url.query().map(|q| format!("?{}", q)).unwrap_or_default()
359+
)
360+
} else {
361+
url.to_string()
362+
}
363+
} else {
364+
url.to_string()
365+
};
366+
311367
Ok(format!(
312368
"{}&X-Amz-Signature={}",
313-
self.presigned_url_no_sig(expiry, custom_headers.as_ref(), custom_queries.as_ref())?,
369+
url_str,
314370
self.presigned_authorization(custom_headers.as_ref())?
315371
))
316372
}

0 commit comments

Comments
 (0)