mirrors/seaweedfs
1
0
Fork 0
mirror of https://github.com/seaweedfs/seaweedfs.git synced 2026-05-22 01:31:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

s3api: simplify objectLockVersionToCheckForDelete to single return value

Browse Source 
Since all branches now return true (fail-closed), eliminate the boolean return value and the dead-code if guards. The function now clearly indicates that object lock checks always execute, with no opt-out path.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Chris Lu
2026-02-16 02:08:32 -08:00
parent 3bd990d2ea
commit 09bfb28db9
2 changed files with 53 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files

55
weed/s3api/s3api_object_handlers_delete.go
View File

@@ -22,19 +22,16 @@ const (
// objectLockVersionToCheckForDelete resolves which version should be validated for Object Lock protections.
// For versioned delete without explicit versionId, this targets the latest version (empty versionId for enabled,
// "null" for suspended) because DeleteObject affects that version's visibility.
func objectLockVersionToCheckForDelete(versioningState, requestedVersionID string) (string, bool) {
// This always returns a version ID to check; object lock protections are fail-closed.
func objectLockVersionToCheckForDelete(versioningState, requestedVersionID string) string {
if requestedVersionID != "" {
return requestedVersionID, true
return requestedVersionID
}
switch versioningState {
case s3_constants.VersioningEnabled, "":
return "", true
case s3_constants.VersioningSuspended:
return "null", true
default:
return "", true
if versioningState == s3_constants.VersioningSuspended {
return "null"
}
return ""
}
func (s3a *S3ApiServer) DeleteObjectHandler(w http.ResponseWriter, r *http.Request) {
@@ -70,14 +67,12 @@ func (s3a *S3ApiServer) DeleteObjectHandler(w http.ResponseWriter, r *http.Reque
auditLog = s3err.GetAccessLog(r, http.StatusNoContent, s3err.ErrNone)
}
lockCheckVersionID, shouldCheckObjectLock := objectLockVersionToCheckForDelete(versioningState, versionId)
if shouldCheckObjectLock {
governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, object)
if err := s3a.enforceObjectLockProtections(r, bucket, object, lockCheckVersionID, governanceBypassAllowed); err != nil {
glog.V(2).Infof("DeleteObjectHandler: object lock check failed for %s/%s (version: %s): %v", bucket, object, lockCheckVersionID, err)
s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied)
return
}
lockCheckVersionID := objectLockVersionToCheckForDelete(versioningState, versionId)
governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, object)
if err := s3a.enforceObjectLockProtections(r, bucket, object, lockCheckVersionID, governanceBypassAllowed); err != nil {
glog.V(2).Infof("DeleteObjectHandler: object lock check failed for %s/%s (version: %s): %v", bucket, object, lockCheckVersionID, err)
s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied)
return
}
if versioningConfigured {
@@ -252,20 +247,18 @@ func (s3a *S3ApiServer) DeleteMultipleObjectsHandler(w http.ResponseWriter, r *h
}
// Check object lock permissions before deletion (applies to all buckets: versioned or non-versioned)
lockCheckVersionID, shouldCheckObjectLock := objectLockVersionToCheckForDelete(versioningState, object.VersionId)
if shouldCheckObjectLock {
// Validate governance bypass for this specific object
governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, object.Key)
if err := s3a.enforceObjectLockProtections(r, bucket, object.Key, lockCheckVersionID, governanceBypassAllowed); err != nil {
glog.V(2).Infof("DeleteMultipleObjectsHandler: object lock check failed for %s/%s (version: %s): %v", bucket, object.Key, lockCheckVersionID, err)
deleteErrors = append(deleteErrors, DeleteError{
Code: s3err.GetAPIError(s3err.ErrAccessDenied).Code,
Message: s3err.GetAPIError(s3err.ErrAccessDenied).Description,
Key: object.Key,
VersionId: object.VersionId,
})
continue
}
lockCheckVersionID := objectLockVersionToCheckForDelete(versioningState, object.VersionId)
// Validate governance bypass for this specific object
governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, object.Key)
if err := s3a.enforceObjectLockProtections(r, bucket, object.Key, lockCheckVersionID, governanceBypassAllowed); err != nil {
glog.V(2).Infof("DeleteMultipleObjectsHandler: object lock check failed for %s/%s (version: %s): %v", bucket, object.Key, lockCheckVersionID, err)
deleteErrors = append(deleteErrors, DeleteError{
Code: s3err.GetAPIError(s3err.ErrAccessDenied).Code,
Message: s3err.GetAPIError(s3err.ErrAccessDenied).Description,
Key: object.Key,
VersionId: object.VersionId,
})
continue
}
var deleteVersionId string

66
weed/s3api/s3api_object_handlers_delete_test.go
View File

@@ -9,61 +9,53 @@ import (
func TestObjectLockVersionToCheckForDelete(t *testing.T) {
tests := []struct {
name string
versioningState string
requestedVersionID string
expectedVersionID string
expectedShouldCheck bool
name string
versioningState string
requestedVersionID string
expectedVersionID string
}{
{
name: "enabled versioning without version id checks latest version",
versioningState: s3_constants.VersioningEnabled,
requestedVersionID: "",
expectedVersionID: "",
expectedShouldCheck: true,
name: "enabled versioning without version id checks latest version",
versioningState: s3_constants.VersioningEnabled,
requestedVersionID: "",
expectedVersionID: "",
},
{
name: "suspended versioning without version id checks null version",
versioningState: s3_constants.VersioningSuspended,
requestedVersionID: "",
expectedVersionID: "null",
expectedShouldCheck: true,
name: "suspended versioning without version id checks null version",
versioningState: s3_constants.VersioningSuspended,
requestedVersionID: "",
expectedVersionID: "null",
},
{
name: "specific version id is always checked",
versioningState: s3_constants.VersioningEnabled,
requestedVersionID: "3LgYQ7f7VxQ3",
expectedVersionID: "3LgYQ7f7VxQ3",
expectedShouldCheck: true,
name: "specific version id is always checked",
versioningState: s3_constants.VersioningEnabled,
requestedVersionID: "3LgYQ7f7VxQ3",
expectedVersionID: "3LgYQ7f7VxQ3",
},
{
name: "non-versioned buckets still check current object",
versioningState: "",
requestedVersionID: "",
expectedVersionID: "",
expectedShouldCheck: true,
name: "non-versioned buckets still check current object",
versioningState: "",
requestedVersionID: "",
expectedVersionID: "",
},
{
name: "unknown versioning state without version id is checked (fail-closed for safety)",
versioningState: "UnexpectedState",
requestedVersionID: "",
expectedVersionID: "",
expectedShouldCheck: true,
name: "unknown versioning state defaults to empty version",
versioningState: "UnexpectedState",
requestedVersionID: "",
expectedVersionID: "",
},
{
name: "suspended versioning with specific version id checks that version",
versioningState: s3_constants.VersioningSuspended,
requestedVersionID: "abc123",
expectedVersionID: "abc123",
expectedShouldCheck: true,
name: "suspended versioning with specific version id checks that version",
versioningState: s3_constants.VersioningSuspended,
requestedVersionID: "abc123",
expectedVersionID: "abc123",
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
versionID, shouldCheck := objectLockVersionToCheckForDelete(tc.versioningState, tc.requestedVersionID)
versionID := objectLockVersionToCheckForDelete(tc.versioningState, tc.requestedVersionID)
assert.Equal(t, tc.expectedVersionID, versionID)
assert.Equal(t, tc.expectedShouldCheck, shouldCheck)
})
}
}