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.

Sign up for GitHub

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

Jump to bottom

S3 PutObject on SDK V2 throws NotImplemented if passed http req.Body io.ReadCloser #1938

Closed
Sid-Sun opened this issue Nov 24, 2022 · 9 comments
Closed

S3 PutObject on SDK V2 throws NotImplemented if passed http req.Body io.ReadCloser #1938

Sid-Sun opened this issue Nov 24, 2022 · 9 comments
Assignees
@RanVaknin
Labels
bug This issue is a bug.

Comments

@Sid-Sun
Copy link

Sid-Sun commented Nov 24, 2022

Describe the bug

I am receiving a HTTP request whose body I want to put to an object on AWS, on SDK V1 I would just pass the request body ReadCloser to the S3 uploader upload func and it would internally upload it in multipart to S3.

When I pass it to s3 PutObject on SDK V2 I get:

operation error S3: PutObject, https response error StatusCode: 501, api error NotImplemented: A header you provided implies functionality that is not implemented

code (v2):

	_, err = sp.client.PutObject(context.Background(), &s3.PutObjectInput{
		Key:     &id,
		Body:    data,
		Bucket:  &sp.bucketname,
		ACL:     sp.acl,
	})
	if err != nil {
		return err
	}

code (v1):

	_, err = sp.uploader.Upload(&s3manager.UploadInput{
		Key:    &id,
		Body:   data,
		Bucket: &sp.bucketname,
		ACL:    &sp.acl,
	})

if I read the body and then pass the data through a reader it works perfectly fine

code (v2 - working):

	d, _ := ioutil.ReadAll(data)
	_, err = sp.client.PutObject(context.Background(), &s3.PutObjectInput{
		Key:     &id,
		Body:    bytes.NewReader(d),
		Bucket:  &sp.bucketname,
		ACL:     sp.acl,
	})
	if err != nil {
		return err
	}

Expected Behavior

AWS SDK V2 S3 PutObject should upload the data to S3 object from request's io read closer without needing to load it in memory and passing a reader over the bytes. Ideally, internally using multipart upload like SDK V1.

Current Behavior

AWS SDK V2 S3 PutObject throws an unimplemented functionality error:

operation error S3: PutObject, https response error StatusCode: 501, api error NotImplemented: A header you provided implies functionality that is not implemented

Reproduction Steps

try to use SDK V2 S3 PutObject with a read closer

Possible Solution

No response

Additional Information/Context

No response

AWS Go SDK V2 Module Versions Used

imported in file:

	"github.com/aws/aws-sdk-go-v2/aws"
	"github.com/aws/aws-sdk-go-v2/service/s3"
	"github.com/aws/aws-sdk-go-v2/service/s3/types"

go mod graph output:

