Many S3 compatibility improvements:

- return XML errors - implement AuthorizationHeaderMalformed error to redirect clients to correct location (used by minio client) - implement GetBucketLocation - fix DeleteObjects XML parsing and response
Merge pull request 'Correctly encode ampersand' (#61 ) from bug/ampersand into main
2026-08-14 16:13:11 +00:00 · 2021-04-28 01:05:40 +02:00 · 2021-04-27 23:15:01 +02:00 · 2021-04-27 23:10:43 +02:00 · 2021-04-27 23:09:26 +02:00
8 changed files with 123 additions and 34 deletions
@@ -3,6 +3,8 @@
 set -ex
 shopt -s expand_aliases

+export LC_ALL=C.UTF-8
+export LANG=C.UTF-8
 SCRIPT_FOLDER="`dirname \"$0\"`"
 REPO_FOLDER="${SCRIPT_FOLDER}/../"

@@ -19,41 +21,45 @@ garage status
 garage key list
 garage bucket list

-dd if=/dev/urandom of=/tmp/garage.1.rnd bs=1k count=2 # < INLINE_THRESHOLD = 3072 bytes
-dd if=/dev/urandom of=/tmp/garage.2.rnd bs=1M count=5
-dd if=/dev/urandom of=/tmp/garage.3.rnd bs=1M count=10
+dd if=/dev/urandom of=/tmp/garage.1.rnd bs=1k count=2 # No multipart, inline storage (< INLINE_THRESHOLD = 3072 bytes)
+dd if=/dev/urandom of=/tmp/garage.2.rnd bs=1M count=5 # No multipart but file will be chunked
+dd if=/dev/urandom of=/tmp/garage.3.rnd bs=1M count=10 # by default, AWS starts using multipart at 8MB

 echo "s3 api testing..."
+awsgrg --version
+s3cmd --version
+python3 --version
+
 for idx in $(seq 1 3); do
-  # AWS sends	
-  awsgrg cp /tmp/garage.$idx.rnd s3://eprouvette/garage.$idx.aws
-  
+  # AWS sends
+  awsgrg cp "/tmp/garage.$idx.rnd" "s3://eprouvette/&+-é\"/garage.$idx.aws"
+
  awsgrg ls s3://eprouvette
-  
-  awsgrg cp s3://eprouvette/garage.$idx.aws /tmp/garage.$idx.dl
+
+  awsgrg cp "s3://eprouvette/&+-é\"/garage.$idx.aws" "/tmp/garage.$idx.dl"
  diff /tmp/garage.$idx.rnd /tmp/garage.$idx.dl
  rm /tmp/garage.$idx.dl
-  
-  s3grg get s3://eprouvette/garage.$idx.aws /tmp/garage.$idx.dl
+
+  s3grg get "s3://eprouvette/&+-é\"/garage.$idx.aws" "/tmp/garage.$idx.dl"
  diff /tmp/garage.$idx.rnd /tmp/garage.$idx.dl
  rm /tmp/garage.$idx.dl
-  
-  awsgrg rm s3://eprouvette/garage.$idx.aws
+
+  awsgrg rm "s3://eprouvette/&+-é\"/garage.$idx.aws"

  # S3CMD sends
-  s3grg put /tmp/garage.$idx.rnd s3://eprouvette/garage.$idx.s3cmd
+  s3grg put "/tmp/garage.$idx.rnd" "s3://eprouvette/&+-é\"/garage.$idx.s3cmd"

  s3grg ls s3://eprouvette
 	
-  s3grg get s3://eprouvette/garage.$idx.s3cmd /tmp/garage.$idx.dl
+  s3grg get "s3://eprouvette/&+-é\"/garage.$idx.s3cmd" "/tmp/garage.$idx.dl"
  diff /tmp/garage.$idx.rnd /tmp/garage.$idx.dl
  rm /tmp/garage.$idx.dl
-  
-  awsgrg cp s3://eprouvette/garage.$idx.s3cmd /tmp/garage.$idx.dl
+
+  awsgrg cp "s3://eprouvette/&+-é\"/garage.$idx.s3cmd" "/tmp/garage.$idx.dl"
  diff /tmp/garage.$idx.rnd /tmp/garage.$idx.dl
  rm /tmp/garage.$idx.dl
 	
-  s3grg rm s3://eprouvette/garage.$idx.s3cmd
+  s3grg rm "s3://eprouvette/&+-é\"/garage.$idx.s3cmd"
 done
 rm /tmp/garage.{1,2,3}.rnd

@@ -14,6 +14,7 @@ use garage_model::garage::Garage;
 use crate::error::*;
 use crate::signature::check_signature;

+use crate::s3_bucket::*;
 use crate::s3_copy::*;
 use crate::s3_delete::*;
 use crate::s3_get::*;
@@ -52,17 +53,21 @@ async fn handler(
 	req: Request<Body>,
 	addr: SocketAddr,
 ) -> Result<Response<Body>, GarageError> {
-	info!("{} {} {}", addr, req.method(), req.uri());
+	let uri = req.uri().clone();
+	info!("{} {} {}", addr, req.method(), uri);
 	debug!("{:?}", req);
-	match handler_inner(garage, req).await {
+	match handler_inner(garage.clone(), req).await {
 		Ok(x) => {
 			debug!("{} {:?}", x.status(), x.headers());
 			Ok(x)
 		}
 		Err(e) => {
-			let body: Body = Body::from(format!("{}\n", e));
-			let mut http_error = Response::new(body);
-			*http_error.status_mut() = e.http_status_code();
+			let body: Body = Body::from(e.aws_xml(&garage.config.s3_api.s3_region, uri.path()));
+			let http_error = Response::builder()
+				.status(e.http_status_code())
+				.header("Content-Type", "application/xml")
+				.body(body)?;
+
 			if e.http_status_code().is_server_error() {
 				warn!("Response: error {}, {}", e.http_status_code(), e);
 			} else {
@@ -211,9 +216,14 @@ async fn handler_inner(garage: Arc<Garage>, req: Request<Body>) -> Result<Respon
 				))
 			}
 			&Method::GET => {
-				// ListObjects or ListObjectsV2 query
-				let q = parse_list_objects_query(bucket, &params)?;
-				Ok(handle_list(garage, &q).await?)
+				if params.contains_key("location") {
+					// GetBucketLocation call
+					Ok(handle_get_bucket_location(garage)?)
+				} else {
+					// ListObjects or ListObjectsV2 query
+					let q = parse_list_objects_query(bucket, &params)?;
+					Ok(handle_list(garage, &q).await?)
+				}
 			}
 			&Method::POST => {
 				if params.contains_key(&"delete".to_string()) {
@@ -2,7 +2,8 @@

 /// Escape &str for xml inclusion
 pub fn xml_escape(s: &str) -> String {
-	s.replace("<", "&lt;")
+	s.replace("&", "&amp;")
+		.replace("<", "&lt;")
 		.replace(">", "&gt;")
 		.replace("\"", "&quot;")
 }
@@ -1,8 +1,12 @@
+use std::fmt::Write;
+
 use err_derive::Error;
 use hyper::StatusCode;

 use garage_util::error::Error as GarageError;

+use crate::encoding::*;
+
 /// Errors of this crate
 #[derive(Debug, Error)]
 pub enum Error {
@@ -24,6 +28,10 @@ pub enum Error {
 	#[error(display = "Forbidden: {}", _0)]
 	Forbidden(String),

+	/// Authorization Header Malformed
+	#[error(display = "Authorization header malformed, expected scope: {}", _0)]
+	AuthorizationHeaderMalformed(String),
+
 	/// The object requested don't exists
 	#[error(display = "Not found")]
 	NotFound,
@@ -77,6 +85,29 @@ impl Error {
 			_ => StatusCode::BAD_REQUEST,
 		}
 	}
+
+	pub fn aws_code(&self) -> &'static str {
+		match self {
+			Error::NotFound => "NoSuchKey",
+			Error::Forbidden(_) => "AccessDenied",
+			Error::AuthorizationHeaderMalformed(_) => "AuthorizationHeaderMalformed",
+			Error::InternalError(GarageError::RPC(_)) => "ServiceUnavailable",
+			Error::InternalError(_) | Error::Hyper(_) | Error::HTTP(_) => "InternalError",
+			_ => "InvalidRequest",
+		}
+	}
+
+	pub fn aws_xml(&self, garage_region: &str, path: &str) -> String {
+		let mut xml = String::new();
+		writeln!(&mut xml, r#"<?xml version="1.0" encoding="UTF-8"?>"#).unwrap();
+		writeln!(&mut xml, "<Error>").unwrap();
+		writeln!(&mut xml, "\t<Code>{}</Code>", self.aws_code()).unwrap();
+		writeln!(&mut xml, "\t<Message>{}</Message>", self).unwrap();
+		writeln!(&mut xml, "\t<Resource>{}</Resource>", xml_escape(path)).unwrap();
+		writeln!(&mut xml, "\t<Region>{}</Region>", garage_region).unwrap();
+		writeln!(&mut xml, "</Error>").unwrap();
+		xml
+	}
 }

 /// Trait to map error to the Bad Request error code
@@ -12,6 +12,7 @@ pub use api_server::run_api_server;

 mod signature;

+mod s3_bucket;
 mod s3_copy;
 mod s3_delete;
 pub mod s3_get;
@@ -0,0 +1,24 @@
+use std::fmt::Write;
+use std::sync::Arc;
+
+use hyper::{Body, Response};
+
+use garage_model::garage::Garage;
+
+use crate::error::*;
+
+pub fn handle_get_bucket_location(garage: Arc<Garage>) -> Result<Response<Body>, Error> {
+	let mut xml = String::new();
+
+	writeln!(&mut xml, r#"<?xml version="1.0" encoding="UTF-8"?>"#).unwrap();
+	writeln!(
+		&mut xml,
+		r#"<LocationConstraint xmlns="http://s3.amazonaws.com/doc/2006-03-01/">{}</LocationConstraint>"#,
+		garage.config.s3_api.s3_region
+	)
+	.unwrap();
+
+	Ok(Response::builder()
+		.header("Content-Type", "application/xml")
+		.body(Body::from(xml.into_bytes()))?)
+}
@@ -85,11 +85,18 @@ pub async fn handle_delete_objects(

 	let mut retxml = String::new();
 	writeln!(&mut retxml, r#"<?xml version="1.0" encoding="UTF-8"?>"#).unwrap();
-	writeln!(&mut retxml, "<DeleteObjectsOutput>").unwrap();
+	writeln!(
+		&mut retxml,
+		r#"<DeleteResult xmlns="http://s3.amazonaws.com/doc/2006-03-01/">"#
+	)
+	.unwrap();

 	for obj in cmd.objects.iter() {
 		match handle_delete_internal(&garage, bucket, &obj.key).await {
 			Ok((deleted_version, delete_marker_version)) => {
+				if cmd.quiet {
+					continue;
+				}
 				writeln!(&mut retxml, "\t<Deleted>").unwrap();
 				writeln!(&mut retxml, "\t\t<Key>{}</Key>", xml_escape(&obj.key)).unwrap();
 				writeln!(
@@ -121,7 +128,7 @@ pub async fn handle_delete_objects(
 		}
 	}

-	writeln!(&mut retxml, "</DeleteObjectsOutput>").unwrap();
+	writeln!(&mut retxml, "</DeleteResult>").unwrap();

 	Ok(Response::builder()
 		.header("Content-Type", "application/xml")
@@ -129,6 +136,7 @@ pub async fn handle_delete_objects(
 }

 struct DeleteRequest {
+	quiet: bool,
 	objects: Vec<DeleteObject>,
 }

@@ -137,7 +145,10 @@ struct DeleteObject {
 }

 fn parse_delete_objects_xml(xml: &roxmltree::Document) -> Option<DeleteRequest> {
-	let mut ret = DeleteRequest { objects: vec![] };
+	let mut ret = DeleteRequest {
+		quiet: false,
+		objects: vec![],
+	};

 	let root = xml.root();
 	let delete = root.first_child()?;
@@ -153,6 +164,12 @@ fn parse_delete_objects_xml(xml: &roxmltree::Document) -> Option<DeleteRequest>
 			ret.objects.push(DeleteObject {
 				key: key_str.to_string(),
 			});
+		} else if item.has_tag_name("Quiet") {
+			if item.text()? == "true" {
+				ret.quiet = true;
+			} else {
+				ret.quiet = false;
+			}
 		} else {
 			return None;
 		}
@@ -58,10 +58,7 @@ pub async fn check_signature(
 		garage.config.s3_api.s3_region
 	);
 	if authorization.scope != scope {
-		return Err(Error::BadRequest(format!(
-			"Invalid scope in authorization field, expected: {}",
-			scope
-		)));
+		return Err(Error::AuthorizationHeaderMalformed(scope.to_string()));
 	}

 	let key = garage
@@ -101,7 +98,9 @@ pub async fn check_signature(
 		return Err(Error::Forbidden(format!("Invalid signature")));
 	}

-	let content_sha256 = if authorization.content_sha256 == "UNSIGNED-PAYLOAD" {
+	let content_sha256 = if authorization.content_sha256 == "UNSIGNED-PAYLOAD"
+		|| authorization.content_sha256 == "STREAMING-AWS4-HMAC-SHA256-PAYLOAD"
+	{
 		None
 	} else {
 		let bytes = hex::decode(authorization.content_sha256)
Author	SHA1	Message	Date
Alex Auvolat	dcfc32cf85	Many S3 compatibility improvements: - return XML errors - implement AuthorizationHeaderMalformed error to redirect clients to correct location (used by minio client) - implement GetBucketLocation - fix DeleteObjects XML parsing and response	2021-04-28 01:05:40 +02:00
Alex	368eb35484	Merge pull request 'Correctly encode ampersand' (#61 ) from bug/ampersand into main Reviewed-on: https://git.deuxfleurs.fr/Deuxfleurs/garage/pulls/61	2021-04-27 23:15:01 +02:00
Alex Auvolat	642186c530	Fix #59 (& issue)	2021-04-27 23:10:43 +02:00
Quentin Dufour	e01f74e763	Introduce test case that demonstrates #59 (the & problem)	2021-04-27 23:09:26 +02:00