From 278376643a46db50d6e04cc73d853f30fc1f3708 Mon Sep 17 00:00:00 2001 From: Artem Tataurov Date: Thu, 17 Nov 2022 18:01:38 +0300 Subject: [PATCH] [#221] add error response on key duplicates Signed-off-by: Artem Tataurov --- CHANGELOG.md | 1 + integration_test.go | 38 ++++++++++++++++++++++++++++++++++++++ uploader/filter.go | 22 +++++++++++++++------- uploader/filter_test.go | 22 +++++++++++++++++++++- uploader/upload.go | 7 ++++++- 5 files changed, 81 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 088df83..596457d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ If you configure application using `.yaml` file change: ### Added - Support the `Date` header on upload (#214) +- Add error response on attribute duplicates (#221) - Timeout for individual operations in streaming RPC (#234) ## [0.25.0] - 2022-10-31 diff --git a/integration_test.go b/integration_test.go index 4607ef6..d7cb8d0 100644 --- a/integration_test.go +++ b/integration_test.go @@ -67,6 +67,7 @@ func TestIntegration(t *testing.T) { require.NoError(t, err, version) t.Run("simple put "+version, func(t *testing.T) { simplePut(ctx, t, clientPool, CID, version) }) + t.Run("put with duplicate keys "+version, func(t *testing.T) { putWithDuplicateKeys(t, CID) }) t.Run("simple get "+version, func(t *testing.T) { simpleGet(ctx, t, clientPool, ownerID, CID, version) }) t.Run("get by attribute "+version, func(t *testing.T) { getByAttr(ctx, t, clientPool, ownerID, CID, version) }) t.Run("get zip "+version, func(t *testing.T) { getZip(ctx, t, clientPool, ownerID, CID, version) }) @@ -171,6 +172,43 @@ func makePutRequestAndCheck(ctx context.Context, t *testing.T, p *pool.Pool, cnr } } +func putWithDuplicateKeys(t *testing.T, CID cid.ID) { + url := testHost + "/upload/" + CID.String() + + attr := "X-Attribute-User-Attribute" + content := "content of file" + valOne, valTwo := "first_value", "second_value" + fileName := "newFile.txt" + + var buff bytes.Buffer + w := multipart.NewWriter(&buff) + fw, err := w.CreateFormFile("file", fileName) + require.NoError(t, err) + _, err = io.Copy(fw, bytes.NewBufferString(content)) + require.NoError(t, err) + err = w.Close() + require.NoError(t, err) + + request, err := http.NewRequest(http.MethodPost, url, &buff) + require.NoError(t, err) + request.Header.Set("Content-Type", w.FormDataContentType()) + request.Header.Add(attr, valOne) + request.Header.Add(attr, valTwo) + + resp, err := http.DefaultClient.Do(request) + require.NoError(t, err) + + defer func() { + err := resp.Body.Close() + require.NoError(t, err) + }() + + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.Equal(t, "key duplication error: "+attr+"\n", string(body)) + require.Equal(t, http.StatusBadRequest, resp.StatusCode) +} + func simpleGet(ctx context.Context, t *testing.T, clientPool *pool.Pool, ownerID user.ID, CID cid.ID, version string) { content := "content of file" attributes := map[string]string{ diff --git a/uploader/filter.go b/uploader/filter.go index e453cfe..c7349c7 100644 --- a/uploader/filter.go +++ b/uploader/filter.go @@ -26,7 +26,8 @@ func systemTranslator(key, prefix []byte) []byte { return bytes.ToUpper(key) } -func filterHeaders(l *zap.Logger, header *fasthttp.RequestHeader) map[string]string { +func filterHeaders(l *zap.Logger, header *fasthttp.RequestHeader) (map[string]string, error) { + var err error result := make(map[string]string) prefix := []byte(utils.UserAttributeHeaderPrefix) @@ -42,23 +43,30 @@ func filterHeaders(l *zap.Logger, header *fasthttp.RequestHeader) map[string]str } // removing attribute prefix - key = bytes.TrimPrefix(key, prefix) + clearKey := bytes.TrimPrefix(key, prefix) // checks that it's a system NeoFS header for _, system := range neofsAttributeHeaderPrefixes { - if bytes.HasPrefix(key, system) { - key = systemTranslator(key, system) + if bytes.HasPrefix(clearKey, system) { + clearKey = systemTranslator(clearKey, system) break } } // checks that the attribute key is not empty - if len(key) == 0 { + if len(clearKey) == 0 { + return + } + + // check if key gets duplicated + // return error containing full key name (with prefix) + if _, ok := result[string(clearKey)]; ok { + err = fmt.Errorf("key duplication error: %s", string(key)) return } // make string representation of key / val - k, v := string(key), string(val) + k, v := string(clearKey), string(val) result[k] = v @@ -67,7 +75,7 @@ func filterHeaders(l *zap.Logger, header *fasthttp.RequestHeader) map[string]str zap.String("val", v)) }) - return result + return result, err } func prepareExpirationHeader(headers map[string]string, epochDurations *epochDurations, now time.Time) error { diff --git a/uploader/filter_test.go b/uploader/filter_test.go index 585fac8..de4c541 100644 --- a/uploader/filter_test.go +++ b/uploader/filter_test.go @@ -16,8 +16,27 @@ import ( func TestFilter(t *testing.T) { log := zap.NewNop() + t.Run("duplicate keys error", func(t *testing.T) { + req := &fasthttp.RequestHeader{} + req.DisableNormalizing() + req.Add("X-Attribute-DupKey", "first-value") + req.Add("X-Attribute-DupKey", "second-value") + _, err := filterHeaders(log, req) + require.Error(t, err) + }) + + t.Run("duplicate system keys error", func(t *testing.T) { + req := &fasthttp.RequestHeader{} + req.DisableNormalizing() + req.Add("X-Attribute-Neofs-DupKey", "first-value") + req.Add("X-Attribute-Neofs-DupKey", "second-value") + _, err := filterHeaders(log, req) + require.Error(t, err) + }) + req := &fasthttp.RequestHeader{} req.DisableNormalizing() + req.Set("X-Attribute-Neofs-Expiration-Epoch1", "101") req.Set("X-Attribute-NEOFS-Expiration-Epoch2", "102") req.Set("X-Attribute-neofs-Expiration-Epoch3", "103") @@ -30,7 +49,8 @@ func TestFilter(t *testing.T) { "__NEOFS__EXPIRATION_EPOCH2": "102", } - result := filterHeaders(log, req) + result, err := filterHeaders(log, req) + require.NoError(t, err) require.Equal(t, expected, result) } diff --git a/uploader/upload.go b/uploader/upload.go index 8e65456..2effd16 100644 --- a/uploader/upload.go +++ b/uploader/upload.go @@ -114,7 +114,12 @@ func (u *Uploader) Upload(c *fasthttp.RequestCtx) { response.Error(c, "could not receive multipart/form: "+err.Error(), fasthttp.StatusBadRequest) return } - filtered := filterHeaders(u.log, &c.Request.Header) + filtered, err := filterHeaders(u.log, &c.Request.Header) + if err != nil { + log.Error("could not process headers", zap.Error(err)) + response.Error(c, err.Error(), fasthttp.StatusBadRequest) + return + } if needParseExpiration(filtered) { epochDuration, err := getEpochDurations(c, u.pool) if err != nil {