github.com/aws/aws-sdk-go-v2@v1.16.16 github.com/aws/smithy-go@v1.13.3
github.com/aws/aws-sdk-go-v2@v1.16.16 github.com/google/go-cmp@v0.5.8
github.com/aws/aws-sdk-go-v2@v1.16.16 github.com/jmespath/go-jmespath@v0.4.0
github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream@v1.4.8 github.com/aws/smithy-go@v1.13.3
github.com/aws/aws-sdk-go-v2/config@v1.17.7 github.com/aws/aws-sdk-go-v2@v1.16.16
github.com/aws/aws-sdk-go-v2/config@v1.17.7 github.com/aws/aws-sdk-go-v2/credentials@v1.12.20
github.com/aws/aws-sdk-go-v2/config@v1.17.7 github.com/aws/aws-sdk-go-v2/feature/ec2/imds@v1.12.17
github.com/aws/aws-sdk-go-v2/config@v1.17.7 github.com/aws/aws-sdk-go-v2/internal/ini@v1.3.24
github.com/aws/aws-sdk-go-v2/config@v1.17.7 github.com/aws/aws-sdk-go-v2/service/sso@v1.11.23
github.com/aws/aws-sdk-go-v2/config@v1.17.7 github.com/aws/aws-sdk-go-v2/service/ssooidc@v1.13.5
github.com/aws/aws-sdk-go-v2/config@v1.17.7 github.com/aws/aws-sdk-go-v2/service/sts@v1.16.19
github.com/aws/aws-sdk-go-v2/config@v1.17.7 github.com/aws/smithy-go@v1.13.3
github.com/aws/aws-sdk-go-v2/config@v1.17.7 github.com/google/go-cmp@v0.5.8
github.com/aws/aws-sdk-go-v2/credentials@v1.12.20 github.com/aws/aws-sdk-go-v2@v1.16.16
github.com/aws/aws-sdk-go-v2/credentials@v1.12.20 github.com/aws/aws-sdk-go-v2/feature/ec2/imds@v1.12.17
github.com/aws/aws-sdk-go-v2/credentials@v1.12.20 github.com/aws/aws-sdk-go-v2/service/sso@v1.11.23
github.com/aws/aws-sdk-go-v2/credentials@v1.12.20 github.com/aws/aws-sdk-go-v2/service/ssooidc@v1.13.5
github.com/aws/aws-sdk-go-v2/credentials@v1.12.20 github.com/aws/aws-sdk-go-v2/service/sts@v1.16.19
github.com/aws/aws-sdk-go-v2/credentials@v1.12.20 github.com/aws/smithy-go@v1.13.3
github.com/aws/aws-sdk-go-v2/credentials@v1.12.20 github.com/google/go-cmp@v0.5.8
github.com/aws/aws-sdk-go-v2/feature/ec2/imds@v1.12.17 github.com/aws/aws-sdk-go-v2@v1.16.16
github.com/aws/aws-sdk-go-v2/feature/ec2/imds@v1.12.17 github.com/aws/smithy-go@v1.13.3
github.com/aws/aws-sdk-go-v2/feature/ec2/imds@v1.12.17 github.com/google/go-cmp@v0.5.8
github.com/aws/aws-sdk-go-v2/internal/configsources@v1.1.23 github.com/aws/aws-sdk-go-v2@v1.16.16
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2@v2.4.17 github.com/aws/aws-sdk-go-v2@v1.16.16
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2@v2.4.17 github.com/aws/smithy-go@v1.13.3
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2@v2.4.17 github.com/google/go-cmp@v0.5.8
github.com/aws/aws-sdk-go-v2/internal/ini@v1.3.24 github.com/aws/aws-sdk-go-v2@v1.16.16
github.com/aws/aws-sdk-go-v2/internal/v4a@v1.0.14 github.com/aws/aws-sdk-go-v2@v1.16.16
github.com/aws/aws-sdk-go-v2/internal/v4a@v1.0.14 github.com/aws/smithy-go@v1.13.3
github.com/aws/aws-sdk-go-v2/internal/v4a@v1.0.14 github.com/google/go-cmp@v0.5.8
github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding@v1.9.9 github.com/aws/smithy-go@v1.13.3
github.com/aws/aws-sdk-go-v2/service/internal/checksum@v1.1.18 github.com/aws/aws-sdk-go-v2@v1.16.16
github.com/aws/aws-sdk-go-v2/service/internal/checksum@v1.1.18 github.com/aws/smithy-go@v1.13.3
github.com/aws/aws-sdk-go-v2/service/internal/checksum@v1.1.18 github.com/google/go-cmp@v0.5.8
github.com/aws/aws-sdk-go-v2/service/internal/presigned-url@v1.9.17 github.com/aws/aws-sdk-go-v2@v1.16.16
github.com/aws/aws-sdk-go-v2/service/internal/presigned-url@v1.9.17 github.com/aws/smithy-go@v1.13.3
github.com/aws/aws-sdk-go-v2/service/internal/presigned-url@v1.9.17 github.com/google/go-cmp@v0.5.8
github.com/aws/aws-sdk-go-v2/service/internal/s3shared@v1.13.17 github.com/aws/aws-sdk-go-v2@v1.16.16
github.com/aws/aws-sdk-go-v2/service/internal/s3shared@v1.13.17 github.com/aws/smithy-go@v1.13.3
github.com/aws/aws-sdk-go-v2/service/s3@v1.27.11 github.com/aws/aws-sdk-go-v2@v1.16.16
github.com/aws/aws-sdk-go-v2/service/s3@v1.27.11 github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream@v1.4.8
github.com/aws/aws-sdk-go-v2/service/s3@v1.27.11 github.com/aws/aws-sdk-go-v2/internal/configsources@v1.1.23
github.com/aws/aws-sdk-go-v2/service/s3@v1.27.11 github.com/aws/aws-sdk-go-v2/internal/endpoints/v2@v2.4.17
github.com/aws/aws-sdk-go-v2/service/s3@v1.27.11 github.com/aws/aws-sdk-go-v2/internal/v4a@v1.0.14
github.com/aws/aws-sdk-go-v2/service/s3@v1.27.11 github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding@v1.9.9
github.com/aws/aws-sdk-go-v2/service/s3@v1.27.11 github.com/aws/aws-sdk-go-v2/service/internal/checksum@v1.1.18
github.com/aws/aws-sdk-go-v2/service/s3@v1.27.11 github.com/aws/aws-sdk-go-v2/service/internal/presigned-url@v1.9.17
github.com/aws/aws-sdk-go-v2/service/s3@v1.27.11 github.com/aws/aws-sdk-go-v2/service/internal/s3shared@v1.13.17
github.com/aws/aws-sdk-go-v2/service/s3@v1.27.11 github.com/aws/smithy-go@v1.13.3
github.com/aws/aws-sdk-go-v2/service/s3@v1.27.11 github.com/google/go-cmp@v0.5.8
github.com/aws/aws-sdk-go-v2/service/sso@v1.11.23 github.com/aws/aws-sdk-go-v2@v1.16.16
github.com/aws/aws-sdk-go-v2/service/sso@v1.11.23 github.com/aws/aws-sdk-go-v2/internal/configsources@v1.1.23
github.com/aws/aws-sdk-go-v2/service/sso@v1.11.23 github.com/aws/aws-sdk-go-v2/internal/endpoints/v2@v2.4.17
github.com/aws/aws-sdk-go-v2/service/sso@v1.11.23 github.com/aws/smithy-go@v1.13.3
github.com/aws/aws-sdk-go-v2/service/ssooidc@v1.13.5 github.com/aws/aws-sdk-go-v2@v1.16.16
github.com/aws/aws-sdk-go-v2/service/ssooidc@v1.13.5 github.com/aws/aws-sdk-go-v2/internal/configsources@v1.1.23
github.com/aws/aws-sdk-go-v2/service/ssooidc@v1.13.5 github.com/aws/aws-sdk-go-v2/internal/endpoints/v2@v2.4.17
github.com/aws/aws-sdk-go-v2/service/ssooidc@v1.13.5 github.com/aws/smithy-go@v1.13.3
github.com/aws/aws-sdk-go-v2/service/sts@v1.16.19 github.com/aws/aws-sdk-go-v2@v1.16.16
github.com/aws/aws-sdk-go-v2/service/sts@v1.16.19 github.com/aws/aws-sdk-go-v2/internal/configsources@v1.1.23
github.com/aws/aws-sdk-go-v2/service/sts@v1.16.19 github.com/aws/aws-sdk-go-v2/internal/endpoints/v2@v2.4.17
github.com/aws/aws-sdk-go-v2/service/sts@v1.16.19 github.com/aws/aws-sdk-go-v2/service/internal/presigned-url@v1.9.17
github.com/aws/aws-sdk-go-v2/service/sts@v1.16.19 github.com/aws/smithy-go@v1.13.3
github.com/aws/smithy-go@v1.13.3 github.com/google/go-cmp@v0.5.8
github.com/aws/smithy-go@v1.13.3 github.com/jmespath/go-jmespath@v0.4.0

