From a5b867af69ff7be5e0f0944b2ee4275524d542e9 Mon Sep 17 00:00:00 2001 From: LHHDZ Date: Fri, 14 Oct 2022 13:50:44 +0800 Subject: [PATCH] don't skip empty value at `AssembleEntryWithAcp` (#3855) * add acl helper functionalities Signed-off-by: changlin.shi * add tests Signed-off-by: changlin.shi * remove 0 when create map Signed-off-by: changlin.shi * delete when empty at `AssembleEntryWithAcp` `PutBucketAcl/PutObjectAcl` allow request with empty grants, `AssembleEntryWithAcp` shouldn't skip empty value Signed-off-by: changlin.shi Signed-off-by: changlin.shi --- weed/s3api/s3acl/acl_helper.go | 4 ++ weed/s3api/s3acl/acl_helper_test.go | 70 ++++++++++++++--------------- 2 files changed, 38 insertions(+), 36 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) - - 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") - } + }, } - { - //case2 - entry := &filer_pb.Entry{} - AssembleEntryWithAcp(entry, "", nil) + entry := &filer_pb.Entry{} + AssembleEntryWithAcp(entry, expectOwner, expectGrants) - resultOwner := GetAcpOwner(entry.Extended, defaultOwner) - if resultOwner != defaultOwner { - t.Fatalf("owner not expect") - } + resultOwner := GetAcpOwner(entry.Extended, defaultOwner) + if resultOwner != expectOwner { + t.Fatalf("owner not expect") + } - resultGrants := GetAcpGrants(entry.Extended) - if len(resultGrants) != 0 { - t.Fatal("grants not expect") - } + resultGrants := GetAcpGrants(entry.Extended) + if !grantsEquals(resultGrants, expectGrants) { + t.Fatal("grants 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") } }