diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f7f34cd..3feaa206 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ This document outlines major changes between releases. - Use correct keys in `list-multipart-uploads` response (#185) - Fix parsing `key-marker` for object list versions (#243) - Fix marshaling errors in `DeleteObjects` method (#222) +- Fix status code in GET/HEAD delete marker (#226) ### Added - Add `trace_id` value into log record when tracing is enabled (#142) diff --git a/api/handler/delete_test.go b/api/handler/delete_test.go index bef7b312..e837cd02 100644 --- a/api/handler/delete_test.go +++ b/api/handler/delete_test.go @@ -363,6 +363,27 @@ func TestDeleteMarkers(t *testing.T) { require.Len(t, listOIDsFromMockedFrostFS(t, tc, bktName), 0, "shouldn't be any object in frostfs") } +func TestGetHeadDeleteMarker(t *testing.T) { + hc := prepareHandlerContext(t) + + bktName, objName := "bucket-for-removal", "object-to-delete" + createTestBucket(hc, bktName) + putBucketVersioning(t, hc, bktName, true) + + putObject(hc, bktName, objName) + + deleteMarkerVersionID, _ := deleteObject(t, hc, bktName, objName, emptyVersion) + + w := headObjectBase(hc, bktName, objName, deleteMarkerVersionID) + require.Equal(t, w.Code, http.StatusMethodNotAllowed) + require.Equal(t, w.Result().Header.Get(api.AmzDeleteMarker), "true") + + w, r := prepareTestRequest(hc, bktName, objName, nil) + hc.Handler().GetObjectHandler(w, r) + assertStatus(hc.t, w, http.StatusNotFound) + require.Equal(t, w.Result().Header.Get(api.AmzDeleteMarker), "true") +} + func TestDeleteObjectFromListCache(t *testing.T) { tc := prepareHandlerContext(t) diff --git a/api/handler/util.go b/api/handler/util.go index d2a0b78a..f6e2d902 100644 --- a/api/handler/util.go +++ b/api/handler/util.go @@ -3,6 +3,7 @@ package handler import ( "context" "errors" + "fmt" "net/http" "strconv" "strings" @@ -28,6 +29,7 @@ func (h *handler) reqLogger(ctx context.Context) *zap.Logger { } func (h *handler) logAndSendError(w http.ResponseWriter, logText string, reqInfo *middleware.ReqInfo, err error, additional ...zap.Field) { + err = handleDeleteMarker(w, err) code := middleware.WriteErrorResponse(w, reqInfo, transformToS3Error(err)) fields := []zap.Field{ zap.Int("status", code), @@ -44,6 +46,16 @@ func (h *handler) logAndSendError(w http.ResponseWriter, logText string, reqInfo h.log.Error(logs.RequestFailed, fields...) // consider using h.reqLogger (it requires accept context.Context or http.Request) } +func handleDeleteMarker(w http.ResponseWriter, err error) error { + var target layer.DeleteMarkerError + if !errors.As(err, &target) { + return err + } + + w.Header().Set(api.AmzDeleteMarker, "true") + return fmt.Errorf("%w: %s", s3errors.GetAPIError(target.ErrorCode), err) +} + func (h *handler) logAndSendErrorNoHeader(w http.ResponseWriter, logText string, reqInfo *middleware.ReqInfo, err error, additional ...zap.Field) { middleware.WriteErrorResponseNoHeader(w, reqInfo, transformToS3Error(err)) fields := []zap.Field{ diff --git a/api/layer/object.go b/api/layer/object.go index a2896b02..e4059c69 100644 --- a/api/layer/object.go +++ b/api/layer/object.go @@ -80,8 +80,16 @@ type ( Marker string ContinuationToken string } + + DeleteMarkerError struct { + ErrorCode apiErrors.ErrorCode + } ) +func (e DeleteMarkerError) Error() string { + return "object is delete marker" +} + const ( continuationToken = "" ) @@ -389,7 +397,7 @@ func (n *layer) headLastVersionIfNotDeleted(ctx context.Context, bkt *data.Bucke } if node.IsDeleteMarker() { - return nil, fmt.Errorf("%w: found version is delete marker", apiErrors.GetAPIError(apiErrors.ErrNoSuchKey)) + return nil, DeleteMarkerError{ErrorCode: apiErrors.ErrNoSuchKey} } meta, err := n.objectHead(ctx, bkt, node.OID) @@ -445,6 +453,10 @@ func (n *layer) headVersion(ctx context.Context, bkt *data.BucketInfo, p *HeadOb return extObjInfo, nil } + if foundVersion.IsDeleteMarker() { + return nil, DeleteMarkerError{ErrorCode: apiErrors.ErrMethodNotAllowed} + } + meta, err := n.objectHead(ctx, bkt, foundVersion.OID) if err != nil { if client.IsErrObjectNotFound(err) {