Skip to content

Commit

Permalink
[s3] Fix a bug in CopyObject and improve Metadata handling
Browse files Browse the repository at this point in the history
  • Loading branch information
dzbarsky committed Sep 19, 2024
1 parent 471ce01 commit e8f775e
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 6 deletions.
2 changes: 1 addition & 1 deletion .bazelrc
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
common --enable_bzlmod
common --experimental_output_paths=strip

common --test_output=errors
16 changes: 15 additions & 1 deletion services/s3/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"encoding/xml"
"io"
"log/slog"
"mime"
"net/http"
"os"
"reflect"
Expand Down Expand Up @@ -145,11 +146,24 @@ func NewHandler(logger *slog.Logger, s3 *S3) func(w http.ResponseWriter, r *http
}
}

func isASCII(s string) bool {
for i := 0; i < len(s); i++ {
if s[i] >= 128 {
return false
}
}
return true
}

func extractMetadata(header http.Header) map[string]string {
metadata := make(map[string]string)
for k := range header {
if strings.HasPrefix(strings.ToLower(k), "x-amz-meta-") {
metadata[k] = header.Get(k)
v := header.Get(k)
if !isASCII(v) {
v = mime.BEncoding.Encode("utf-8", v)
}
metadata[k] = v
}
}
return metadata
Expand Down
6 changes: 5 additions & 1 deletion services/s3/itest/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@ load("@rules_go//go:def.bzl", "go_test")

go_test(
name = "itest_test",
srcs = ["s3_test.go"],
srcs = [
"copy_object_test.go",
"get_object_test.go",
"s3_test.go",
],
deps = [
"//server",
"//services/s3",
Expand Down
35 changes: 35 additions & 0 deletions services/s3/itest/copy_object_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package itest

import (
"context"
"strings"
"testing"

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

func TestCopyObject(t *testing.T) {
ctx := context.Background()
client, srv := makeClientServerPair()
defer srv.Shutdown(ctx)

key := "test-key"
_, err := client.PutObject(ctx, &s3.PutObjectInput{
Bucket: &bucket,
Key: &key,
Body: strings.NewReader("hello"),
})
if err != nil {
t.Fatal(err)
}

_, err = client.CopyObject(ctx, &s3.CopyObjectInput{
Bucket: &bucket,
Key: &key,
CopySource: aws.String("/" + bucket + "/" + key),
})
if err != nil {
t.Fatal(err)
}
}
50 changes: 50 additions & 0 deletions services/s3/itest/get_object_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package itest

import (
"context"
"reflect"
"strings"
"testing"

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

func TestGetObject_Metadata(t *testing.T) {
// See https://docs.aws.amazon.com/AmazonS3/latest/userguide/UsingMetadata.html
ctx := context.Background()
client, srv := makeClientServerPair()
defer srv.Shutdown(ctx)

key := "test-key"
_, err := client.PutObject(ctx, &s3.PutObjectInput{
Bucket: &bucket,
Key: &key,
Body: strings.NewReader("hello"),
Metadata: map[string]string{
"ascii": "AMAZONS3",
"non-ascii": "ÄMÄZÕÑ S3",
},
})
if err != nil {
t.Fatal(err)
}

resp, err := client.GetObject(ctx, &s3.GetObjectInput{
Bucket: &bucket,
Key: &key,
})
if err != nil {
t.Fatal(err)
}
got := resp.Metadata
want := map[string]string{
"ascii": "AMAZONS3",
// Encoding doesn't match exactly but maybe compatible enough?
"non-ascii": "=?utf-8?b?w4RNw4Raw5XDkSBTMw==?=",
// "non-ascii": "=?UTF-8?B?w4PChE3Dg8KEWsODwpXDg8KRIFMz?=",
}

if !reflect.DeepEqual(got, want) {
t.Fatalf("Wanted %v, got %v", want, got)
}
}
10 changes: 8 additions & 2 deletions services/s3/itest/s3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"log/slog"
"net"
"net/http"
"os"
"reflect"
"strconv"
"strings"
Expand All @@ -24,17 +25,22 @@ import (
var bucket = "test-bucket"

func makeClientServerPair() (*s3.Client, *http.Server) {
logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{
Level: slog.LevelWarn,
}))

listener, err := net.Listen("tcp", "127.0.0.1:0")
if err != nil {
panic(err)
}
impl, err := s3Impl.New(s3Impl.Options{
Addr: listener.Addr().String(),
Addr: listener.Addr().String(),
Logger: logger,
})
if err != nil {
panic(err)
}
srv := server.NewWithHandlerChain(s3Impl.NewHandler(slog.Default(), impl))
srv := server.NewWithHandlerChain(s3Impl.NewHandler(logger, impl))
go srv.Serve(listener)

client := s3.New(s3.Options{
Expand Down
6 changes: 5 additions & 1 deletion services/s3/s3.go
Original file line number Diff line number Diff line change
Expand Up @@ -516,11 +516,15 @@ func (s *S3) CopyObject(input CopyObjectInput) (*CopyObjectOutput, *awserrors.Er
s.mu.Lock()
defer s.mu.Unlock()

// "bucket/path/to/key"
// "bucket/path/to/key", maybe with leading slash.
copySource, err := url.PathUnescape(input.CopySource)
if err != nil {
return nil, awserrors.XXX_TODO(err.Error())
}
if copySource[0] == '/' {
copySource = copySource[1:]
}

parts := strings.SplitN(copySource, "/", 2)
sourceBucket := parts[0]
sourceKey := parts[1]
Expand Down

0 comments on commit e8f775e

Please sign in to comment.