From 0938d7ee82f4b473df10a385dd460c011f4ffefe Mon Sep 17 00:00:00 2001
From: Denis Kirillov <d.kirillov@yadro.com>
Date: Tue, 17 Oct 2023 14:21:39 +0300
Subject: [PATCH] [#226] Fix status code in GET/HEAD delete marker

Signed-off-by: Denis Kirillov <d.kirillov@yadro.com>
---
 CHANGELOG.md               |  1 +
 api/handler/delete_test.go | 21 +++++++++++++++++++++
 api/handler/util.go        | 12 ++++++++++++
 api/layer/object.go        | 14 +++++++++++++-
 4 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8f7f34c..3feaa20 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 bef7b31..e837cd0 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 d2a0b78..f6e2d90 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 a2896b0..e4059c6 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 = "<continuation-token>"
 )
@@ -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) {