From fcf1c45ad21177f4df306fdc1eb736823244715d Mon Sep 17 00:00:00 2001
From: Denis Kirillov <d.kirillov@yadro.com>
Date: Thu, 17 Aug 2023 11:54:11 +0300
Subject: [PATCH] [#188] Fix url escaping

Url escaping has already been done in `net/http/request.go`

Signed-off-by: Denis Kirillov <d.kirillov@yadro.com>
---
 CHANGELOG.md                    |  1 +
 api/handler/object_list_test.go | 20 +++++++++++++++
 api/middleware/reqinfo.go       |  9 +------
 api/router_test.go              | 45 +++++++++++++++++++++++++++++++++
 4 files changed, 67 insertions(+), 8 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 00451eae..69120695 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -13,6 +13,7 @@ This document outlines major changes between releases.
 - Replace part on re-upload when use multipart upload (#176)
 - Fix goroutine leak on put object error (#178)
 - Fix parsing signed headers in presigned urls (#182)
+- Fix url escaping (#188)
 
 ### Added
 - Add a metric with addresses of nodes of the same and highest priority that are currently healthy (#51)
diff --git a/api/handler/object_list_test.go b/api/handler/object_list_test.go
index d4a96346..a91c4284 100644
--- a/api/handler/object_list_test.go
+++ b/api/handler/object_list_test.go
@@ -100,6 +100,26 @@ func TestS3BucketListDelimiterBasic(t *testing.T) {
 	require.Equal(t, "quux/", listV1Response.CommonPrefixes[1].Prefix)
 }
 
+func TestS3BucketListV2DelimiterPercentage(t *testing.T) {
+	tc := prepareHandlerContext(t)
+
+	bktName := "bucket-for-listing"
+	objects := []string{"b%ar", "b%az", "c%ab", "foo"}
+	bktInfo, _ := createBucketAndObject(tc, bktName, objects[0])
+
+	for _, objName := range objects[1:] {
+		createTestObject(tc, bktInfo, objName)
+	}
+
+	listV2Response := listObjectsV2(t, tc, bktName, "", "%", "", "", -1)
+	require.Equal(t, "%", listV2Response.Delimiter)
+	require.Len(t, listV2Response.Contents, 1)
+	require.Equal(t, "foo", listV2Response.Contents[0].Key)
+	require.Len(t, listV2Response.CommonPrefixes, 2)
+	require.Equal(t, "b%", listV2Response.CommonPrefixes[0].Prefix)
+	require.Equal(t, "c%", listV2Response.CommonPrefixes[1].Prefix)
+}
+
 func TestS3BucketListV2DelimiterPrefix(t *testing.T) {
 	tc := prepareHandlerContext(t)
 
diff --git a/api/middleware/reqinfo.go b/api/middleware/reqinfo.go
index 4e34b1cf..524288ea 100644
--- a/api/middleware/reqinfo.go
+++ b/api/middleware/reqinfo.go
@@ -242,14 +242,7 @@ func AddObjectName(l *zap.Logger) Func {
 
 			rctx := chi.RouteContext(ctx)
 			// trim leading slash (always present)
-			obj := rctx.RoutePath[1:]
-
-			object, err := url.PathUnescape(obj)
-			if err != nil {
-				object = obj
-			}
-
-			reqInfo.ObjectName = object
+			reqInfo.ObjectName = rctx.RoutePath[1:]
 
 			reqLogger := reqLogOrDefault(ctx, l)
 			r = r.WithContext(SetReqLogger(ctx, reqLogger.With(zap.String("object", reqInfo.ObjectName))))
diff --git a/api/router_test.go b/api/router_test.go
index 2142485e..153eec51 100644
--- a/api/router_test.go
+++ b/api/router_test.go
@@ -61,6 +61,51 @@ func TestRouterObjectWithSlashes(t *testing.T) {
 	require.Equal(t, objName, resp.ReqInfo.ObjectName)
 }
 
+func TestRouterObjectEscaping(t *testing.T) {
+	chiRouter := prepareRouter(t)
+
+	bktName := "dkirillov"
+
+	for _, tc := range []struct {
+		name            string
+		expectedObjName string
+		objName         string
+	}{
+		{
+			name:            "simple",
+			expectedObjName: "object",
+			objName:         "object",
+		},
+		{
+			name:            "with slashes",
+			expectedObjName: "fix/object",
+			objName:         "fix/object",
+		},
+		{
+			name:            "with percentage",
+			expectedObjName: "fix/object%ac",
+			objName:         "fix/object%ac",
+		},
+		{
+			name:            "with percentage escaped",
+			expectedObjName: "fix/object%ac",
+			objName:         "fix/object%25ac",
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			target := fmt.Sprintf("/%s/%s", bktName, tc.objName)
+
+			w := httptest.NewRecorder()
+			r := httptest.NewRequest(http.MethodPut, target, nil)
+
+			chiRouter.ServeHTTP(w, r)
+			resp := readResponse(t, w)
+			require.Equal(t, "PutObject", resp.Method)
+			require.Equal(t, tc.expectedObjName, resp.ReqInfo.ObjectName)
+		})
+	}
+}
+
 func prepareRouter(t *testing.T) *chi.Mux {
 	throttleOps := middleware.ThrottleOpts{
 		Limit:          10,