support redirection on s3 endpoint

2 files changed, 138 insertions, 37 deletions

diff --git a/src/api/s3/get.rs b/src/api/s3/get.rs
index f5d3cf11..eea3434e 100644
--- a/src/api/s3/get.rs
+++ b/src/api/s3/get.rs
@@ -163,7 +163,15 @@ pub async fn handle_head(
 	key: &str,
 	part_number: Option<u64>,
 ) -> Result<Response<ResBody>, Error> {
-	handle_head_without_ctx(ctx.garage, req, ctx.bucket_id, key, part_number).await
+	handle_head_without_ctx(
+		ctx.garage,
+		req,
+		ctx.bucket_id,
+		key,
+		StatusCode::OK,
+		part_number,
+	)
+	.await
 }
 
 /// Handle HEAD request for website
@@ -172,6 +180,7 @@ pub async fn handle_head_without_ctx(
 	req: &Request<impl Body>,
 	bucket_id: Uuid,
 	key: &str,
+	status_code: StatusCode,
 	part_number: Option<u64>,
 ) -> Result<Response<ResBody>, Error> {
 	let object = garage
@@ -272,7 +281,7 @@ pub async fn handle_head_without_ctx(
 			checksum_mode,
 		)
 		.header(CONTENT_LENGTH, format!("{}", version_meta.size))
-		.status(StatusCode::OK)
+		.status(status_code)
 		.body(empty_body())?)
 	}
 }
@@ -285,7 +294,16 @@ pub async fn handle_get(
 	part_number: Option<u64>,
 	overrides: GetObjectOverrides,
 ) -> Result<Response<ResBody>, Error> {
-	handle_get_without_ctx(ctx.garage, req, ctx.bucket_id, key, part_number, overrides).await
+	handle_get_without_ctx(
+		ctx.garage,
+		req,
+		ctx.bucket_id,
+		key,
+		StatusCode::OK,
+		part_number,
+		overrides,
+	)
+	.await
 }
 
 /// Handle GET request
