fix issue where list on vhost-bucket would list bucket instead of bucket content
Some checks failed
continuous-integration/drone/pr Build is failing
Some checks failed
continuous-integration/drone/pr Build is failing
This commit is contained in:
parent
4205cdef99
commit
2e70257d29
1 changed files with 29 additions and 73 deletions
|
@ -84,9 +84,6 @@ async fn handler_inner(garage: Arc<Garage>, req: Request<Body>) -> Result<Respon
|
||||||
let path = req.uri().path().to_string();
|
let path = req.uri().path().to_string();
|
||||||
let path = percent_encoding::percent_decode_str(&path).decode_utf8()?;
|
let path = percent_encoding::percent_decode_str(&path).decode_utf8()?;
|
||||||
let (api_key, content_sha256) = check_signature(&garage, &req).await?;
|
let (api_key, content_sha256) = check_signature(&garage, &req).await?;
|
||||||
if path == "/" {
|
|
||||||
return handle_list_buckets(&api_key);
|
|
||||||
}
|
|
||||||
|
|
||||||
let authority = req
|
let authority = req
|
||||||
.headers()
|
.headers()
|
||||||
|
@ -94,14 +91,20 @@ async fn handler_inner(garage: Arc<Garage>, req: Request<Body>) -> Result<Respon
|
||||||
.ok_or_else(|| Error::BadRequest("HOST header required".to_owned()))?
|
.ok_or_else(|| Error::BadRequest("HOST header required".to_owned()))?
|
||||||
.to_str()?;
|
.to_str()?;
|
||||||
|
|
||||||
// Get bucket
|
|
||||||
let host = authority_to_host(authority)?;
|
let host = authority_to_host(authority)?;
|
||||||
|
|
||||||
let (bucket, key) = parse_bucket_key(
|
let bucket = garage
|
||||||
&path,
|
.config
|
||||||
Some(&host),
|
.s3_api
|
||||||
garage.config.s3_api.root_domain.as_deref(),
|
.root_domain
|
||||||
)?;
|
.as_ref()
|
||||||
|
.and_then(|root_domain| host_to_bucket(&host, &root_domain));
|
||||||
|
|
||||||
|
if path == "/" && bucket.is_none() {
|
||||||
|
return handle_list_buckets(&api_key);
|
||||||
|
}
|
||||||
|
|
||||||
|
let (bucket, key) = parse_bucket_key(&path, bucket)?;
|
||||||
let allowed = match req.method() {
|
let allowed = match req.method() {
|
||||||
&Method::HEAD | &Method::GET => api_key.allow_read(bucket),
|
&Method::HEAD | &Method::GET => api_key.allow_read(bucket),
|
||||||
_ => api_key.allow_write(bucket),
|
_ => api_key.allow_write(bucket),
|
||||||
|
@ -152,7 +155,7 @@ async fn handler_inner(garage: Arc<Garage>, req: Request<Body>) -> Result<Respon
|
||||||
let copy_source = req.headers().get("x-amz-copy-source").unwrap().to_str()?;
|
let copy_source = req.headers().get("x-amz-copy-source").unwrap().to_str()?;
|
||||||
let copy_source =
|
let copy_source =
|
||||||
percent_encoding::percent_decode_str(copy_source).decode_utf8()?;
|
percent_encoding::percent_decode_str(copy_source).decode_utf8()?;
|
||||||
let (source_bucket, source_key) = parse_bucket_key(©_source, None, None)?;
|
let (source_bucket, source_key) = parse_bucket_key(©_source, None)?;
|
||||||
if !api_key.allow_read(source_bucket) {
|
if !api_key.allow_read(source_bucket) {
|
||||||
return Err(Error::Forbidden(format!(
|
return Err(Error::Forbidden(format!(
|
||||||
"Reading from bucket {} not allowed for this key",
|
"Reading from bucket {} not allowed for this key",
|
||||||
|
@ -260,24 +263,21 @@ async fn handler_inner(garage: Arc<Garage>, req: Request<Body>) -> Result<Respon
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Extract the bucket name and the key name from an HTTP path
|
/// Extract the bucket name and the key name from an HTTP path and possibly Host header
|
||||||
///
|
///
|
||||||
/// S3 internally manages only buckets and keys. This function splits
|
/// S3 internally manages only buckets and keys. This function splits
|
||||||
/// an HTTP path to get the corresponding bucket name and key.
|
/// an HTTP path to get the corresponding bucket name and key.
|
||||||
fn parse_bucket_key<'a>(
|
fn parse_bucket_key<'a>(
|
||||||
path: &'a str,
|
path: &'a str,
|
||||||
host: Option<&'a str>,
|
host_bucket: Option<&'a str>,
|
||||||
root: Option<&str>,
|
|
||||||
) -> Result<(&'a str, Option<&'a str>), Error> {
|
) -> Result<(&'a str, Option<&'a str>), Error> {
|
||||||
let path = path.trim_start_matches('/');
|
let path = path.trim_start_matches('/');
|
||||||
|
|
||||||
if host.and(root).is_some() {
|
if let Some(bucket) = host_bucket {
|
||||||
if let Some(bucket) = host_to_bucket(host.unwrap(), root.unwrap()) {
|
if !path.is_empty() {
|
||||||
if !path.is_empty() {
|
return Ok((bucket, Some(path)));
|
||||||
return Ok((bucket, Some(path)));
|
} else {
|
||||||
} else {
|
return Ok((bucket, None));
|
||||||
return Ok((bucket, None));
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -304,7 +304,7 @@ mod tests {
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn parse_bucket_containing_a_key() -> Result<(), Error> {
|
fn parse_bucket_containing_a_key() -> Result<(), Error> {
|
||||||
let (bucket, key) = parse_bucket_key("/my_bucket/a/super/file.jpg", None, None)?;
|
let (bucket, key) = parse_bucket_key("/my_bucket/a/super/file.jpg", None)?;
|
||||||
assert_eq!(bucket, "my_bucket");
|
assert_eq!(bucket, "my_bucket");
|
||||||
assert_eq!(key.expect("key must be set"), "a/super/file.jpg");
|
assert_eq!(key.expect("key must be set"), "a/super/file.jpg");
|
||||||
Ok(())
|
Ok(())
|
||||||
|
@ -312,10 +312,10 @@ mod tests {
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn parse_bucket_containing_no_key() -> Result<(), Error> {
|
fn parse_bucket_containing_no_key() -> Result<(), Error> {
|
||||||
let (bucket, key) = parse_bucket_key("/my_bucket/", None, None)?;
|
let (bucket, key) = parse_bucket_key("/my_bucket/", None)?;
|
||||||
assert_eq!(bucket, "my_bucket");
|
assert_eq!(bucket, "my_bucket");
|
||||||
assert!(key.is_none());
|
assert!(key.is_none());
|
||||||
let (bucket, key) = parse_bucket_key("/my_bucket", None, None)?;
|
let (bucket, key) = parse_bucket_key("/my_bucket", None)?;
|
||||||
assert_eq!(bucket, "my_bucket");
|
assert_eq!(bucket, "my_bucket");
|
||||||
assert!(key.is_none());
|
assert!(key.is_none());
|
||||||
Ok(())
|
Ok(())
|
||||||
|
@ -323,74 +323,30 @@ mod tests {
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn parse_bucket_containing_no_bucket() {
|
fn parse_bucket_containing_no_bucket() {
|
||||||
let parsed = parse_bucket_key("", None, None);
|
let parsed = parse_bucket_key("", None);
|
||||||
assert!(parsed.is_err());
|
assert!(parsed.is_err());
|
||||||
let parsed = parse_bucket_key("/", None, None);
|
let parsed = parse_bucket_key("/", None);
|
||||||
assert!(parsed.is_err());
|
assert!(parsed.is_err());
|
||||||
let parsed = parse_bucket_key("////", None, None);
|
let parsed = parse_bucket_key("////", None);
|
||||||
assert!(parsed.is_err());
|
assert!(parsed.is_err());
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn parse_bucket_with_vhost_and_key() -> Result<(), Error> {
|
fn parse_bucket_with_vhost_and_key() -> Result<(), Error> {
|
||||||
let (bucket, key) = parse_bucket_key(
|
let (bucket, key) = parse_bucket_key("/a/super/file.jpg", Some("my-bucket"))?;
|
||||||
"/a/super/file.jpg",
|
|
||||||
Some("my-bucket.garage.tld"),
|
|
||||||
Some("garage.tld"),
|
|
||||||
)?;
|
|
||||||
assert_eq!(bucket, "my-bucket");
|
assert_eq!(bucket, "my-bucket");
|
||||||
assert_eq!(key.expect("key must be set"), "a/super/file.jpg");
|
assert_eq!(key.expect("key must be set"), "a/super/file.jpg");
|
||||||
|
|
||||||
let (bucket, key) = parse_bucket_key(
|
|
||||||
"/my_bucket/a/super/file.jpg",
|
|
||||||
Some("not-garage.tld"),
|
|
||||||
Some("garage.tld"),
|
|
||||||
)?;
|
|
||||||
assert_eq!(bucket, "my_bucket");
|
|
||||||
assert_eq!(key.expect("key must be set"), "a/super/file.jpg");
|
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn parse_bucket_with_vhost_no_key() -> Result<(), Error> {
|
fn parse_bucket_with_vhost_no_key() -> Result<(), Error> {
|
||||||
let (bucket, key) = parse_bucket_key("", Some("my-bucket.garage.tld"), Some("garage.tld"))?;
|
let (bucket, key) = parse_bucket_key("", Some("my-bucket"))?;
|
||||||
assert_eq!(bucket, "my-bucket");
|
assert_eq!(bucket, "my-bucket");
|
||||||
assert!(key.is_none());
|
assert!(key.is_none());
|
||||||
let (bucket, key) =
|
let (bucket, key) = parse_bucket_key("/", Some("my-bucket"))?;
|
||||||
parse_bucket_key("/", Some("my-bucket.garage.tld"), Some("garage.tld"))?;
|
|
||||||
assert_eq!(bucket, "my-bucket");
|
assert_eq!(bucket, "my-bucket");
|
||||||
assert!(key.is_none());
|
assert!(key.is_none());
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn parse_bucket_missmatch_vhost() {
|
|
||||||
let test_vec = [
|
|
||||||
"/my_bucket/a/super/file.jpg",
|
|
||||||
"/my_bucket/",
|
|
||||||
"/my_bucket",
|
|
||||||
"",
|
|
||||||
"/",
|
|
||||||
"////",
|
|
||||||
];
|
|
||||||
let eq = |l, r| match (l, r) {
|
|
||||||
(Ok(l), Ok(r)) => l == r,
|
|
||||||
(Err(_), Err(_)) => true,
|
|
||||||
_ => false,
|
|
||||||
};
|
|
||||||
for test in test_vec {
|
|
||||||
assert!(eq(
|
|
||||||
parse_bucket_key(test, None, None),
|
|
||||||
parse_bucket_key(test, Some("bucket.garage.tld"), None)
|
|
||||||
));
|
|
||||||
assert!(eq(
|
|
||||||
parse_bucket_key(test, None, None),
|
|
||||||
parse_bucket_key(test, None, Some("garage.tld"))
|
|
||||||
));
|
|
||||||
assert!(eq(
|
|
||||||
parse_bucket_key(test, None, None),
|
|
||||||
parse_bucket_key(test, Some("not-garage.tld"), Some("garage.tld"))
|
|
||||||
));
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue