don't skip empty value at `AssembleEntryWithAcp` (#3855)

* add acl helper functionalities

Signed-off-by: changlin.shi <changlin.shi@ly.com>

* add tests

Signed-off-by: changlin.shi <changlin.shi@ly.com>

* remove 0 when create map

Signed-off-by: changlin.shi <changlin.shi@ly.com>

* delete when empty at `AssembleEntryWithAcp`

`PutBucketAcl/PutObjectAcl` allow request with empty grants, `AssembleEntryWithAcp`  shouldn't skip empty value

Signed-off-by: changlin.shi <changlin.shi@ly.com>

Signed-off-by: changlin.shi <changlin.shi@ly.com>

2 files changed, 36 insertions, 34 deletions

diff --git a/weed/s3api/s3acl/acl_helper.go b/weed/s3api/s3acl/acl_helper.go
index e54e67556..bb956368e 100644
--- a/weed/s3api/s3acl/acl_helper.go
+++ b/weed/s3api/s3acl/acl_helper.go
@@ -411,6 +411,8 @@ func AssembleEntryWithAcp(objectEntry *filer_pb.Entry, objectOwner string, grant
 
 	if len(objectOwner) > 0 {
 		objectEntry.Extended[s3_constants.ExtAmzOwnerKey] = []byte(objectOwner)
+	} else {
+		delete(objectEntry.Extended, s3_constants.ExtAmzOwnerKey)
 	}
 
 	if len(grants) > 0 {
@@ -420,6 +422,8 @@ func AssembleEntryWithAcp(objectEntry *filer_pb.Entry, objectOwner string, grant
 			return s3err.ErrInvalidRequest
 		}
 		objectEntry.Extended[s3_constants.ExtAmzAclKey] = grantsBytes
+	} else {
+		delete(objectEntry.Extended, s3_constants.ExtAmzAclKey)
 	}
 
 	return s3err.ErrNone
diff --git a/weed/s3api/s3acl/acl_helper_test.go b/weed/s3api/s3acl/acl_helper_test.go
index efc137989..ce177595b 100644
--- a/weed/s3api/s3acl/acl_helper_test.go
+++ b/weed/s3api/s3acl/acl_helper_test.go
@@ -487,46 +487,44 @@ func TestDetermineReqGrants(t *testing.T) {
 
 func TestAssembleEntryWithAcp(t *testing.T) {
 	defaultOwner := "admin"
-	{
-		//case1
-		expectOwner := "accountS"
-		expectGrants := []*s3.Grant{
-			{
-				Permission: &s3_constants.PermissionRead,
-				Grantee: &s3.Grantee{
-					Type: &s3_constants.GrantTypeGroup,
-					ID:   &s3account.AccountAdmin.Id,
-					URI:  &s3_constants.GranteeGroupAllUsers,
-				},
+
+	//case1
+	//assemble with non-empty grants
+	expectOwner := "accountS"
+	expectGrants := []*s3.Grant{
+		{
+			Permission: &s3_constants.PermissionRead,
+			Grantee: &s3.Grantee{
+				Type: &s3_constants.GrantTypeGroup,
+				ID:   &s3account.AccountAdmin.Id,
+				URI:  &s3_constants.GranteeGroupAllUsers,
 			},
-		}
-		entry := &filer_pb.Entry{}
-		AssembleEntryWithAcp(entry, expectOwner, expectGrants)
+		},
+	}
+	entry := &filer_pb.Entry{}
+	AssembleEntryWithAcp(entry, expectOwner, expectGrants)
 
-		resultOwner := GetAcpOwner(entry.Extended, defaultOwner)
-		if resultOwner != expectOwner {
-			t.Fatalf("owner not expect")
-		}
+	resultOwner := GetAcpOwner(entry.Extended, defaultOwner)
+	if resultOwner != expectOwner {
+		t.Fatalf("owner not expect")
+	}
 
-		resultGrants := GetAcpGrants(entry.Extended)
-		if !grantsEquals(resultGrants, expectGrants) {
-			t.Fatal("grants not expect")
-		}
+	resultGrants := GetAcpGrants(entry.Extended)
+	if !grantsEquals(resultGrants, expectGrants) {
+		t.Fatal("grants not expect")
 	}
-	{
-		//case2
-		entry := &filer_pb.Entry{}
-		AssembleEntryWithAcp(entry, "", nil)
 
-		resultOwner := GetAcpOwner(entry.Extended, defaultOwner)
-		if resultOwner != defaultOwner {
-			t.Fatalf("owner not expect")
-		}
+	//case2
+	//assemble with empty grants (override)
+	AssembleEntryWithAcp(entry, "", nil)
+	resultOwner = GetAcpOwner(entry.Extended, defaultOwner)
+	if resultOwner != defaultOwner {
+		t.Fatalf("owner not expect")
+	}
 
-		resultGrants := GetAcpGrants(entry.Extended)
-		if len(resultGrants) != 0 {
-			t.Fatal("grants not expect")
-		}
+	resultGrants = GetAcpGrants(entry.Extended)
+	if len(resultGrants) != 0 {
+		t.Fatal("grants not expect")
 	}
 
 }