@@ -294,6 +312,7 @@ pub async fn handle_get_without_ctx(
 	req: &Request<impl Body>,
 	bucket_id: Uuid,
 	key: &str,
+	status_code: StatusCode,
 	part_number: Option<u64>,
 	overrides: GetObjectOverrides,
 ) -> Result<Response<ResBody>, Error> {
@@ -329,11 +348,15 @@ pub async fn handle_get_without_ctx(
 
 	let checksum_mode = checksum_mode(&req);
 
-	match (part_number, parse_range_header(req, last_v_meta.size)?) {
-		(Some(_), Some(_)) => Err(Error::bad_request(
+	match (
+		part_number,
+		parse_range_header(req, last_v_meta.size)?,
+		status_code == StatusCode::OK,
+	) {
+		(Some(_), Some(_), _) => Err(Error::bad_request(
 			"Cannot specify both partNumber and Range header",
 		)),
-		(Some(pn), None) => {
+		(Some(pn), None, true) => {
 			handle_get_part(
 				garage,
 				last_v,
@@ -346,7 +369,7 @@ pub async fn handle_get_without_ctx(
 			)
 			.await
 		}
-		(None, Some(range)) => {
+		(None, Some(range), true) => {
 			handle_get_range(
 				garage,
 				last_v,
@@ -360,7 +383,8 @@ pub async fn handle_get_without_ctx(
 			)
 			.await
 		}
-		(None, None) => {
+		_ => {
+			// either not a range, or an error request: always return the full doc
 			handle_get_full(
 				garage,
 				last_v,
@@ -370,6 +394,7 @@ pub async fn handle_get_without_ctx(
 				&headers,
 				overrides,
 				checksum_mode,
+				status_code,
 			)
 			.await
 		}
@@ -385,6 +410,7 @@ async fn handle_get_full(
 	meta_inner: &ObjectVersionMetaInner,
 	overrides: GetObjectOverrides,
 	checksum_mode: ChecksumMode,
+	status_code: StatusCode,
 ) -> Result<Response<ResBody>, Error> {
 	let mut resp_builder = object_headers(
 		version,
@@ -394,7 +420,7 @@ async fn handle_get_full(
 		checksum_mode,
 	)
 	.header(CONTENT_LENGTH, format!("{}", version_meta.size))
-	.status(StatusCode::OK);
+	.status(status_code);
 	getobject_override_headers(overrides, &mut resp_builder)?;
 
 	let stream = full_object_byte_stream(garage, version, version_data, encryption);
diff --git a/src/api/s3/website.rs b/src/api/s3/website.rs
index 6af55677..934a20ff 100644
--- a/src/api/s3/website.rs
+++ b/src/api/s3/website.rs
@@ -10,7 +10,7 @@ use crate::s3::error::*;
 use crate::s3::xml::{to_xml_with_header, xmlns_tag, IntValue, Value};
 use crate::signature::verify_signed_content;
 
-use garage_model::bucket_table::*;
+use garage_model::bucket_table::{self, *};
 use garage_util::data::*;
 
 pub async fn handle_get_website(ctx: ReqCtx) -> Result<Response<ResBody>, Error> {
@@ -25,7 +25,8 @@ pub async fn handle_get_website(ctx: ReqCtx) -> Result<Response<ResBody>, Error>
 				suffix: Value(website.index_document.to_string()),
 			}),
 			redirect_all_requests_to: None,
-			routing_rules: None,
+			// TODO put the correct config here
+			routing_rules: Vec::new(),
 		};
 		let xml = to_xml_with_header(&wc)?;
 		Ok(Response::builder()
@@ -101,8 +102,12 @@ pub struct WebsiteConfiguration {
 	pub index_document: Option<Suffix>,
 	#[serde(rename = "RedirectAllRequestsTo")]
 	pub redirect_all_requests_to: Option<Target>,
-	#[serde(rename = "RoutingRules")]
-	pub routing_rules: Option<Vec<RoutingRule>>,
+	#[serde(
+		rename = "RoutingRules",
+		default,
+		skip_serializing_if = "Vec::is_empty"
+	)]
+	pub routing_rules: Vec<RoutingRule>,
 }
 
 #[derive(Debug, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
@@ -166,7 +171,7 @@ impl WebsiteConfiguration {
 		if self.redirect_all_requests_to.is_some()
 			&& (self.error_document.is_some()
 				|| self.index_document.is_some()
-				|| self.routing_rules.is_some())
+				|| !self.routing_rules.is_empty())
 		{
 			return Err(Error::bad_request(
 				"Bad XML: can't have RedirectAllRequestsTo and other fields",
@@ -181,10 +186,15 @@ impl WebsiteConfiguration {
 		if let Some(ref rart) = self.redirect_all_requests_to {
 			rart.validate()?;
 		}
-		if let Some(ref rrs) = self.routing_rules {
-			for rr in rrs {
-				rr.inner.validate()?;
-			}
+		for rr in &self.routing_rules {
+			rr.inner.validate()?;
+		}
+		if self.routing_rules.len() > 1000 {
+			// we will do linear scans, best to avoid overly long configuration. The
+			// limit was choosen arbitrarily
+			return Err(Error::bad_request(
+				"Bad XML: RoutingRules can't have more than 1000 child elements",
+			));
 		}
 
 		Ok(())
@@ -195,10 +205,12 @@ impl WebsiteConfiguration {
 			Err(Error::NotImplemented(
 				"S3 website redirects are not currently implemented in Garage.".into(),
 			))
-		} else if self.routing_rules.map(|x| !x.is_empty()).unwrap_or(false) {
-			Err(Error::NotImplemented(
-				"S3 routing rules are not currently implemented in Garage.".into(),
-			))
+			/*
+			} else if self.routing_rules.map(|x| !x.is_empty()).unwrap_or(false) {
+				Err(Error::NotImplemented(
+					"S3 routing rules are not currently implemented in Garage.".into(),
+				))
+					*/
 		} else {
 			Ok(WebsiteConfig {
 				index_document: self
@@ -206,6 +218,35 @@ impl WebsiteConfiguration {
 					.map(|x| x.suffix.0)
 					.unwrap_or_else(|| "index.html".to_string()),
 				error_document: self.error_document.map(|x| x.key.0),
+				routing_rules: self
+					.routing_rules
+					.into_iter()
+					.map(|rule| {
+						bucket_table::RoutingRule {
+							condition: rule.inner.condition.map(|condition| {
+								bucket_table::Condition {
+									http_error_code: condition.http_error_code.map(|c| c.0 as u16),
+									prefix: condition.prefix.map(|p| p.0),
+								}
+							}),
+							redirect: bucket_table::Redirect {
+								hostname: rule.inner.redirect.hostname.map(|h| h.0),
+								protocol: rule.inner.redirect.protocol.map(|p| p.0),
+								// aws default to 301, which i find punitive in case of
+								// missconfiguration (can be permanently cached on the
+								// user agent)
+								http_redirect_code: rule
+									.inner
+									.redirect
+									.http_redirect_code
+									.map(|c| c.0 as u16)
+									.unwrap_or(302),
+								replace_key_prefix: rule.inner.redirect.replace_prefix.map(|k| k.0),
+								replace_key: rule.inner.redirect.replace_full.map(|k| k.0),
+							},
+						}
+					})
+					.collect(),
 			})
 		}
 	}
@@ -248,35 +289,69 @@ impl Target {
 
 impl RoutingRuleInner {
 	pub fn validate(&self) -> Result<(), Error> {
-		let has_prefix = self
-			.condition
-			.as_ref()
-			.and_then(|c| c.prefix.as_ref())
-			.is_some();
-		self.redirect.validate(has_prefix)
+		if let Some(condition) = &self.condition {
+			condition.validate()?;
+		}
+		self.redirect.validate()
+	}
+}
+
+impl Condition {
+	pub fn validate(&self) -> Result<bool, Error> {
+		if let Some(ref error_code) = self.http_error_code {
+			// TODO do other error codes make sense? Aws only allows 4xx and 5xx
+			if error_code.0 != 404 {
+				return Err(Error::bad_request(
+					"Bad XML: HttpErrorCodeReturnedEquals must be 404 or absent",
+				));
+			}
+		}
+		Ok(self.prefix.is_some())
 	}
 }
 
 impl Redirect {
-	pub fn validate(&self, has_prefix: bool) -> Result<(), Error> {
+	pub fn validate(&self) -> Result<(), Error> {
 		if self.replace_prefix.is_some() {
 			if self.replace_full.is_some() {
 				return Err(Error::bad_request(
 					"Bad XML: both ReplaceKeyPrefixWith and ReplaceKeyWith are set",
 				));
 			}
-			if !has_prefix {
-				return Err(Error::bad_request(
-					"Bad XML: ReplaceKeyPrefixWith is set, but  KeyPrefixEquals isn't",
-				));
-			}
 		}
 		if let Some(ref protocol) = self.protocol {
 			if protocol.0 != "http" && protocol.0 != "https" {
 				return Err(Error::bad_request("Bad XML: invalid protocol"));
 			}
 		}
-		// TODO there are probably more invalide cases, but which ones?
+		if let Some(ref http_redirect_code) = self.http_redirect_code {
+			match http_redirect_code.0 {
+				// aws allows all 3xx except 300, but some are non-sensical (not modified,
+				// use proxy...)
+				301 | 302 | 303 | 307 | 308 => {
+					if self.hostname.is_none() && self.protocol.is_some() {
+						return Err(Error::bad_request(
+							"Bad XML: HostName must be set if Protocol is set",
+						));
+					}
+				}
+				// aws doesn't allow these codes, but netlify does, and it seems like a
+				// cool feature (change the page seen without changing the url shown by the
+				// user agent)
+				200 | 404 => {
+					if self.hostname.is_some() || self.protocol.is_some() {
+						// hostname would mean different bucket, protocol doesn't make
+						// sense
+						return Err(Error::bad_request(
+                                        "Bad XML: an HttpRedirectCode of 200 is not acceptable alongside HostName or Protocol",
+                                ));
+					}
+				}
+				_ => {
+					return Err(Error::bad_request("Bad XML: invalid HttpRedirectCode"));
+				}
+			}
+		}
 		Ok(())
 	}
 }
@@ -330,7 +405,7 @@ mod tests {
 				hostname: Value("garage.tld".to_owned()),
 				protocol: Some(Value("https".to_owned())),
 			}),
-			routing_rules: Some(vec![RoutingRule {
+			routing_rules: vec![RoutingRule {
 				inner: RoutingRuleInner {
 					condition: Some(Condition {
 						http_error_code: Some(IntValue(404)),
@@ -344,7 +419,7 @@ mod tests {
 						replace_full: Some(Value("fullkey".to_owned())),
 					},
 				},
-			}]),
+			}],
 		};
 		assert_eq! {
 			ref_value,