Skip to content

Commit c45d436

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 9b5dd99 commit c45d436

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
@@ -3842,6 +3842,76 @@ mod test {
38423842
assert!(url.contains("/test/test.file?"))
38433843
}
38443844

3845+
#[maybe_async::test(
3846+
feature = "sync",
3847+
async(all(not(feature = "sync"), feature = "with-tokio"), tokio::test),
3848+
async(
3849+
all(not(feature = "sync"), feature = "with-async-std"),
3850+
async_std::test
3851+
)
3852+
)]
3853+
async fn test_presign_url_standard_ports() {
3854+
// Test that presigned URLs preserve standard ports in the host header
3855+
// This is crucial for signature validation
3856+
3857+
// Test with HTTP standard port 80
3858+
let region_http_80 = Region::Custom {
3859+
region: "eu-central-1".to_owned(),
3860+
endpoint: "http://minio:80".to_owned(),
3861+
};
3862+
let credentials = Credentials::new(
3863+
Some("test_access_key"),
3864+
Some("test_secret_key"),
3865+
None,
3866+
None,
3867+
None,
3868+
).unwrap();
3869+
let bucket_http_80 = Bucket::new("test-bucket", region_http_80, credentials.clone())
3870+
.unwrap()
3871+
.with_path_style();
3872+
3873+
let presigned_url_80 = bucket_http_80.presign_get("/test.file", 3600, None).await.unwrap();
3874+
println!("Presigned URL with port 80: {}", presigned_url_80);
3875+
3876+
// Port 80 MUST be preserved in the URL for signature validation
3877+
assert!(
3878+
presigned_url_80.starts_with("http://minio:80/"),
3879+
"URL must preserve port 80, got: {}",
3880+
presigned_url_80
3881+
);
3882+
3883+
// Test with HTTPS standard port 443
3884+
let region_https_443 = Region::Custom {
3885+
region: "eu-central-1".to_owned(),
3886+
endpoint: "https://minio:443".to_owned(),
3887+
};
3888+
let bucket_https_443 = Bucket::new("test-bucket", region_https_443, credentials.clone())
3889+
.unwrap()
3890+
.with_path_style();
3891+
3892+
let presigned_url_443 = bucket_https_443.presign_get("/test.file", 3600, None).await.unwrap();
3893+
println!("Presigned URL with port 443: {}", presigned_url_443);
3894+
3895+
// Port 443 MUST be preserved in the URL for signature validation
3896+
assert!(
3897+
presigned_url_443.starts_with("https://minio:443/"),
3898+
"URL must preserve port 443, got: {}",
3899+
presigned_url_443
3900+
);
3901+
3902+
// Test with non-standard port (should always include port)
3903+
let region_http_9000 = Region::Custom {
3904+
region: "eu-central-1".to_owned(),
3905+
endpoint: "http://minio:9000".to_owned(),
3906+
};
3907+
let bucket_http_9000 = Bucket::new("test-bucket", region_http_9000, credentials)
3908+
.unwrap()
3909+
.with_path_style();
3910+
3911+
let presigned_url_9000 = bucket_http_9000.presign_get("/test.file", 3600, None).await.unwrap();
3912+
assert!(presigned_url_9000.contains("minio:9000"), "Non-standard port should be preserved in URL");
3913+
}
3914+
38453915
#[maybe_async::test(
38463916
feature = "sync",
38473917
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)