IfNoneMatch on GetObject fails when ETag is enclosed in quotes

[ginkel]

Hi there,

AFAICS the noneMatch (and match) check in com.adobe.testing.s3mock.service.ObjectService#verifyObjectMatching always fails if the supplied ETag is enclosed in quotes (which is a requirement if I understand https://github.com/adobe/S3Mock/issues/801 correctly), i.e., the GetObject operation never returns NOT_MODIFIED.

Thanks, Thilo

P.S.: Tested w/ version 2.6.1

[afranken]

thanks for the report. It seems that our etag handling is still inconsistent. There also does not seem to be a unit or integration test for the "if (not) modified since" use-case...

[afranken]

This turned out to be more complex than I thought - will have to try out a few more things locally and against a real S3 object... Read the RFCs https://www.rfc-editor.org/rfc/rfc2616#section-14.19 https://www.rfc-editor.org/rfc/rfc7232 which seem to be partially implemented by the S3 backend.

Noticed several challenges for this fix:

• There is only one integration test for the ETag behaviour, and it makes the wrong assumptions: GetPutDeleteObjectV1IT#shouldCreateAndRespectEtag
• There are no unit-tests for ETag functionality
• Seems like AWS SDK v1 and v2 handle ETags differently. For example, when doing a PutObject in v1, the enclosing quotes are stripped out when retrieving the ETag value from the response. If the value should be used in a subsequent request against S3, you need to add the quotes again. When doing the same in v2, the PutObject response ETag still contains the enclosing quotes, so you can immediately pass that value to a subsequent request.
• v1 requests can pass a list of ETags for "match" / "none-match", while in v2 it's only a single value. We will have to support both.
• There are a few methods defined in the API that (for example HeadObject) that support "match" / "none-match", which are not correctly implemented in S3Mock.
• It is not possible to add "match"/"none-match" to PutObject requests although according to the RFCs it should to prevent the "lost update" problem... Weird, do they expect the client to make a HeadObject call first?
• S3Mock would need to support the "*" use-case as well, where this would match any Etag for an existing object, for both the "match" and "none-match" use-cases.

And there are probably several other things I forgot to write down here...

[afranken]

@ginkel just released 2.6.3 fixing ETag handling yet again - hopefully for good!

[ginkel]

@afranken Works like a charm, thank you very much! :-)