Compiler and Version used

go version go1.18 linux/amd64

Operating System and version

Fedora SilverBlue 36 linux 6.0.8-200.fc36.x86_64 (and on container)
@Sid-Sun Sid-Sun added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Nov 24, 2022
@RanVaknin
Copy link
Contributor

Hi @Sid-Sun ,

Can you please include your full code, or at least full code example showing how you make a request and assign the body to the s3 request body?

thanks,
Ran

@RanVaknin RanVaknin self-assigned this Nov 28, 2022
@RanVaknin RanVaknin added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed needs-triage This issue or PR still needs to be triaged. labels Nov 28, 2022
@Sid-Sun
Copy link
Author

Sid-Sun commented Nov 28, 2022

Hi! @RanVaknin

I will try to provide a more concise, specific snippet later today, however, if it helps you can check the code repo:
Sid-Sun/ioctl-api@83c8f53

Thanks!

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Nov 29, 2022
@RanVaknin
Copy link
Contributor

@Sid-Sun ,

I'm not sure I see what is the use case in that commit you linked.

Looking forward to seeing your repro code.

Thanks,
Ran

@RanVaknin RanVaknin added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Nov 29, 2022
@github-actions
Copy link

github-actions bot commented Dec 2, 2022

This issue has not received a response in 1 week. If you want to keep this issue open, please just leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Dec 2, 2022
@Sid-Sun
Copy link
Author

