[#151] Fix getting s3 object with the FrostFS OID name

Prioritize getting s3 object with the key, which equals to valid FrostFS OID, rather than getting non-existent object with OID via native protocol for GET and HEAD requests

Signed-off-by: Nikita Zinkevich <n.zinkevich@yadro.com>

internal/handler/download.go

@@ -4,14 +4,18 @@ import (
 	"archive/zip"
 	"bufio"
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
 	"net/url"
 	"time"
 
+	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/api"
+	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/data"
 	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/logs"
 	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/response"
+	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/tree"
 	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/utils"
 	"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/bearer"
 	cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id"
@@ -21,23 +25,70 @@ import (
 	"go.uber.org/zap"
 )
 
+var errObjectDeleted = errors.New("object deleted")
+
 // DownloadByAddressOrBucketName handles download requests using simple cid/oid or bucketname/key format.
 func (h *Handler) DownloadByAddressOrBucketName(c *fasthttp.RequestCtx) {
-	oidURLParam := c.UserValue("oid").(string)
+	cidParam := c.UserValue("cid").(string)
-	downloadQueryParam := c.QueryArgs().GetBool("download")
+	oidParam := c.UserValue("oid").(string)
+	downloadParam := c.QueryArgs().GetBool("download")
 
-	switch {
+	ctx := utils.GetContextFromRequest(c)
-	case isObjectID(oidURLParam):
+	log := utils.GetReqLogOrDefault(ctx, h.log)
-		h.byNativeAddress(c, h.receiveFile)
+
-	case !isContainerRoot(oidURLParam) && (downloadQueryParam || !isDir(oidURLParam)):
+	bktInfo, err := h.getBucketInfo(ctx, cidParam, log)
-		h.byS3Path(c, h.receiveFile)
+	if err != nil {
-	default:
+		logAndSendBucketError(c, log, err)
+		return
+	}
+
+	foundOid, err := h.checkS3ObjectExist(ctx, bktInfo, oidParam)
+	if errors.Is(err, errObjectDeleted) {
+		log.Error(logs.ObjectWasDeleted)
+		response.Error(c, "object deleted", fasthttp.StatusNotFound)
+		return
+	}
+	if err != nil && !errors.Is(err, tree.ErrNodeNotFound) {
+		logAndSendBucketError(c, log, err)
+		return
+	}
+
+	var objID oid.ID
+	if foundOid != nil && shouldDownload(oidParam, downloadParam) {
+		// Receive file via S3
+		addr := newAddress(bktInfo.CID, foundOid.OID)
+		h.receiveFile(ctx, h.newRequest(c, log), addr)
+	} else if err = objID.DecodeString(oidParam); err == nil {
+		// Receive file via native protocol
+		addr := newAddress(bktInfo.CID, objID)
+		h.receiveFile(ctx, h.newRequest(c, log), addr)
+	} else {
 		h.browseIndex(c)
 	}
 }
 
-func (h *Handler) newRequest(ctx *fasthttp.RequestCtx, log *zap.Logger) *request {
+func shouldDownload(oidParam string, downloadParam bool) bool {
-	return &request{
+	return !isDir(oidParam) || downloadParam
+}
+
+func (h *Handler) checkS3ObjectExist(ctx context.Context, bktInfo *data.BucketInfo, oidParam string) (*api.NodeVersion, error) {
+	unescapedKey, err := url.QueryUnescape(oidParam)
+	if err != nil {
+		return nil, err
+	}
+	foundOid, err := h.tree.GetLatestVersion(ctx, &bktInfo.CID, unescapedKey)
+	if err != nil {
+		return nil, err
+	}
+	if foundOid.DeleteMarker {
+		return nil, errObjectDeleted
+	}
+
+	return foundOid, err
+}
+
+func (h *Handler) newRequest(ctx *fasthttp.RequestCtx, log *zap.Logger) request {
+	return request{
 		RequestCtx: ctx,
 		log:        log,
 	}
@@ -45,7 +96,64 @@ func (h *Handler) newRequest(ctx *fasthttp.RequestCtx, log *zap.Logger) *request
 
 // DownloadByAttribute handles attribute-based download requests.
 func (h *Handler) DownloadByAttribute(c *fasthttp.RequestCtx) {
-	h.byAttribute(c, h.receiveFile)
+	scid, _ := c.UserValue("cid").(string)
+	key, _ := c.UserValue("attr_key").(string)
+	val, _ := c.UserValue("attr_val").(string)
+
+	ctx := utils.GetContextFromRequest(c)
+	log := utils.GetReqLogOrDefault(ctx, h.log)
+
+	key, err := url.QueryUnescape(key)
+	if err != nil {
+		log.Error(logs.FailedToUnescapeQuery, zap.String("cid", scid), zap.String("attr_key", key), zap.Error(err))
+		response.Error(c, "could not unescape attr_key: "+err.Error(), fasthttp.StatusBadRequest)
+		return
+	}
+
+	val, err = url.QueryUnescape(val)
+	if err != nil {
+		log.Error(logs.FailedToUnescapeQuery, zap.String("cid", scid), zap.String("attr_val", val), zap.Error(err))
+		response.Error(c, "could not unescape attr_val: "+err.Error(), fasthttp.StatusBadRequest)
+		return
+	}
+
+	log = log.With(zap.String("cid", scid), zap.String("attr_key", key), zap.String("attr_val", val))
+
+	bktInfo, err := h.getBucketInfo(ctx, scid, log)
+	if err != nil {
+		logAndSendBucketError(c, log, err)
+		return
+	}
+
+	res, err := h.search(ctx, bktInfo.CID, key, val, object.MatchStringEqual)
+	if err != nil {
+		log.Error(logs.CouldNotSearchForObjects, zap.Error(err))
+		response.Error(c, "could not search for objects: "+err.Error(), fasthttp.StatusBadRequest)
+		return
+	}
+
+	defer res.Close()
+
+	buf := make([]oid.ID, 1)
+
+	n, err := res.Read(buf)
+	if n == 0 {
+		if errors.Is(err, io.EOF) {
+			log.Error(logs.ObjectNotFound, zap.Error(err))
+			response.Error(c, "object not found", fasthttp.StatusNotFound)
+			return
+		}
+
+		log.Error(logs.ReadObjectListFailed, zap.Error(err))
+		response.Error(c, "read object list failed: "+err.Error(), fasthttp.StatusBadRequest)
+		return
+	}
+
+	var addr oid.Address
+	addr.SetContainer(bktInfo.CID)
+	addr.SetObject(buf[0])
+
+	h.receiveFile(ctx, h.newRequest(c, log), addr)
 }
 
 func (h *Handler) search(ctx context.Context, cnrID cid.ID, key, val string, op object.SearchMatchType) (ResObjectSearch, error) {

internal/handler/handler.go

@@ -12,7 +12,6 @@ import (
 	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/data"
 	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/handler/middleware"
 	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/logs"
-	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/response"
 	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/tree"
 	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/utils"
 	"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/bearer"
@@ -190,138 +189,6 @@ func New(params *AppParams, config Config, tree *tree.Tree, workerPool *ants.Poo
 	}
 }
 
-// byNativeAddress is a wrapper for function (e.g. request.headObject, request.receiveFile) that
-// prepares request and object address to it.
-func (h *Handler) byNativeAddress(c *fasthttp.RequestCtx, f func(context.Context, request, oid.Address)) {
-	idCnr, _ := c.UserValue("cid").(string)
-	idObj, _ := url.PathUnescape(c.UserValue("oid").(string))
-
-	ctx := utils.GetContextFromRequest(c)
-	reqLog := utils.GetReqLogOrDefault(ctx, h.log)
-	log := reqLog.With(zap.String("cid", idCnr), zap.String("oid", idObj))
-
-	bktInfo, err := h.getBucketInfo(ctx, idCnr, log)
-	if err != nil {
-		logAndSendBucketError(c, log, err)
-		return
-	}
-
-	objID := new(oid.ID)
-	if err = objID.DecodeString(idObj); err != nil {
-		log.Error(logs.WrongObjectID, zap.Error(err))
-		response.Error(c, "wrong object id", fasthttp.StatusBadRequest)
-		return
-	}
-
-	addr := newAddress(bktInfo.CID, *objID)
-
-	f(ctx, *h.newRequest(c, log), addr)
-}
-
-// byS3Path is a wrapper for function (e.g. request.headObject, request.receiveFile) that
-// resolves object address from S3-like path <bucket name>/<object key>.
-func (h *Handler) byS3Path(c *fasthttp.RequestCtx, f func(context.Context, request, oid.Address)) {
-	bucketname := c.UserValue("cid").(string)
-	key := c.UserValue("oid").(string)
-
-	ctx := utils.GetContextFromRequest(c)
-	reqLog := utils.GetReqLogOrDefault(ctx, h.log)
-	log := reqLog.With(zap.String("bucketname", bucketname), zap.String("key", key))
-
-	unescapedKey, err := url.QueryUnescape(key)
-	if err != nil {
-		logAndSendBucketError(c, log, err)
-		return
-	}
-
-	bktInfo, err := h.getBucketInfo(ctx, bucketname, log)
-	if err != nil {
-		logAndSendBucketError(c, log, err)
-		return
-	}
-
-	foundOid, err := h.tree.GetLatestVersion(ctx, &bktInfo.CID, unescapedKey)
-	if err != nil {
-		if errors.Is(err, tree.ErrNodeAccessDenied) {
-			response.Error(c, "Access Denied", fasthttp.StatusForbidden)
-		} else {
-			response.Error(c, "object wasn't found", fasthttp.StatusNotFound)
-			log.Error(logs.GetLatestObjectVersion, zap.Error(err))
-		}
-		return
-	}
-	if foundOid.DeleteMarker {
-		log.Error(logs.ObjectWasDeleted)
-		response.Error(c, "object deleted", fasthttp.StatusNotFound)
-		return
-	}
-	addr := newAddress(bktInfo.CID, foundOid.OID)
-
-	f(ctx, *h.newRequest(c, log), addr)
-}
-
-// byAttribute is a wrapper similar to byNativeAddress.
-func (h *Handler) byAttribute(c *fasthttp.RequestCtx, f func(context.Context, request, oid.Address)) {
-	scid, _ := c.UserValue("cid").(string)
-	key, _ := c.UserValue("attr_key").(string)
-	val, _ := c.UserValue("attr_val").(string)
-
-	ctx := utils.GetContextFromRequest(c)
-	log := utils.GetReqLogOrDefault(ctx, h.log)
-
-	key, err := url.QueryUnescape(key)
-	if err != nil {
-		log.Error(logs.FailedToUnescapeQuery, zap.String("cid", scid), zap.String("attr_key", key), zap.Error(err))
-		response.Error(c, "could not unescape attr_key: "+err.Error(), fasthttp.StatusBadRequest)
-		return
-	}
-
-	val, err = url.QueryUnescape(val)
-	if err != nil {
-		log.Error(logs.FailedToUnescapeQuery, zap.String("cid", scid), zap.String("attr_val", val), zap.Error(err))
-		response.Error(c, "could not unescape attr_val: "+err.Error(), fasthttp.StatusBadRequest)
-		return
-	}
-
-	log = log.With(zap.String("cid", scid), zap.String("attr_key", key), zap.String("attr_val", val))
-
-	bktInfo, err := h.getBucketInfo(ctx, scid, log)
-	if err != nil {
-		logAndSendBucketError(c, log, err)
-		return
-	}
-
-	res, err := h.search(ctx, bktInfo.CID, key, val, object.MatchStringEqual)
-	if err != nil {
-		log.Error(logs.CouldNotSearchForObjects, zap.Error(err))
-		response.Error(c, "could not search for objects: "+err.Error(), fasthttp.StatusBadRequest)
-		return
-	}
-
-	defer res.Close()
-
-	buf := make([]oid.ID, 1)
-
-	n, err := res.Read(buf)
-	if n == 0 {
-		if errors.Is(err, io.EOF) {
-			log.Error(logs.ObjectNotFound, zap.Error(err))
-			response.Error(c, "object not found", fasthttp.StatusNotFound)
-			return
-		}
-
-		log.Error(logs.ReadObjectListFailed, zap.Error(err))
-		response.Error(c, "read object list failed: "+err.Error(), fasthttp.StatusBadRequest)
-		return
-	}
-
-	var addrObj oid.Address
-	addrObj.SetContainer(bktInfo.CID)
-	addrObj.SetObject(buf[0])
-
-	f(ctx, *h.newRequest(c, log), addrObj)
-}
-
 // resolveContainer decode container id, if it's not a valid container id
 // then trey to resolve name using provided resolver.
 func (h *Handler) resolveContainer(ctx context.Context, containerID string) (*cid.ID, error) {

internal/handler/head.go

@@ -2,12 +2,16 @@ package handler
 
 import (
 	"context"
+	"errors"
 	"io"
 	"net/http"
+	"net/url"
 	"strconv"
 	"time"
 
 	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/logs"
+	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/response"
+	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/tree"
 	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/utils"
 	"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object"
 	oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id"
@@ -102,18 +106,99 @@ func idsToResponse(resp *fasthttp.Response, obj *object.Object) {
 
 // HeadByAddressOrBucketName handles head requests using simple cid/oid or bucketname/key format.
 func (h *Handler) HeadByAddressOrBucketName(c *fasthttp.RequestCtx) {
-	test, _ := c.UserValue("oid").(string)
+	cidParam, _ := c.UserValue("cid").(string)
-	var id oid.ID
+	oidParam, _ := c.UserValue("oid").(string)
 
-	err := id.DecodeString(test)
+	ctx := utils.GetContextFromRequest(c)
+	log := utils.GetReqLogOrDefault(ctx, h.log).With(
+		zap.String("cid", cidParam),
+		zap.String("oid", oidParam),
+	)
+
+	bktInfo, err := h.getBucketInfo(ctx, cidParam, log)
 	if err != nil {
-		h.byS3Path(c, h.headObject)
+		logAndSendBucketError(c, log, err)
+		return
+	}
+	foundOid, checkErr := h.checkS3ObjectExist(ctx, bktInfo, oidParam)
+	if checkErr != nil && !errors.Is(checkErr, tree.ErrNodeNotFound) {
+		logAndSendBucketError(c, log, checkErr)
+		return
+	}
+
+	var objID oid.ID
+	if foundOid != nil {
+		// Head object via S3
+		addr := newAddress(bktInfo.CID, foundOid.OID)
+		h.headObject(ctx, h.newRequest(c, log), addr)
+	} else if err = objID.DecodeString(oidParam); err == nil {
+		// Head object via native protocol
+		addr := newAddress(bktInfo.CID, objID)
+		h.headObject(ctx, h.newRequest(c, log), addr)
 	} else {
-		h.byNativeAddress(c, h.headObject)
+		logAndSendBucketError(c, log, checkErr)
+		return
 	}
 }
 
 // HeadByAttribute handles attribute-based head requests.
 func (h *Handler) HeadByAttribute(c *fasthttp.RequestCtx) {
-	h.byAttribute(c, h.headObject)
+	cidParam, _ := c.UserValue("cid").(string)
+	key, _ := c.UserValue("attr_key").(string)
+	val, _ := c.UserValue("attr_val").(string)
+
+	ctx := utils.GetContextFromRequest(c)
+	log := utils.GetReqLogOrDefault(ctx, h.log)
+
+	key, err := url.QueryUnescape(key)
+	if err != nil {
+		log.Error(logs.FailedToUnescapeQuery, zap.String("cid", cidParam), zap.String("attr_key", key), zap.Error(err))
+		response.Error(c, "could not unescape attr_key: "+err.Error(), fasthttp.StatusBadRequest)
+		return
+	}
+
+	val, err = url.QueryUnescape(val)
+	if err != nil {
+		log.Error(logs.FailedToUnescapeQuery, zap.String("cid", cidParam), zap.String("attr_val", val), zap.Error(err))
+		response.Error(c, "could not unescape attr_val: "+err.Error(), fasthttp.StatusBadRequest)
+		return
+	}
+
+	log = log.With(zap.String("cid", cidParam), zap.String("attr_key", key), zap.String("attr_val", val))
+
+	bktInfo, err := h.getBucketInfo(ctx, cidParam, log)
+	if err != nil {
+		logAndSendBucketError(c, log, err)
+		return
+	}
+
+	res, err := h.search(ctx, bktInfo.CID, key, val, object.MatchStringEqual)
+	if err != nil {
+		log.Error(logs.CouldNotSearchForObjects, zap.Error(err))
+		response.Error(c, "could not search for objects: "+err.Error(), fasthttp.StatusBadRequest)
+		return
+	}
+
+	defer res.Close()
+
+	buf := make([]oid.ID, 1)
+
+	n, err := res.Read(buf)
+	if n == 0 {
+		if errors.Is(err, io.EOF) {
+			log.Error(logs.ObjectNotFound, zap.Error(err))
+			response.Error(c, "object not found", fasthttp.StatusNotFound)
+			return
+		}
+
+		log.Error(logs.ReadObjectListFailed, zap.Error(err))
+		response.Error(c, "read object list failed: "+err.Error(), fasthttp.StatusBadRequest)
+		return
+	}
+
+	var addr oid.Address
+	addr.SetContainer(bktInfo.CID)
+	addr.SetObject(buf[0])
+
+	h.headObject(ctx, h.newRequest(c, log), addr)
 }

internal/handler/utils.go

@@ -45,15 +45,6 @@ func isDir(name string) bool {
 	return strings.HasSuffix(name, "/")
 }
 
-func isObjectID(s string) bool {
-	var objID oid.ID
-	return objID.DecodeString(s) == nil
-}
-
-func isContainerRoot(key string) bool {
-	return key == ""
-}
-
 func loadAttributes(attrs []object.Attribute) map[string]string {
 	result := make(map[string]string)
 	for _, attr := range attrs {