[8.13](backport #38125) [AWS] [S3] Remove url.QueryUnescape() from aw…

…s-s3 input in polling mode (#38165) * [AWS] [S3] Remove url.QueryUnescape() from aws-s3 input in polling mode (#38125) We introduced [^1] the `url.QueryUnescape()` function to unescape object keys from S3 notification in SQS messages. However, the object keys in the S3 list object responses do not require [^2] unescape. We must remove the unescape to avoid unintended changes to the S3 object key. [^1]: #18370 [^2]: #38012 (comment) --------- Co-authored-by: Andrea Spacca <[email protected]>
elastic · Mar 4, 2024 · 966cceb · 966cceb
1 parent 723601f
commit 966cceb
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 10 deletions.
diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc
@@ -95,6 +95,7 @@ fields added to events containing the Beats version. {pull}37553[37553]
 - Update github.com/lestrrat-go/jwx dependency. {pull}37799[37799]
 - [threatintel] MISP pagination fixes {pull}37898[37898]
 - Fix file handle leak when handling errors in filestream {pull}37973[37973]
+- Fix "failed processing S3 event for object key" error on aws-s3 input when key contains the "+" character {issue}38012[38012] {pull}38125[38125]
 
 *Heartbeat*
 

diff --git a/x-pack/filebeat/input/awss3/s3.go b/x-pack/filebeat/input/awss3/s3.go
@@ -8,7 +8,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"net/url"
 	"sync"
 	"time"
 
@@ -208,14 +207,7 @@ func (p *s3Poller) GetS3Objects(ctx context.Context, s3ObjectPayloadChan chan<-
 		// Metrics
 		p.metrics.s3ObjectsListedTotal.Add(uint64(totListedObjects))
 		for _, object := range page.Contents {
-			// Unescape s3 key name. For example, convert "%3D" back to "=".
-			filename, err := url.QueryUnescape(*object.Key)
-			if err != nil {
-				p.log.Errorw("Error when unescaping object key, skipping.", "error", err, "s3_object", *object.Key)
-				continue
-			}
-
-			state := newState(bucketName, filename, *object.ETag, p.listPrefix, *object.LastModified)
+			state := newState(bucketName, *object.Key, *object.ETag, p.listPrefix, *object.LastModified)
 			if p.states.MustSkip(state, p.store) {
 				p.log.Debugw("skipping state.", "state", state)
 				continue
@@ -240,7 +232,7 @@ func (p *s3Poller) GetS3Objects(ctx context.Context, s3ObjectPayloadChan chan<-
 				s3ObjectHandler: s3Processor,
 				s3ObjectInfo: s3ObjectInfo{
 					name:         bucketName,
-					key:          filename,
+					key:          *object.Key,
 					etag:         *object.ETag,
 					lastModified: *object.LastModified,
 					listingID:    listingID.String(),

diff --git a/x-pack/filebeat/input/awss3/s3_test.go b/x-pack/filebeat/input/awss3/s3_test.go
@@ -93,6 +93,11 @@ func TestS3Poller(t *testing.T) {
 							Key:          aws.String("key5"),
 							LastModified: aws.Time(time.Now()),
 						},
+						{
+							ETag:         aws.String("etag6"),
+							Key:          aws.String("2024-02-08T08:35:00+00:02.json.gz"),
+							LastModified: aws.Time(time.Now()),
+						},
 					},
 				}, nil
 			})
@@ -124,6 +129,10 @@ func TestS3Poller(t *testing.T) {
 			GetObject(gomock.Any(), gomock.Eq(bucket), gomock.Eq("key5")).
 			Return(nil, errFakeConnectivityFailure)
 
+		mockAPI.EXPECT().
+			GetObject(gomock.Any(), gomock.Eq(bucket), gomock.Eq("2024-02-08T08:35:00+00:02.json.gz")).
+			Return(nil, errFakeConnectivityFailure)
+
 		s3ObjProc := newS3ObjectProcessorFactory(logp.NewLogger(inputName), nil, mockAPI, nil, backupConfig{}, numberOfWorkers)
 		receiver := newS3Poller(logp.NewLogger(inputName), nil, mockAPI, mockPublisher, s3ObjProc, newStates(inputCtx), store, bucket, "key", "region", "provider", numberOfWorkers, pollInterval)
 		require.Error(t, context.DeadlineExceeded, receiver.Poll(ctx))