cp,pipe: don't omit some of the metadata flags (#658)

#657 mentioned that `acl` flag was broken in `v2.2.1`. After looking at the issue, it was discovered that not only the `acl` flag was broken, there are couple of more flags that were being omitted during the mentioned commands, which all of them caused by this faulty [PR](#621). Resolves #657.
peak · Sep 15, 2023 · 48f7e59 · 48f7e59
1 parent 10e4ccf
commit 48f7e59
Show file tree

Hide file tree

Showing 12 changed files with 246 additions and 83 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,10 +1,12 @@
 # Changelog
+## v2.2.2 - 13 Sep 2023 
 
+#### Bugfixes
+- Fixed `cp` and `pipe` to not omit some of the metadata flags. ([#657](https://github.com/peak/s5cmd/issues/657))
 ## v2.2.1 - 23 Aug 2023
 
 #### Bugfixes
 - Fixed incorrect `s5cmd version` output ([#650](https://github.com/peak/s5cmd/pull/650))
-
 ## v2.2.0 - 21 Aug 2023
 
 #### Features

diff --git a/command/cp.go b/command/cp.go
@@ -689,10 +689,17 @@ func (c Copy) doUpload(ctx context.Context, srcurl *url.URL, dsturl *url.URL, ex
 	if err != nil {
 		return err
 	}
-	metadata := storage.Metadata{UserDefined: extradata}
 
-	if c.storageClass != "" {
-		metadata.StorageClass = string(c.storageClass)
+	metadata := storage.Metadata{
+		UserDefined:        extradata,
+		ACL:                c.acl,
+		CacheControl:       c.cacheControl,
+		Expires:            c.expires,
+		StorageClass:       string(c.storageClass),
+		ContentEncoding:    c.contentEncoding,
+		ContentDisposition: c.contentDisposition,
+		EncryptionMethod:   c.encryptionMethod,
+		EncryptionKeyID:    c.encryptionKeyID,
 	}
 
 	if c.contentType != "" {
@@ -701,14 +708,6 @@ func (c Copy) doUpload(ctx context.Context, srcurl *url.URL, dsturl *url.URL, ex
 		metadata.ContentType = guessContentType(file)
 	}
 
-	if c.contentEncoding != "" {
-		metadata.ContentEncoding = c.contentEncoding
-	}
-
-	if c.contentDisposition != "" {
-		metadata.ContentDisposition = c.contentDisposition
-	}
-
 	reader := newCountingReaderWriter(file, c.progressbar)
 	err = dstClient.Put(ctx, reader, dsturl, metadata, c.concurrency, c.partSize)
 
@@ -755,20 +754,17 @@ func (c Copy) doCopy(ctx context.Context, srcurl, dsturl *url.URL, extradata map
 		return err
 	}
 
-	metadata := storage.Metadata{UserDefined: extradata}
-	if c.storageClass != "" {
-		metadata.StorageClass = string(c.storageClass)
-	}
-
-	if c.contentType != "" {
-		metadata.ContentType = c.contentType
-	}
-
-	if c.contentEncoding != "" {
-		metadata.ContentEncoding = c.contentEncoding
-	}
-	if c.contentDisposition != "" {
-		metadata.ContentDisposition = c.contentDisposition
+	metadata := storage.Metadata{
+		UserDefined:        extradata,
+		ACL:                c.acl,
+		CacheControl:       c.cacheControl,
+		Expires:            c.expires,
+		StorageClass:       string(c.storageClass),
+		ContentType:        c.contentType,
+		ContentEncoding:    c.contentEncoding,
+		ContentDisposition: c.contentDisposition,
+		EncryptionMethod:   c.encryptionMethod,
+		EncryptionKeyID:    c.encryptionKeyID,
 	}
 
 	err = c.shouldOverride(ctx, srcurl, dsturl)

diff --git a/command/pipe.go b/command/pipe.go
@@ -221,9 +221,16 @@ func (c Pipe) Run(ctx context.Context) error {
 		return err
 	}
 
-	metadata := storage.Metadata{UserDefined: c.metadata}
-	if c.storageClass != "" {
-		metadata.StorageClass = string(c.storageClass)
+	metadata := storage.Metadata{
+		UserDefined:        c.metadata,
+		ACL:                c.acl,
+		CacheControl:       c.cacheControl,
+		Expires:            c.expires,
+		StorageClass:       string(c.storageClass),
+		ContentEncoding:    c.contentEncoding,
+		ContentDisposition: c.contentDisposition,
+		EncryptionMethod:   c.encryptionMethod,
+		EncryptionKeyID:    c.encryptionKeyID,
 	}
 
 	if c.contentType != "" {
@@ -232,13 +239,6 @@ func (c Pipe) Run(ctx context.Context) error {
 		metadata.ContentType = guessContentTypeByExtension(c.dst)
 	}
 
-	if c.contentEncoding != "" {
-		metadata.ContentEncoding = c.contentEncoding
-	}
-	if c.contentDisposition != "" {
-		metadata.ContentDisposition = c.contentDisposition
-	}
-
 	err = client.Put(ctx, &stdin{file: os.Stdin}, c.dst, metadata, c.concurrency, c.partSize)
 	if err != nil {
 		return err

diff --git a/e2e/cp_test.go b/e2e/cp_test.go
@@ -25,6 +25,7 @@ package e2e
 import (
 	"fmt"
 	"net"
+	"net/http"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -720,6 +721,75 @@ func TestCopySingleFileToS3(t *testing.T) {
 	assert.Assert(t, ensureS3Object(s3client, bucket, filename, content, ensureContentType(expectedContentType), ensureContentDisposition(expectedContentDisposition)))
 }
 
+func TestCopySingleFileToS3WithAllMetadataFlags(t *testing.T) {
+	t.Parallel()
+
+	s3client, s5cmd := setup(t)
+
+	bucket := s3BucketFromTestName(t)
+
+	createBucket(t, s3client, bucket)
+
+	const (
+		filename           = "index"
+		content            = `testfilecontent`
+		cacheControl       = "public, max-age=3600"
+		expires            = "2025-01-01T00:00:00Z"
+		storageClass       = "STANDARD_IA"
+		ContentType        = "text/html; charset=utf-8"
+		ContentDisposition = "inline"
+		ContentEncoding    = "utf-8"
+		EncryptionMethod   = "aws:kms"
+		EncryptionKeyID    = "1234abcd-12ab-34cd-56ef-1234567890ab"
+	)
+
+	// expected expires flag is the parsed version of the date in RFC3339 format
+	parsedTime, err := time.Parse(time.RFC3339, expires)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	expectedExpires := parsedTime.Format(http.TimeFormat)
+
+	workdir := fs.NewDir(t, bucket, fs.WithFile(filename, content))
+	defer workdir.Remove()
+
+	srcpath := workdir.Join(filename)
+	dstpath := fmt.Sprintf("s3://%v/", bucket)
+
+	srcpath = filepath.ToSlash(srcpath)
+	cmd := s5cmd("cp",
+		"--cache-control", cacheControl,
+		"--expires", expires,
+		"--storage-class", storageClass,
+		"--content-type", ContentType,
+		"--content-disposition", ContentDisposition,
+		"--content-encoding", ContentEncoding,
+		"--sse", EncryptionMethod,
+		"--sse-kms-key-id", EncryptionKeyID,
+		srcpath, dstpath,
+	)
+
+	result := icmd.RunCmd(cmd)
+
+	result.Assert(t, icmd.Success)
+
+	expected := fs.Expected(t, fs.WithFile(filename, content))
+	assert.Assert(t, fs.Equal(workdir.Path(), expected))
+
+	assert.Assert(t, ensureS3Object(s3client, bucket, filename, content,
+		ensureExpires(expectedExpires),
+		ensureCacheControl(cacheControl),
+		ensureStorageClass(storageClass),
+		ensureContentType(ContentType),
+		ensureContentDisposition(ContentDisposition),
+		ensureContentEncoding(ContentEncoding),
+		ensureEncryptionMethod(EncryptionMethod),
+		ensureEncryptionKeyID(EncryptionKeyID),
+	))
+
+}
+
 // cp dir/file s3://bucket/ --metadata key1=val1 --metadata key2=val2 ...
 func TestCopySingleFileToS3WithArbitraryMetadata(t *testing.T) {
 	t.Parallel()
@@ -800,10 +870,11 @@ func TestCopyS3ToS3WithArbitraryMetadata(t *testing.T) {
 		"Key1": aws.String("foo"),
 		"Key2": aws.String("bar"),
 	}
+
 	srcpath := fmt.Sprintf("s3://%v/%v", bucket, filename)
 	dstpath := fmt.Sprintf("s3://%v/%v_cp", bucket, filename)
 
-	putFileWithMetadata(t, s3client, bucket, filename, content, srcmetadata)
+	putFile(t, s3client, bucket, filename, content, putArbitraryMetadata(srcmetadata))
 	cmd := s5cmd("cp", "--metadata", foo, "--metadata", bar, srcpath, dstpath)
 	result := icmd.RunCmd(cmd)
 	result.Assert(t, icmd.Success)

diff --git a/e2e/pipe_test.go b/e2e/pipe_test.go
@@ -3,11 +3,14 @@ package e2e
 import (
 	"bytes"
 	"fmt"
+	"net/http"
 	"runtime"
 	"testing"
+	"time"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"gotest.tools/v3/assert"
+	"gotest.tools/v3/fs"
 	"gotest.tools/v3/icmd"
 )
 
@@ -461,44 +464,68 @@ func TestUploadStdinToS3WithStorageClassGlacier(t *testing.T) {
 }
 
 // pipe --content-disposition inline s3://bucket/object
-func TestUploadStdinToToS3WithContentDisposition(t *testing.T) {
+func TestUploadStdinToToS3WithAllMetadataFlags(t *testing.T) {
 	t.Parallel()
 
 	s3client, s5cmd := setup(t)
 
 	bucket := s3BucketFromTestName(t)
+
 	createBucket(t, s3client, bucket)
 
 	const (
-		// make sure that Put reads the file header and guess Content-Type correctly.
-		filename = "index.html"
-		content  = `
-<html lang="tr">
-	<head>
-	<meta charset="utf-8">
-	<body>
-		<header></header>
-		<main></main>
-		<footer></footer>
-	</body>
-</html>
-`
-		expectedContentType        = "text/html; charset=utf-8"
-		expectedContentDisposition = "inline"
+		filename           = "index"
+		content            = `testfilecontent`
+		cacheControl       = "public, max-age=3600"
+		expires            = "2025-01-01T00:00:00Z"
+		storageClass       = "STANDARD_IA"
+		ContentType        = "text/html; charset=utf-8"
+		ContentDisposition = "inline"
+		ContentEncoding    = "utf-8"
+		EncryptionMethod   = "aws:kms"
+		EncryptionKeyID    = "1234abcd-12ab-34cd-56ef-1234567890ab"
 	)
 
+	// expected expires flag is the parsed version of the date in RFC3339 format
+	parsedTime, err := time.Parse(time.RFC3339, expires)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	expectedExpires := parsedTime.Format(http.TimeFormat)
+
+	workdir := fs.NewDir(t, bucket, fs.WithFile(filename, content))
+	defer workdir.Remove()
+
 	dstpath := fmt.Sprintf("s3://%v/%v", bucket, filename)
+
 	reader := bytes.NewBufferString(content)
 
-	cmd := s5cmd("pipe", "--content-disposition", "inline", dstpath)
+	cmd := s5cmd("pipe",
+		"--cache-control", cacheControl,
+		"--expires", expires,
+		"--storage-class", storageClass,
+		"--content-type", ContentType,
+		"--content-disposition", ContentDisposition,
+		"--content-encoding", ContentEncoding,
+		"--sse", EncryptionMethod,
+		"--sse-kms-key-id", EncryptionKeyID,
+		dstpath,
+	)
+
 	result := icmd.RunCmd(cmd, icmd.WithStdin(reader))
 
 	result.Assert(t, icmd.Success)
 
-	assertLines(t, result.Stdout(), map[int]compareFunc{
-		0: suffix(`pipe %v`, dstpath),
-	})
-
 	// assert S3
-	assert.Assert(t, ensureS3Object(s3client, bucket, filename, content, ensureContentType(expectedContentType), ensureContentDisposition(expectedContentDisposition)))
+	assert.Assert(t, ensureS3Object(s3client, bucket, filename, content,
+		ensureExpires(expectedExpires),
+		ensureCacheControl(cacheControl),
+		ensureStorageClass(storageClass),
+		ensureContentType(ContentType),
+		ensureContentDisposition(ContentDisposition),
+		ensureContentEncoding(ContentEncoding),
+		ensureEncryptionMethod(EncryptionMethod),
+		ensureEncryptionKeyID(EncryptionKeyID),
+	))
 }