avoid handling status_code-related logic in api/s3/get.rs

src/api/s3/get.rs

@@ -163,15 +163,7 @@ pub async fn handle_head(
 	key: &str,
 	part_number: Option<u64>,
 ) -> Result<Response<ResBody>, Error> {
-	handle_head_without_ctx(
+	handle_head_without_ctx(ctx.garage, req, ctx.bucket_id, key, part_number).await
-		ctx.garage,
-		req,
-		ctx.bucket_id,
-		key,
-		StatusCode::OK,
-		part_number,
-	)
-	.await
 }
 
 /// Handle HEAD request for website
@@ -180,7 +172,6 @@ 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
@@ -281,7 +272,7 @@ pub async fn handle_head_without_ctx(
 			checksum_mode,
 		)
 		.header(CONTENT_LENGTH, format!("{}", version_meta.size))
-		.status(status_code)
+		.status(StatusCode::OK)
 		.body(empty_body())?)
 	}
 }
@@ -294,16 +285,7 @@ pub async fn handle_get(
 	part_number: Option<u64>,
 	overrides: GetObjectOverrides,
 ) -> Result<Response<ResBody>, Error> {
-	handle_get_without_ctx(
+	handle_get_without_ctx(ctx.garage, req, ctx.bucket_id, key, part_number, overrides).await
-		ctx.garage,
-		req,
-		ctx.bucket_id,
-		key,
-		StatusCode::OK,
-		part_number,
-		overrides,
-	)
-	.await
 }
 
 /// Handle GET request
@@ -312,7 +294,6 @@ 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> {
@@ -348,15 +329,11 @@ pub async fn handle_get_without_ctx(
 
 	let checksum_mode = checksum_mode(&req);
 
-	match (
+	match (part_number, parse_range_header(req, last_v_meta.size)?) {
-		part_number,
+		(Some(_), Some(_)) => Err(Error::bad_request(
-		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, true) => {
+		(Some(pn), None) => {
 			handle_get_part(
 				garage,
 				last_v,
@@ -369,7 +346,7 @@ pub async fn handle_get_without_ctx(
 			)
 			.await
 		}
-		(None, Some(range), true) => {
+		(None, Some(range)) => {
 			handle_get_range(
 				garage,
 				last_v,
@@ -383,8 +360,7 @@ 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,
@@ -394,7 +370,6 @@ pub async fn handle_get_without_ctx(
 				&headers,
 				overrides,
 				checksum_mode,
-				status_code,
 			)
 			.await
 		}
@@ -410,7 +385,6 @@ 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,
@@ -420,7 +394,7 @@ async fn handle_get_full(
 		checksum_mode,
 	)
 	.header(CONTENT_LENGTH, format!("{}", version_meta.size))
-	.status(status_code);
+	.status(StatusCode::OK);
 	getobject_override_headers(overrides, &mut resp_builder)?;
 
 	let stream = full_object_byte_stream(garage, version, version_data, encryption);

src/web/web_server.rs

@@ -6,6 +6,7 @@ use tokio::net::{TcpListener, UnixListener};
 use tokio::sync::watch;
 
 use hyper::{
+	body::Body,
 	body::Incoming as IncomingBody,
 	header::{HeaderValue, HOST},
 	Method, Request, Response, StatusCode,
@@ -22,6 +23,7 @@ use crate::error::*;
 
 use garage_api::generic_server::{server_loop, UnixListenerOn};
 use garage_api::helpers::*;
+use garage_api::s3::api_server::ResBody;
 use garage_api::s3::cors::{add_cors_headers, find_matching_cors_rule, handle_options_for_bucket};
 use garage_api::s3::error::{
 	CommonErrorDerivative, Error as ApiError, OkOrBadRequest, OkOrInternalError,
@@ -252,19 +254,10 @@ impl WebServer {
 				.body(empty_body())
 				.unwrap()),
 			(&Method::HEAD, Ok((key, code))) => {
-				handle_head_without_ctx(self.garage.clone(), req, bucket_id, key, code, None).await
+				handle_head(self.garage.clone(), req, bucket_id, key, code).await
 			}
 			(&Method::GET, Ok((key, code))) => {
-				handle_get_without_ctx(
+				handle_get(self.garage.clone(), req, bucket_id, key, code).await
-					self.garage.clone(),
-					req,
-					bucket_id,
-					key,
-					code,
-					None,
-					Default::default(),
-				)
-				.await
 			}
 			_ => Err(ApiError::bad_request("HTTP method not supported")),
 		};
@@ -306,25 +299,22 @@ impl WebServer {
 			) => {
 				match *req.method() {
 					Method::HEAD => {
-						handle_head_without_ctx(
+						handle_head(
 							self.garage.clone(),
 							req,
 							bucket_id,
 							redirect_key,
 							*redirect_code,
-							None,
 						)
 						.await
 					}
 					Method::GET => {
-						handle_get_without_ctx(
+						handle_get(
 							self.garage.clone(),
 							req,
 							bucket_id,
 							redirect_key,
 							*redirect_code,
-							None,
-							Default::default(),
 						)
 						.await
 					}
@@ -361,14 +351,12 @@ impl WebServer {
 					.body(empty_body::<Infallible>())
 					.unwrap();
 
-				match handle_get_without_ctx(
+				match handle_get(
 					self.garage.clone(),
 					&req2,
 					bucket_id,
 					&error_document,
 					error.http_status_code(),
-					None,
-					Default::default(),
 				)
 				.await
 				{
@@ -413,6 +401,63 @@ impl WebServer {
 	}
 }
 
+async fn handle_head(
+	garage: Arc<Garage>,
+	req: &Request<impl Body>,
+	bucket_id: Uuid,
+	key: &str,
+	status_code: StatusCode,
+) -> Result<Response<ResBody>, ApiError> {
+	if status_code != StatusCode::OK {
+		// See comment in handle_get
+		let cleaned_req = Request::builder()
+			.uri(req.uri())
+			.body(empty_body::<Infallible>())
+			.unwrap();
+
+		let mut ret = handle_head_without_ctx(garage, &cleaned_req, bucket_id, key, None).await?;
+		*ret.status_mut() = status_code;
+		Ok(ret)
+	} else {
+		handle_head_without_ctx(garage, req, bucket_id, key, None).await
+	}
+}
+
+pub async fn handle_get(
+	garage: Arc<Garage>,
+	req: &Request<impl Body>,
+	bucket_id: Uuid,
+	key: &str,
+	status_code: StatusCode,
+) -> Result<Response<ResBody>, ApiError> {
+	if status_code != StatusCode::OK {
+		// If we are returning an error document, discard all headers from
+		// the original GET request that would have influenced the result:
+		// - Range header, we don't want to return a subrange of the error document
+		// - Caching directives such as If-None-Match, etc, which are not relevant
+		let cleaned_req = Request::builder()
+			.uri(req.uri())
+			.body(empty_body::<Infallible>())
+			.unwrap();
+
+		let mut ret = handle_get_without_ctx(
+			garage,
+			&cleaned_req,
+			bucket_id,
+			key,
+			None,
+			Default::default(),
+		)
+		.await?;
+
+		*ret.status_mut() = status_code;
+
+		Ok(ret)
+	} else {
+		handle_get_without_ctx(garage, req, bucket_id, key, None, Default::default()).await
+	}
+}
+
 fn error_to_res(e: Error) -> Response<BoxBody<Error>> {
 	// If we are here, it is either that:
 	// - there was an error before trying to get the requested URL