search

minio / minio-go

MinIO Go client SDK for S3 compatible object storage
https://docs.min.io/docs/golang-client-quickstart-guide.html
Apache License 2.0
2.37k stars 621 forks source link

PUT dont set "Transfer-Encoding" but send chucked data #1802

Open greenx opened 1 year ago

greenx commented 1 year ago

Hi,

I testing our new s3 compatible storadge. The PUT test passes, but the GET does not. I checked what Wasp was sending. It sends data in chunks, but does not set the appropriate header, so the data size does not match the expected one.

PUT /warp-benchmark-bucket/OdrYKa9t/1.aXrdRlomKVEntzRL.rnd HTTP/1.1
Host: 10.197.XX.XX:8084
User-Agent: MinIO (linux; amd64) minio-go/v7.0.49 warp/0.6.7
Content-Length: 128265
Authorization: AWS4-HMAC-SHA256 Credential=FhS8yEmUkfyX2q8qG1CWo1V996h8PxySc3vjUe2xcMm805NZL3twBNLRkU64jXrJkgZhSMjGSprL5AvYjSTY86asr/20230407/default/s3/aws4_request,SignedHeaders=host;x-amz-content-sha256;x-amz-date;x-amz-decoded-content-length,Signature=8e2b4193e420fd03adf0278693e4916f329e796038eca3cebc533f8b6ae54453
Content-Type: application/octet-stream
X-Amz-Content-Sha256: STREAMING-AWS4-HMAC-SHA256-PAYLOAD
X-Amz-Date: 20230407T110527Z
X-Amz-Decoded-Content-Length: 128000

14:05:27.740648 IP 10.XX.XX.XX.49366 > 10.197.XX.XX.8084: Flags [.], seq 2268:3716, ack 904, win 254, options [nop,nop,TS val 89444189 ecr 1473088765], length 1448
E...j.@.@...
L.!
.......Wy.2...|...........
.T.]W...10000;chunk-signature=b523a26f4b1f0a06357475c8d3defe3707178e7aa5a16719fc821c9452ec76b5
klauspost commented 1 year ago

Seems like AWS doesn't care. But adding it, since it should be there. See #1803

greenx commented 1 year ago

Hi,

It didn't help. I rebuild WARP with monio-go/v7.0.51 But "PUT" sends data in chunks and does not set the header "Transfer-Encoding:".

image

klauspost commented 1 year ago

@greenx Are you sure this isn't just because it isn't printed? Did you confirm this on the sever side?

greenx commented 1 year ago

I see that on the server these files are saved with a chunk header. image

greenx commented 1 year ago

There is a "content-length" in the headers, but the message body is transmitted in chunks, without the "transfer-encoding" header.

https://www.rfc-editor.org/rfc/rfc9112#section-6.2

harshavardhana commented 1 year ago

@greenx this is actually normal for this operation. The main reason is that content-length is automatically added via Go's net/http. Which looks at our input buffer to see what is the size and then automatically sets it.

That seems to invalidate the value set at req.TransferEncoding - it requires a bit of a refactor and touching some old code.

I would carefully do it if we must fix this. Since this works with MinIO and AWS our current priorities are really kind of met - it would seem like no other S3 vendor has complained either.

Unless I am mistaken at this point your server @greenx should honor what AWS S3 honors, i.e allow chunked transfer via Content-Length being set.

Just treat it as chunked via x-amz-signature-algorithm - that calls it as Streaming!

greenx commented 1 year ago

Hmmm, Judging by this, https://docs.aws.amazon.com/AmazonS3/latest/API/sigv4-streaming.html AWS just scored on the RFC. But, it says "either, or" so you can both comply with the RFC and remain S3 compatible. :-)

klauspost commented 1 year ago

Could look like some Go magic. I will look a bit deeper into it.

VladimirTregubenko commented 2 months ago

Observed against storage.googleapis.com v7.0.57 works fine, but v7.0.70 on getting file returns something like 10000;chunk-signature=fce2a10afb7ceaecfcb0010dbc5eb2dd02d4221228c3a555faf686d49f1274d8\r\n...original_data

harshavardhana commented 2 months ago

Observed against storage.googleapis.com v7.0.57 works fine, but v7.0.70 on getting file returns something like 10000;chunk-signature=fce2a10afb7ceaecfcb0010dbc5eb2dd02d4221228c3a555faf686d49f1274d8\r\n...original_data

Google doesn't support proper AWS S3 semantics. Please use this SDK against S3 compatible implementations.