Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
greenx opened this issue Apr 7, 2023 · 12 comments · Fixed by #1803
Open

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

greenx opened this issue Apr 7, 2023 · 12 comments · Fixed by #1803

Comments

@greenx
Copy link

greenx commented Apr 7, 2023

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 klauspost transferred this issue from minio/warp Apr 7, 2023
klauspost added a commit to klauspost/minio-go that referenced this issue Apr 7, 2023
@klauspost
Copy link
Contributor

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

@greenx
Copy link
Author

greenx commented Apr 10, 2023

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
Copy link
Contributor

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

@greenx
Copy link
Author

greenx commented Apr 10, 2023

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

@greenx
Copy link
Author

greenx commented Apr 11, 2023

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
Copy link
Member

harshavardhana commented Apr 11, 2023

@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
Copy link
Author

greenx commented Apr 11, 2023

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
Copy link
Contributor

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

@VladimirTregubenko
Copy link

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
Copy link
Member

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.

@VladimirTregubenko
Copy link

It's funny - on our project we replaced github.com/minio/minio-go/v7 with github.com/aws/aws-sdk-go-v2 to continue working with GCP storage which doesn't support proper AWS S3 semantics.

@UladzimirTrehubenka
Copy link

BTW GCP is working for us with v7.0.61 (even with SignatureV2) but not with v7.0.62,
where SignatureV2 was replaced with SignatureV4 for GCP (which uses StreamingSignV4) in the PR #1870.
But with latest minio-go (48517d8) finally I got it working with change our code
like this (added TrailingHeaders: false and DisableContentSha256: true)

func NewBucket(cfg *BucketCfg) (*bucket, error) {
	client, err := minio.New(cfg.Endpoint, &minio.Options{
		Creds:  credentials.NewStaticV4(cfg.AccessKeyID, cfg.SecretAccessKey, ""),
		Secure: false, TrailingHeaders: false,
	})
	return &bucket{client: client, bucketName: cfg.BucketName}, err
}

func (b *bucket) PutData(ctx context.Context, data []byte, path, mime string) error {
	size := len(data)
	_, err := b.client.PutObject(ctx, b.bucketName, path, bytes.NewReader(data),
		int64(size), minio.PutObjectOptions{ContentType: mime, DisableContentSha256: true})
	return err
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants