Handle part_number in HeadObject and GetObject (#191) #192

					
				@ -55,2 +55,4 @@

					BucketNotEmpty,

					/// Parts specified in CMU request do not match parts actually uploaded

					#[error(display = "Parts given to CompleteMultipartUpload do not match uploade dparts")]

trinity-1686a commented

2022-01-13 16:12:18 +00:00

typo

src/api/s3_get.rs Outdated

					
				@ -121,0 +123,4 @@

						if let ObjectVersionData::Inline(_, _) = version_data {

							// Not a multipart upload

							return Err(Error::BadRequest(

								"Cannt process part_number argument: not a multipart upload".into(),

trinity-1686a commented

2022-01-13 16:12:27 +00:00

typo

src/api/s3_put.rs Outdated

					
				@ -446,0 +454,4 @@

						.iter()

						.zip(body_list_of_parts.iter().skip(1))

						.all(|(p1, p2)| p1.part_number + 1 == p2.part_number)

					{

trinity-1686a commented

2022-01-13 16:19:43 +00:00

to verify, but I think this is stricter than AWS : I believe I've read somewhere it's possible to have holes between part number, so 2, 4, 6 would be a correct sequence. Will try against AWS when I have time

to verify, but I think this is stricter than AWS : I believe I've read somewhere it's possible to have holes between part number, so `2, 4, 6` would be a correct sequence. Will try against AWS when I have time

👍 1

trinity-1686a commented

2022-01-15 12:15:18 +00:00

I can confirm AWS accept providing the part number list 2, 4, so it neither require starting from 1, nor be sequential, only ordered and between 1 and 10000

I can confirm AWS accept providing the part number list `2, 4`, so it neither require starting from 1, nor be sequential, only ordered and between 1 and 10000

👍 1

lx force-pushed get-head-part-number from f23b61a7f3 to 3c5945a585

2022-01-17 15:19:22 +00:00

Compare

lx force-pushed get-head-part-number from 3c5945a585 to 26a7309839

2022-01-18 11:16:35 +00:00

Compare

lx force-pushed get-head-part-number from 26a7309839 to 43c27eef9b

2022-01-18 11:18:50 +00:00

Compare

lx force-pushed get-head-part-number from 43c27eef9b to 0c7834bfa7

2022-01-24 11:21:44 +00:00

Compare

quentin approved these changes 2022-01-24 17:56:10 +00:00

lx force-pushed get-head-part-number from 0c7834bfa7 to 338b1b83ee

2022-01-24 20:04:45 +00:00

Compare

lx merged commit 338b1b83ee into main

2022-01-24 21:00:34 +00:00

trinity-1686a approved these changes 2022-01-24 21:33:39 +00:00

trinity-1686a left a comment

multiple small diffences with AWS. I wouldn't consider them blocking, if you want to merge this quiclky, just open an issue for them.

Overall, lgtm

multiple small diffences with AWS. I wouldn't consider them blocking, if you want to merge this quiclky, just open an issue for them. Overall, lgtm

src/api/s3_get.rs

					
				@ -121,0 +123,4 @@

						if let ObjectVersionData::Inline(_, _) = version_data {

							// Not a multipart upload

							return Err(Error::BadRequest(

								"Cannot process part_number argument: not a multipart upload".into(),

trinity-1686a commented

2022-01-24 21:09:56 +00:00

minor missmatch with AWS : if not multipart, AWS consider everything is contained in part 1.

src/api/s3_get.rs

					
				@ -121,0 +151,4 @@

						Ok(object_headers(object_version, version_meta)

							.header(CONTENT_LENGTH, format!("{}", part_size))

							.header("x-amz-mp-parts-count", format!("{}", n_parts))

							.status(StatusCode::OK)

trinity-1686a commented

2022-01-24 21:29:07 +00:00

AWS returns byte range headers and HTTP 206 (Partial Content) as if it was a range request

Accept-Ranges: bytes
Content-Range: bytes 0-147/148

AWS returns byte range headers and HTTP 206 (Partial Content) as if it was a range request ``` Accept-Ranges: bytes Content-Range: bytes 0-147/148 ```

src/api/s3_get.rs

					
				@ -316,0 +319,4 @@

							.ok_or(Error::NoSuchKey)?

					} else {

						return Err(Error::BadRequest(

							"Cannot handle part_number: not a multipart upload.".into(),

trinity-1686a commented

2022-01-24 21:16:41 +00:00

Same as above

src/api/s3_get.rs

					
				@ -316,0 +337,4 @@

					let part_size = blocks.iter().map(|(_, b)| b.size).sum();

					if let Some(r) = parse_range_header(req, part_size)? {

trinity-1686a commented

2022-01-24 21:23:40 +00:00

AWS does not support this kind of request (InvalidRequest: Cannot specify both Range header and partNumber query parameter).
I'm not sure if we should support it, S3 libraries won't ever call that, and it's additional code to maitain. On the other hand, it's not a lot of code, so this message is purely informative, and not necessary requiring any changes

AWS does not support this kind of request (InvalidRequest: Cannot specify both Range header and partNumber query parameter). I'm not sure if we should support it, S3 libraries won't ever call that, and it's additional code to maitain. On the other hand, it's not a lot of code, so this message is purely informative, and not necessary requiring any changes

src/api/s3_get.rs

					
				@ -316,0 +355,4 @@

						Ok(object_headers(object_version, version_meta)

							.header(CONTENT_LENGTH, format!("{}", part_size))

							.status(StatusCode::OK)