This is an automated email from the ASF dual-hosted git repository.
chibenwa pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
The following commit(s) were added to refs/heads/master by this push:
new 98222025ba JAMES-4223 Default S3 If-None-Match to true to prevent
silent blob overwrite
98222025ba is described below
commit 98222025ba6b94e79e50b46a5308d856506c1755
Author: ilya terskov <[email protected]>
AuthorDate: Mon Sep 28 13:38:19 2026 +0700
JAMES-4223 Default S3 If-None-Match to true to prevent silent blob overwrite
---
.../servers/partials/configure/blobstore.adoc | 2 +-
.../sample-configuration/blob.properties | 4 +-
.../sample-configuration/blob.properties | 4 +-
.../blob.properties | 4 +-
.../sample-configuration/blob.properties | 4 +-
.../aws/S3BlobStoreConfiguration.java | 2 +-
.../aws/S3BlobStoreConfigurationTest.java | 15 ++++++
.../aws/S3BlobStoreDAOIfNoneMatchTest.java | 12 +++++
.../S3BlobStoreConfigurationReader.java | 2 +-
.../S3BlobStoreConfigurationReaderTest.java | 59 ++++++++++++++++++++++
10 files changed, 97 insertions(+), 11 deletions(-)
diff --git a/docs/modules/servers/partials/configure/blobstore.adoc
b/docs/modules/servers/partials/configure/blobstore.adoc
index c68aad5979..1ef24ae231 100644
--- a/docs/modules/servers/partials/configure/blobstore.adoc
+++ b/docs/modules/servers/partials/configure/blobstore.adoc
@@ -148,7 +148,7 @@ This property determines the duration (in milliseconds) to
wait between retry at
This delay is known as backoff. The jitter factor is 0.5
| objectstorage.s3.ifNoneMatch.enable
-| optional: Boolean. Defaults to false. When enabled, James adds the
`If-None-Match: *` header to every object upload,
+| optional: Boolean. Defaults to true. When enabled, James adds the
`If-None-Match: *` header to every object upload,
turning writes into conditional writes: the object is only written if it does
not exist yet. A `412 Precondition failed`
answer is then treated as a success as the object is already stored.
diff --git a/server/apps/distributed-app/sample-configuration/blob.properties
b/server/apps/distributed-app/sample-configuration/blob.properties
index 6bb307bbf2..01cdb72352 100644
--- a/server/apps/distributed-app/sample-configuration/blob.properties
+++ b/server/apps/distributed-app/sample-configuration/blob.properties
@@ -120,13 +120,13 @@ objectstorage.s3.secretKey=secretKey1
# from being loaded in memory. This settings complements protocol limits.
# objectstorage.s3.in.read.limit=50M
-# Optional, defaults to false. Turn uploads into conditional writes by adding
the `If-None-Match: *` header:
+# Optional, defaults to true. Turn uploads into conditional writes by adding
the `If-None-Match: *` header:
# an object that is already stored is not written again, a `412 Precondition
failed` answer being treated as a success.
# This spares the bandwidth and the storage cost of re-uploading known blobs
and, combined with a bucket policy denying
# `s3:DeleteObject` and requiring `s3:if-none-match` upon PUT, prevents stored
objects from ever being rewritten.
# Requires an object storage supporting conditional writes (AWS S3, MinIO).
Only brings benefits when
# `deduplication.enable=true`: without deduplication each write uses a
distinct blob id, thus none is ever skipped.
-# objectstorage.s3.ifNoneMatch.enable=false
+# objectstorage.s3.ifNoneMatch.enable=true
# ============================================ Blobs Exporting
==============================================
# Read https://james.apache.org/server/config-blob-export.html for further
details
diff --git
a/server/apps/distributed-pop3-app/sample-configuration/blob.properties
b/server/apps/distributed-pop3-app/sample-configuration/blob.properties
index d9587c5011..0ea36406fb 100644
--- a/server/apps/distributed-pop3-app/sample-configuration/blob.properties
+++ b/server/apps/distributed-pop3-app/sample-configuration/blob.properties
@@ -89,13 +89,13 @@ objectstorage.s3.secretKey=secretKey1
# from being loaded in memory. This settings complements protocol limits.
# objectstorage.s3.in.read.limit=50M
-# Optional, defaults to false. Turn uploads into conditional writes by adding
the `If-None-Match: *` header:
+# Optional, defaults to true. Turn uploads into conditional writes by adding
the `If-None-Match: *` header:
# an object that is already stored is not written again, a `412 Precondition
failed` answer being treated as a success.
# This spares the bandwidth and the storage cost of re-uploading known blobs
and, combined with a bucket policy denying
# `s3:DeleteObject` and requiring `s3:if-none-match` upon PUT, prevents stored
objects from ever being rewritten.
# Requires an object storage supporting conditional writes (AWS S3, MinIO).
Only brings benefits when
# `deduplication.enable=true`: without deduplication each write uses a
distinct blob id, thus none is ever skipped.
-# objectstorage.s3.ifNoneMatch.enable=false
+# objectstorage.s3.ifNoneMatch.enable=true
# ============================================ Blobs Exporting
==============================================
# Read https://james.apache.org/server/config-blob-export.html for further
details
diff --git
a/server/apps/postgres-app/sample-configuration-distributed/blob.properties
b/server/apps/postgres-app/sample-configuration-distributed/blob.properties
index 795b0f8bd2..43b4d7859a 100644
--- a/server/apps/postgres-app/sample-configuration-distributed/blob.properties
+++ b/server/apps/postgres-app/sample-configuration-distributed/blob.properties
@@ -82,13 +82,13 @@ objectstorage.s3.secretKey=secretKey1
# from being loaded in memory. This settings complements protocol limits.
# objectstorage.s3.in.read.limit=50M
-# Optional, defaults to false. Turn uploads into conditional writes by adding
the `If-None-Match: *` header:
+# Optional, defaults to true. Turn uploads into conditional writes by adding
the `If-None-Match: *` header:
# an object that is already stored is not written again, a `412 Precondition
failed` answer being treated as a success.
# This spares the bandwidth and the storage cost of re-uploading known blobs
and, combined with a bucket policy denying
# `s3:DeleteObject` and requiring `s3:if-none-match` upon PUT, prevents stored
objects from ever being rewritten.
# Requires an object storage supporting conditional writes (AWS S3, MinIO).
Only brings benefits when
# `deduplication.enable=true`: without deduplication each write uses a
distinct blob id, thus none is ever skipped.
-# objectstorage.s3.ifNoneMatch.enable=false
+# objectstorage.s3.ifNoneMatch.enable=true
# ============================================ Blobs Exporting
==============================================
# Read https://james.apache.org/server/config-blob-export.html for further
details
diff --git
a/server/apps/scaling-pulsar-smtp/sample-configuration/blob.properties
b/server/apps/scaling-pulsar-smtp/sample-configuration/blob.properties
index 3e6e92c079..41e234b158 100644
--- a/server/apps/scaling-pulsar-smtp/sample-configuration/blob.properties
+++ b/server/apps/scaling-pulsar-smtp/sample-configuration/blob.properties
@@ -101,13 +101,13 @@
objectstorage.s3.secretKey=${env:OBJECTSTORAGE_S3_SECRETKEY}
# from being loaded in memory. This settings complements protocol limits.
# objectstorage.s3.in.read.limit=50M
-# Optional, defaults to false. Turn uploads into conditional writes by adding
the `If-None-Match: *` header:
+# Optional, defaults to true. Turn uploads into conditional writes by adding
the `If-None-Match: *` header:
# an object that is already stored is not written again, a `412 Precondition
failed` answer being treated as a success.
# This spares the bandwidth and the storage cost of re-uploading known blobs
and, combined with a bucket policy denying
# `s3:DeleteObject` and requiring `s3:if-none-match` upon PUT, prevents stored
objects from ever being rewritten.
# Requires an object storage supporting conditional writes (AWS S3, MinIO).
Only brings benefits when
# `deduplication.enable=true`: without deduplication each write uses a
distinct blob id, thus none is ever skipped.
-# objectstorage.s3.ifNoneMatch.enable=false
+# objectstorage.s3.ifNoneMatch.enable=true
# ============================================ Blobs Exporting
==============================================
# Read https://james.apache.org/server/config-blob-export.html for further
details
diff --git
a/server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/S3BlobStoreConfiguration.java
b/server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/S3BlobStoreConfiguration.java
index 0ed068e79a..9d0c74b354 100644
---
a/server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/S3BlobStoreConfiguration.java
+++
b/server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/S3BlobStoreConfiguration.java
@@ -92,7 +92,7 @@ public class S3BlobStoreConfiguration {
this.inMemoryReadLimit = Optional.empty();
this.uploadRetrySpec = Optional.empty();
this.fallbackBucketName = Optional.empty();
- this.ifNoneMatchEnabled = false;
+ this.ifNoneMatchEnabled = true;
}
public ReadyToBuild defaultBucketName(Optional<BucketName>
defaultBucketName) {
diff --git
a/server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/S3BlobStoreConfigurationTest.java
b/server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/S3BlobStoreConfigurationTest.java
index b9b771a029..c8edb80cad 100644
---
a/server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/S3BlobStoreConfigurationTest.java
+++
b/server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/S3BlobStoreConfigurationTest.java
@@ -20,6 +20,7 @@
package org.apache.james.blob.objectstorage.aws;
import static
org.apache.james.blob.objectstorage.aws.S3BlobStoreConfiguration.UPLOAD_RETRY_EXCEPTION_PREDICATE;
+import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import java.net.URI;
@@ -60,4 +61,18 @@ public class S3BlobStoreConfigurationTest {
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("SSEC configuration is mandatory when SSEC is
enabled");
}
+
+ @Test
+ void shouldDefaultIfNoneMatchToTrue() {
+ S3BlobStoreConfiguration configuration =
S3BlobStoreConfiguration.builder()
+ .authConfiguration(AwsS3AuthConfiguration.builder()
+ .endpoint(Throwing.supplier(() -> new
URI("http://localhost:1234";)).get())
+ .accessKeyId("accessKeyId")
+ .secretKey("secretKey1")
+ .build())
+ .region(Region.of("af-south-1"))
+ .build();
+
+ assertThat(configuration.ifNoneMatchEnabled()).isTrue();
+ }
}
\ No newline at end of file
diff --git
a/server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/S3BlobStoreDAOIfNoneMatchTest.java
b/server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/S3BlobStoreDAOIfNoneMatchTest.java
index de791ea0eb..f0aedcf9d0 100644
---
a/server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/S3BlobStoreDAOIfNoneMatchTest.java
+++
b/server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/S3BlobStoreDAOIfNoneMatchTest.java
@@ -106,4 +106,16 @@ class S3BlobStoreDAOIfNoneMatchTest {
verify(client, times(2)).putObject(any(PutObjectRequest.class),
any(AsyncRequestBody.class));
}
+
+ @Test
+ void saveByteSourceShouldRetryConditionalRequestConflict() {
+ when(client.putObject(any(PutObjectRequest.class),
any(AsyncRequestBody.class)))
+
.thenReturn(failedFuture(s3Exception(CONDITIONAL_REQUEST_CONFLICT_STATUS_CODE,
CONDITIONAL_REQUEST_CONFLICT_ERROR_CODE)))
+
.thenReturn(CompletableFuture.completedFuture(PutObjectResponse.builder().build()));
+
+ assertThatCode(() -> Mono.from(testee.save(TEST_BUCKET_NAME,
TEST_BLOB_ID, SHORT_BYTEARRAY.asByteSource())).block())
+ .doesNotThrowAnyException();
+
+ verify(client, times(2)).putObject(any(PutObjectRequest.class),
any(AsyncRequestBody.class));
+ }
}
diff --git
a/server/container/guice/blob/s3/src/main/java/org/apache/james/modules/objectstorage/S3BlobStoreConfigurationReader.java
b/server/container/guice/blob/s3/src/main/java/org/apache/james/modules/objectstorage/S3BlobStoreConfigurationReader.java
index 1299d62115..3d744256ca 100644
---
a/server/container/guice/blob/s3/src/main/java/org/apache/james/modules/objectstorage/S3BlobStoreConfigurationReader.java
+++
b/server/container/guice/blob/s3/src/main/java/org/apache/james/modules/objectstorage/S3BlobStoreConfigurationReader.java
@@ -82,7 +82,7 @@ public class S3BlobStoreConfigurationReader {
Optional<String> fallbackNamespace =
Optional.ofNullable(configuration.getString(OBJECTSTORAGE_NAMESPACE_READ_FALLBACK,
null));
- boolean ifNoneMatchEnabled =
configuration.getBoolean(OBJECTSTORAGE_S3_IF_NONE_MATCH_ENABLE, false);
+ boolean ifNoneMatchEnabled =
configuration.getBoolean(OBJECTSTORAGE_S3_IF_NONE_MATCH_ENABLE, true);
S3BlobStoreConfiguration.Builder.ReadyToBuild configBuilder =
S3BlobStoreConfiguration.builder()
.authConfiguration(AwsS3ConfigurationReader.from(configuration))
diff --git
a/server/container/guice/blob/s3/src/test/java/org/apache/james/modules/objectstorage/S3BlobStoreConfigurationReaderTest.java
b/server/container/guice/blob/s3/src/test/java/org/apache/james/modules/objectstorage/S3BlobStoreConfigurationReaderTest.java
new file mode 100644
index 0000000000..a851b6511c
--- /dev/null
+++
b/server/container/guice/blob/s3/src/test/java/org/apache/james/modules/objectstorage/S3BlobStoreConfigurationReaderTest.java
@@ -0,0 +1,59 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.modules.objectstorage;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.apache.commons.configuration2.Configuration;
+import org.apache.commons.configuration2.PropertiesConfiguration;
+import org.apache.james.blob.objectstorage.aws.S3BlobStoreConfiguration;
+import org.junit.jupiter.api.Test;
+
+class S3BlobStoreConfigurationReaderTest {
+
+ private Configuration baseConfiguration() {
+ Configuration configuration = new PropertiesConfiguration();
+ configuration.addProperty("objectstorage.s3.endPoint",
"http://myEndpoint";);
+ configuration.addProperty("objectstorage.s3.accessKeyId",
"myAccessKeyId");
+ configuration.addProperty("objectstorage.s3.secretKey", "mySecretKey");
+ configuration.addProperty("objectstorage.namespace", "myNamespace");
+ configuration.addProperty("objectstorage.s3.region", "us-east-1");
+ return configuration;
+ }
+
+ @Test
+ void shouldDefaultIfNoneMatchToTrue() throws Exception {
+ Configuration configuration = baseConfiguration();
+
+ S3BlobStoreConfiguration s3Configuration =
S3BlobStoreConfigurationReader.from(configuration);
+
+ assertThat(s3Configuration.ifNoneMatchEnabled()).isTrue();
+ }
+
+ @Test
+ void shouldRespectIfNoneMatchExplicitlyDisabled() throws Exception {
+ Configuration configuration = baseConfiguration();
+ configuration.addProperty("objectstorage.s3.ifNoneMatch.enable",
false);
+
+ S3BlobStoreConfiguration s3Configuration =
S3BlobStoreConfigurationReader.from(configuration);
+
+ assertThat(s3Configuration.ifNoneMatchEnabled()).isFalse();
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