Sid-Sun commented Dec 3, 2022

Hey @RanVaknin I can reproduce the error with the following:

package main

import (
	"context"
	"fmt"
	"io"
	"net/http"
	"os"
	"time"

	"github.com/aws/aws-sdk-go-v2/config"
	"github.com/aws/aws-sdk-go-v2/service/s3"
	"github.com/aws/aws-sdk-go-v2/service/s3/types"
	"github.com/go-chi/chi"
)

func main() {
	r := chi.NewRouter()
	r.Put("/{file}", func(w http.ResponseWriter, r *http.Request) {
		err := uploadObject(r.Body)
		if err != nil {
			panic(err)
		}
	})

	srv := &http.Server{
		Addr:    "127.0.0.1:8080",
		Handler: r,
	}

	fmt.Println("Going to listening on http://127.0.0.1:8080")
	if err := srv.ListenAndServe(); err != nil && err != http.ErrServerClosed {
		panic(err)
	}
}

func uploadObject(data io.Reader) error {
	cfg, err := config.LoadDefaultConfig(context.TODO())
	if err != nil {
		panic(err)
	}
	acl := types.ObjectCannedACLPublicRead
	client := s3.NewFromConfig(cfg)

	bucket := os.Getenv("S3_BUCKET")
	key := "testfile"
	t := time.Now().Add(time.Hour)
	_, err = client.PutObject(context.Background(), &s3.PutObjectInput{
		Key:     &key,
		Body:    data,
		Bucket:  &bucket,
		ACL:     acl,
		Expires: &t,
	})
	if err != nil {
		return err
	}

	return nil
}

i get the error:

2022/12/03 07:47:49 http: panic serving 127.0.0.1:45248: operation error S3: PutObject, https response error StatusCode: 501, RequestID: PXWQNFMWSFEMJFPB, HostID: 2O0PcGYC50uNypC/A4Bw9ultLfmTngYA0GKZ3RI5PUsbJXALdPhzm/AMUzIPGNhnxxLjbNS8R40=, api error NotImplemented: A header you provided implies functionality that is not implemented

if i read the body and pass a reader to it, the code works flawlessly, i get no errors and file is uploaded to my S3 bucket:

	r.Put("/{file}", func(w http.ResponseWriter, r *http.Request) {
		data, err := ioutil.ReadAll(r.Body)
		if err != nil {
			panic(err)
		}
		
		err = uploadObject(bytes.NewReader(data))
		if err != nil {
			panic(err)
		}
	})

@github-actions github-actions bot removed closing-soon This issue will automatically close in 4 days unless further comments are made. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Dec 4, 2022
@RanVaknin
Copy link
Contributor

Hi @Sid-Sun,

Thanks for including your repro code! 👍
The reason why you need to wrap your data object in a byte reader, is for the request to have a valid content-length for the body you are sending.

Since you have chi-go setup there, Im not going to reproduce it on my machine, but what I'm guessing is happening, is that since the content-length of the body is unknown, the SDK will send a transfer-encoding: chunked header to S3 to try and mitigate the body missing, and S3 will return that header related error.

This is expected behavior for V2.

Thanks,
Ran~

@github-actions
Copy link

github-actions bot commented Dec 6, 2022

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@Sid-Sun
Copy link
Author

Sid-Sun commented Dec 6, 2022

@RanVaknin thanks for the response, however, I don't understand how the SDK would find the body size from a io.Reader interface regardless of its source as the interface only exposes the Read function which reads from source and returns the number of read bytes and error.

Can you add any insights here?

Thanks

@RanVaknin RanVaknin mentioned this issue Feb 23, 2023
Closed
@cartermckinnon
Copy link
Member

cartermckinnon commented Dec 27, 2023 •
edited
Loading

@Sid-Sun I'm late to the party, but feature/s3/manager.Uploader does this properly: https://pkg.go.dev/github.com/aws/aws-sdk-go-v2/feature/s3/manager

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@RanVaknin RanVaknin

Labels
bug This issue is a bug.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@cartermckinnon @Sid-Sun @RanVaknin