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-16 00:43: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.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
+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
+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	
+  # AWS sends
-  awsgrg cp /tmp/garage.$idx.rnd s3://eprouvette/garage.$idx.aws
+  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 body: Body = Body::from(e.aws_xml(&garage.config.s3_api.s3_region, uri.path()));
-			let mut http_error = Response::new(body);
+			let http_error = Response::builder()
-			*http_error.status_mut() = e.http_status_code();
+				.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
+				if params.contains_key("location") {
-				let q = parse_list_objects_query(bucket, &params)?;
+					// GetBucketLocation call
-				Ok(handle_list(garage, &q).await?)
+					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!(
+		return Err(Error::AuthorizationHeaderMalformed(scope.to_string()));
 			"Invalid scope in authorization field, expected: {}",
 			scope
 		)));
 	}
 	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