From 87cbd09fa7ffbdc1b8eb2ab5098a5e91e815c216 Mon Sep 17 00:00:00 2001
From: Ricardo Maraschini <ricardo.maraschini@gmail.com>
Date: Mon, 30 Nov 2020 13:04:14 +0100
Subject: [PATCH] Ignore self reference object on empty prefix

When a given prefix is empty and we attempt to list its content AWS
returns that the prefix contains one object with key defined as the
prefix with an extra "/" at the end.

e.g.

If we call ListObjects() passing to it an existing but empty prefix,
say "my/empty/prefix", AWS will return that "my/empty/prefix/" is an
object inside "my/empty/prefix" (ListObjectsOutput.Contents).

This extra "/" causes the upload purging process to panic. On normal
circunstances we never find empty prefixes on S3 but users may touch
it.

Signed-off-by: Ricardo Maraschini <rmarasch@redhat.com>
---
 registry/storage/driver/s3-aws/s3.go      |  5 ++++
 registry/storage/driver/s3-aws/s3_test.go | 34 +++++++++++++++++++++++
 2 files changed, 39 insertions(+)

diff --git a/registry/storage/driver/s3-aws/s3.go b/registry/storage/driver/s3-aws/s3.go
index cdc7e981c..5a4541cd0 100644
--- a/registry/storage/driver/s3-aws/s3.go
+++ b/registry/storage/driver/s3-aws/s3.go
@@ -986,6 +986,11 @@ func (d *driver) doWalk(parentCtx context.Context, objectCount *int64, path, pre
 		}
 
 		for _, file := range objects.Contents {
+			// empty prefixes are listed as objects inside its own prefix.
+			// https://docs.aws.amazon.com/AmazonS3/latest/user-guide/using-folders.html
+			if strings.HasSuffix(*file.Key, "/") {
+				continue
+			}
 			walkInfos = append(walkInfos, walkInfoContainer{
 				FileInfoFields: storagedriver.FileInfoFields{
 					IsDir:   false,
diff --git a/registry/storage/driver/s3-aws/s3_test.go b/registry/storage/driver/s3-aws/s3_test.go
index be02772e9..dec479166 100644
--- a/registry/storage/driver/s3-aws/s3_test.go
+++ b/registry/storage/driver/s3-aws/s3_test.go
@@ -5,6 +5,7 @@ import (
 	"io/ioutil"
 	"math/rand"
 	"os"
+	"reflect"
 	"strconv"
 	"testing"
 
@@ -164,6 +165,39 @@ func TestEmptyRootList(t *testing.T) {
 	}
 }
 
+// TestWalkEmptySubDirectory assures we list an empty sub directory only once when walking
+// through its parent directory.
+func TestWalkEmptySubDirectory(t *testing.T) {
+	if skipS3() != "" {
+		t.Skip(skipS3())
+	}
+
+	drv, err := s3DriverConstructor("", s3.StorageClassStandard)
+	if err != nil {
+		t.Fatalf("unexpected error creating rooted driver: %v", err)
+	}
+
+	// create an empty sub directory.
+	s3driver := drv.StorageDriver.(*driver)
+	if _, err := s3driver.S3.PutObject(&s3.PutObjectInput{
+		Bucket: aws.String(os.Getenv("S3_BUCKET")),
+		Key:    aws.String("/testdir/emptydir/"),
+	}); err != nil {
+		t.Fatalf("error creating empty directory: %s", err)
+	}
+
+	bucketFiles := []string{}
+	s3driver.Walk(context.Background(), "/testdir", func(fileInfo storagedriver.FileInfo) error {
+		bucketFiles = append(bucketFiles, fileInfo.Path())
+		return nil
+	})
+
+	expected := []string{"/testdir/emptydir"}
+	if !reflect.DeepEqual(bucketFiles, expected) {
+		t.Errorf("expecting files %+v, found %+v instead", expected, bucketFiles)
+	}
+}
+
 func TestStorageClass(t *testing.T) {
 	if skipS3() != "" {
 		t.Skip(skipS3())