Skip to content

Commit 786653e

Browse files
committed
fix: keep the endpoint path out of the Host header
`Region::Custom` accepts an endpoint that carries a path, and `Region::host` keeps that path so `Bucket::url` can place the bucket and key underneath it. `host_header` returned the same string, so the path travelled in the `Host` header as well: PUT /storage/v1/s3/media/0000.bin HTTP/1.1 host: project.supabase.co/storage/v1/s3 RFC 9110 §7.2 defines `Host` as `uri-host [ ":" port ]`. A server that checks answers `400 Bad Request` with no detail, which makes every request to such an endpoint fail and says nothing about why. Supabase Storage's S3 endpoint is `https://<project>.supabase.co/storage/v1/s3` and is unusable for this reason: `put_object` fails before any signature is verified. AWS, MinIO and R2 have no path in their endpoints, so nothing here had exercised the case. `host_header` now takes the authority and leaves `Region::host` alone, so the request line keeps the endpoint path it needs. Verified against a Supabase Storage bucket, where `put_object`, `head_object`, `get_object` and `get_object_range` all succeed after the change and only the last three are reachable before it, and against MinIO, which is unaffected.
1 parent b584ce7 commit 786653e

1 file changed

Lines changed: 36 additions & 1 deletion

File tree

s3/src/request/request_trait.rs

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,20 @@ impl async_std::io::Read for ResponseDataStream {
206206
}
207207
}
208208

209+
/// The authority of an endpoint host, dropping any path that follows it.
210+
///
211+
/// A custom endpoint may carry a path, as Supabase Storage's
212+
/// `https://<project>.supabase.co/storage/v1/s3` does, and `Region::host`
213+
/// keeps it so that `Bucket::url` can place the bucket and key underneath.
214+
/// RFC 9110 §7.2 defines `Host` as `uri-host [ ":" port ]`, so that path
215+
/// cannot travel in the header.
216+
fn authority_of(host: &str) -> &str {
217+
match host.find('/') {
218+
Some(position) => &host[..position],
219+
None => host,
220+
}
221+
}
222+
209223
#[maybe_async::maybe_async]
210224
pub trait Request {
211225
type Response;
@@ -291,7 +305,7 @@ pub trait Request {
291305
}
292306

293307
fn host_header(&self) -> String {
294-
self.bucket().host()
308+
authority_of(&self.bucket().host()).to_string()
295309
}
296310

297311
#[maybe_async::async_impl]
@@ -876,6 +890,27 @@ mod tests {
876890
use futures_util::stream;
877891
use tokio::io::AsyncReadExt;
878892

893+
#[test]
894+
fn test_authority_of_keeps_a_plain_host() {
895+
assert_eq!(
896+
authority_of("s3.eu-central-1.amazonaws.com"),
897+
"s3.eu-central-1.amazonaws.com"
898+
);
899+
assert_eq!(authority_of("127.0.0.1:9000"), "127.0.0.1:9000");
900+
}
901+
902+
#[test]
903+
fn test_authority_of_drops_an_endpoint_path() {
904+
assert_eq!(
905+
authority_of("project.supabase.co/storage/v1/s3"),
906+
"project.supabase.co"
907+
);
908+
assert_eq!(
909+
authority_of("127.0.0.1:54321/storage/v1/s3"),
910+
"127.0.0.1:54321"
911+
);
912+
}
913+
879914
#[tokio::test]
880915
async fn test_async_read_implementation() {
881916
// Create a mock stream with test data

0 commit comments

Comments
 (0)