[awss3 input] unescape characters in s3 file names

[tetianakravchenko]

At some point there was introduced this fix #18370 to unescape characters in s3 file names.
Since then it seems that the implementation has changed and now there is similar error:

failed processing S3 event for object key "2024-02-08T08:35:00 00:00.json" in bucket "xxxx": failed to get s3 object (elapsed_time_ns=143737473): s3 GetObject failed: operation error S3: GetObject, https response error StatusCode: 404, RequestID: ...

Filname: 2024-02-08T08:35:00+00:00.json

this error seems to be coming from the GetObject call

[zmoog]

The error happens because the poller turns the original key

2024-02-08T08:35:00+00:02.json.gz

Into

2024-02-08T08:35:00 00:02.json.gz

The Poller performs this change at:

beats/x-pack/filebeat/input/awss3/s3.go

Lines 211 to 212 in aba91a4

	// Unescape s3 key name. For example, convert "%3D" back to "=".
	filename, err := url.QueryUnescape(*object.Key)

Quick test using the Go Playground:
https://go.dev/play/p/GvQcEQG607C

This only happens when using the S3 input in polling mode; the S3 input in SQS mode can successfully process the same object.

As a workaround, I recommend using the S3 input in SQS mode.

[zmoog]

It seems the list objects API doesn't escape the keys as the S3 notification did. So we don't need to unescape it.

For example, here's how the list objects API returns the following two objects in S3.

S3 objects:

S3 list objects response:

The filename, err := url.QueryUnescape(*object.Key) was originally introduced with #18370 to unescape object keys in the S3 notifications via SQS messages. However, since it is now used in polling only, it is probably no longer required and can be removed.

[zmoog]

@kaiyan-sheng, @aspacca, please let me know what you think.

If the url.QueryUnescape(*object.Key) is no longer required, I can open a PR.

[kaiyan-sheng]

@zmoog Thanks for looking into it! Yes I think we can remove the QueryUnescape in s3 polling!

[zmoog]

Yes I think we can remove the QueryUnescape in s3 polling!

Great. I'll draft a PR and run a couple of tests then.

@aspacca, let me know what you think 🙇

[aspacca]

@zmoog, yes, you're totally on spot!

The bug was already reported #33998 and I came to the same conclusion

As mentioned in my comment on that issue:

there is indeed a bug in the filebeat input, since we should unescape the s3 object key only when the value comes from an s3-sqs notification. in your case, since you are using the direct s3 listing input, the key is not escaped and we should not unescape it.

so, as @kaiyan-sheng also mentioned, we should remove QueryUnescape only when dealing with s3 polling

[aspacca]

@zmoog , sorry, please note that #18370 is the old version of the input.

it was refactored as we know, this is the place where QueryUnescape is invoked now here for sqs-s3: https://github.com/elastic/beats/blob/main/x-pack/filebeat/input/awss3/sqs_s3_event.go#L279

We should not remove this call.

But rather this one: https://github.com/elastic/beats/blob/main/x-pack/filebeat/input/awss3/s3.go#L212
That's the one made for s3-listing mode