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

Allow empty values on prefix headers #2816

Merged
merged 2 commits into from
Oct 7, 2024
Merged

Conversation

Madrigal
Copy link
Contributor

@Madrigal Madrigal commented Oct 3, 2024

  • fix: allow empty values on prefix headers
  • Add changelog

Related smithy-go change aws/smithy-go#544

Had an issue where calling S3 with metadata with an empty value caused SDKs to not serialize the value and not send the header. This allows headers with an empty string to be serialized.

Testing

Wrote the following test program

package main

import (
	"context"
	"log"

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

func main() {
	cfg, err := config.LoadDefaultConfig(context.TODO(),
		config.WithRegion("us-west-2"),
		config.WithClientLogMode(aws.LogResponseWithBody|aws.LogRequestWithBody),
	)
	if err != nil {
		log.Fatalf("error: %v", err)
	}

	svc := s3.NewFromConfig(cfg)
	_, err = svc.PutObject(context.Background(), &s3.PutObjectInput{
		Bucket: aws.String("lmadrig-test-bucket"),
		Key:    aws.String("package.json"),
		Metadata: map[string]string{
			"key1": "value1",
			"key2": "",
		},
	})
	if err != nil {
		log.Fatalf("s3 error: %v", err)
	}

Ensure the logs contain the key2 empty header

PUT /package.json?x-id=PutObject HTTP/1.1
(...)
X-Amz-Date: 20241003T212511Z
X-Amz-Meta-Key1: value1
X-Amz-Meta-Key2:

Protocol tests will be updated at smithy-lang/smithy#2415

@Madrigal Madrigal requested a review from a team as a code owner October 3, 2024 21:31
@Madrigal Madrigal merged commit 99e2be8 into main Oct 7, 2024
12 checks passed
@Madrigal Madrigal deleted the fix-allow-empty-prefix-headers branch October 7, 2024 16:43
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 this pull request may close these issues.

3 participants