Skip to content

Commit 5745440

Browse files
afrankenafranken
committed
Verify object tagging, fixes
Includes minor fixes.
1 parent 339bc49 commit 5745440

File tree

6 files changed

+166
-11
lines changed

6 files changed

+166
-11
lines changed

‎server/src/main/java/com/adobe/testing/s3mock/ObjectController.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -554,6 +554,7 @@ public ResponseEntity<Void> putObjectTagging(
554554
var bucket = bucketService.verifyBucketExists(bucketName);
555555

556556
var s3ObjectMetadata = objectService.verifyObjectExists(bucketName, key.key(), versionId);
557+
objectService.verifyObjectTags(body.tagSet().tags());
557558
objectService.setObjectTags(bucketName, key.key(), versionId, body.tagSet().tags());
558559
return ResponseEntity
559560
.ok()
@@ -673,8 +674,7 @@ public ResponseEntity<Retention> getObjectRetention(
673674
@RequestParam(value = VERSION_ID, required = false) @Nullable String versionId) {
674675
var bucket = bucketService.verifyBucketExists(bucketName);
675676
bucketService.verifyBucketObjectLockEnabled(bucketName);
676-
var s3ObjectMetadata = objectService.verifyObjectLockConfiguration(bucketName, key.key(),
677-
versionId);
677+
var s3ObjectMetadata = objectService.verifyObjectLockConfiguration(bucketName, key.key(), versionId);
678678

679679
return ResponseEntity
680680
.ok()

‎server/src/main/java/com/adobe/testing/s3mock/S3Exception.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,11 @@ public class S3Exception extends RuntimeException {
4343
"The list of parts was not in ascending order. The parts list must be specified in "
4444
+ "order by part number.");
4545

46+
public static final S3Exception INVALID_TAG =
47+
new S3Exception(BAD_REQUEST.value(), "InvalidTag",
48+
"Your request contains tag input that is not valid. For example, your request might contain "
49+
+ "duplicate keys, keys or values that are too long, or system tags.");
50+
4651
public static S3Exception completeRequestMissingChecksum(String algorithm, Integer partNumber) {
4752
return new S3Exception(BAD_REQUEST.value(), BAD_REQUEST_CODE,
4853
"The upload was created using a " + algorithm + " checksum. "

‎server/src/main/java/com/adobe/testing/s3mock/service/ObjectService.java‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static com.adobe.testing.s3mock.S3Exception.BAD_REQUEST_CONTENT;
2020
import static com.adobe.testing.s3mock.S3Exception.BAD_REQUEST_MD5;
2121
import static com.adobe.testing.s3mock.S3Exception.INVALID_REQUEST_RETAIN_DATE;
22+
import static com.adobe.testing.s3mock.S3Exception.INVALID_TAG;
2223
import static com.adobe.testing.s3mock.S3Exception.NOT_FOUND_OBJECT_LOCK;
2324
import static com.adobe.testing.s3mock.S3Exception.NOT_MODIFIED;
2425
import static com.adobe.testing.s3mock.S3Exception.NO_SUCH_KEY;
@@ -49,8 +50,10 @@
4950
import java.nio.file.Path;
5051
import java.time.Instant;
5152
import java.util.ArrayList;
53+
import java.util.HashSet;
5254
import java.util.List;
5355
import java.util.Map;
56+
import java.util.regex.Pattern;
5457
import org.jspecify.annotations.Nullable;
5558
import org.slf4j.Logger;
5659
import org.slf4j.LoggerFactory;
@@ -59,6 +62,14 @@ public class ObjectService extends ServiceBase {
5962
static final String WILDCARD_ETAG = "\"*\"";
6063
static final String WILDCARD = "*";
6164
private static final Logger LOG = LoggerFactory.getLogger(ObjectService.class);
65+
private static final Pattern TAG_ALLOWED_CHARS = Pattern.compile("[\\w+ \\-=.:/@]*");
66+
private static final int MAX_ALLOWED_TAGS = 50;
67+
private static final int MIN_ALLOWED_TAG_KEY_LENGTH = 1;
68+
private static final int MAX_ALLOWED_TAG_KEY_LENGTH = 128;
69+
private static final int MIN_ALLOWED_TAG_VALUE_LENGTH = 0;
70+
private static final int MAX_ALLOWED_TAG_VALUE_LENGTH = 256;
71+
private static final String DISALLOWED_TAG_KEY_PREFIX = "aws:";
72+
6273
private final BucketStore bucketStore;
6374
private final ObjectStore objectStore;
6475

@@ -175,6 +186,48 @@ public void setObjectTags(String bucketName, String key, @Nullable String versio
175186
objectStore.storeObjectTags(bucketMetadata, uuid, versionId, tags);
176187
}
177188

189+
public void verifyObjectTags(List<Tag> tags) {
190+
if (tags.size() > MAX_ALLOWED_TAGS) {
191+
throw INVALID_TAG;
192+
}
193+
verifyDuplicateTagKeys(tags);
194+
for (var tag : tags) {
195+
verifyTagKeyPrefix(tag.key());
196+
verifyTagLength(MIN_ALLOWED_TAG_KEY_LENGTH, MAX_ALLOWED_TAG_KEY_LENGTH, tag.key());
197+
verifyTagChars(tag.key());
198+
199+
verifyTagLength(MIN_ALLOWED_TAG_VALUE_LENGTH, MAX_ALLOWED_TAG_VALUE_LENGTH, tag.value());
200+
verifyTagChars(tag.value());
201+
}
202+
}
203+
204+
private void verifyDuplicateTagKeys(List<Tag> tags) {
205+
var tagKeys = new HashSet<String>();
206+
for (var tag : tags) {
207+
if (!tagKeys.add(tag.key())) {
208+
throw INVALID_TAG;
209+
}
210+
}
211+
}
212+
213+
private void verifyTagKeyPrefix(String tagKey) {
214+
if (tagKey.startsWith(DISALLOWED_TAG_KEY_PREFIX)) {
215+
throw INVALID_TAG;
216+
}
217+
}
218+
219+
private void verifyTagLength(int minLength, int maxLength, String tag) {
220+
if (tag.length() < minLength || tag.length() > maxLength) {
221+
throw INVALID_TAG;
222+
}
223+
}
224+
225+
private void verifyTagChars(String tag) {
226+
if (!TAG_ALLOWED_CHARS.matcher(tag).matches()) {
227+
throw INVALID_TAG;
228+
}
229+
}
230+
178231
public void setLegalHold(String bucketName, String key, @Nullable String versionId, LegalHold legalHold) {
179232
var bucketMetadata = bucketStore.getBucketMetadata(bucketName);
180233
var uuid = bucketMetadata.getID(key);

‎server/src/main/java/com/adobe/testing/s3mock/store/ObjectStore.java‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -100,11 +100,11 @@ public S3ObjectMetadata storeS3ObjectMetadata(
100100
var existingVersions = getS3ObjectVersions(bucket, id);
101101
if (existingVersions != null) {
102102
versionId = existingVersions.createVersion();
103-
writeVersionsfile(bucket, id, existingVersions);
103+
writeVersionsFile(bucket, id, existingVersions);
104104
} else {
105105
var newVersions = createS3ObjectVersions(bucket, id);
106106
versionId = newVersions.createVersion();
107-
writeVersionsfile(bucket, id, newVersions);
107+
writeVersionsFile(bucket, id, newVersions);
108108
}
109109
}
110110
var dataFile = inputPathToFile(path, getDataFilePath(bucket, id, versionId));
@@ -279,7 +279,9 @@ public void storeRetention(BucketMetadata bucket, UUID id, @Nullable String vers
279279
public S3ObjectMetadata getS3ObjectMetadata(BucketMetadata bucket, UUID id, @Nullable String versionId) {
280280
if (bucket.isVersioningEnabled() && versionId == null) {
281281
var s3ObjectVersions = getS3ObjectVersions(bucket, id);
282-
versionId = s3ObjectVersions.getLatestVersion();
282+
if (s3ObjectVersions != null) {
283+
versionId = s3ObjectVersions.getLatestVersion();
284+
}
283285
}
284286
var metaPath = getMetaFilePath(bucket, id, versionId);
285287

@@ -321,7 +323,7 @@ public S3ObjectVersions createS3ObjectVersions(BucketMetadata bucket, UUID id) {
321323
} else {
322324
synchronized (lockStore.get(id)) {
323325
try {
324-
writeVersionsfile(bucket, id, new S3ObjectVersions(id));
326+
writeVersionsFile(bucket, id, new S3ObjectVersions(id));
325327
return objectMapper.readValue(metaPath.toFile(), S3ObjectVersions.class);
326328
} catch (java.io.IOException e) {
327329
throw new IllegalArgumentException("Could not read object versions-file " + id, e);
@@ -458,22 +460,31 @@ public boolean deleteObject(
458460
}
459461
}
460462

463+
/**
464+
* Deletes a specific version of an object, if found.
465+
* If this is the last version of an object, it deletes the object.
466+
* Returns true if the *LAST* version was deleted.
467+
*/
461468
private boolean doDeleteVersion(BucketMetadata bucket, UUID id, String versionId) {
462469
synchronized (lockStore.get(id)) {
463470
try {
464471
var existingVersions = getS3ObjectVersions(bucket, id);
472+
if (existingVersions == null) {
473+
//no versions exist, nothing to delete.
474+
return false;
475+
}
465476
if (existingVersions.versions().size() <= 1) {
466477
//this is the last version of an object, delete object completely.
467478
return doDeleteObject(bucket, id);
468479
} else {
469480
//there is at least one version of an object left, delete only the version.
470481
existingVersions.deleteVersion(versionId);
471-
writeVersionsfile(bucket, id, existingVersions);
482+
writeVersionsFile(bucket, id, existingVersions);
483+
return false;
472484
}
473485
} catch (Exception e) {
474486
throw new IllegalStateException("Could not delete object-version " + id, e);
475487
}
476-
return false;
477488
}
478489
}
479490

@@ -502,7 +513,7 @@ private boolean insertDeleteMarker(
502513
var existingVersions = getS3ObjectVersions(bucket, id);
503514
if (existingVersions != null) {
504515
versionId = existingVersions.createVersion();
505-
writeVersionsfile(bucket, id, existingVersions);
516+
writeVersionsFile(bucket, id, existingVersions);
506517
}
507518
writeMetafile(bucket, S3ObjectMetadata.deleteMarker(s3ObjectMetadata, versionId));
508519
} catch (Exception e) {
@@ -575,7 +586,7 @@ private Path getVersionFilePath(BucketMetadata bucket, UUID id) {
575586
return getObjectFolderPath(bucket, id).resolve(VERSIONS_FILE);
576587
}
577588

578-
private void writeVersionsfile(BucketMetadata bucket, UUID id,
589+
private void writeVersionsFile(BucketMetadata bucket, UUID id,
579590
S3ObjectVersions s3ObjectVersions) {
580591
try {
581592
synchronized (lockStore.get(id)) {

‎server/src/main/java/com/adobe/testing/s3mock/store/S3ObjectMetadata.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ public record S3ObjectMetadata(
7676
checksumType = checksumType == null ? ChecksumType.FULL_OBJECT : checksumType;
7777
}
7878

79-
public static S3ObjectMetadata deleteMarker(S3ObjectMetadata metadata, String versionId) {
79+
public static S3ObjectMetadata deleteMarker(S3ObjectMetadata metadata, @Nullable String versionId) {
8080
return new S3ObjectMetadata(metadata.id,
8181
metadata.key(),
8282
metadata.size(),

‎server/src/test/kotlin/com/adobe/testing/s3mock/service/ObjectServiceTest.kt‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,13 @@ package com.adobe.testing.s3mock.service
1717

1818
import com.adobe.testing.s3mock.ChecksumTestUtil
1919
import com.adobe.testing.s3mock.S3Exception
20+
import com.adobe.testing.s3mock.S3Exception.INVALID_TAG
2021
import com.adobe.testing.s3mock.dto.ChecksumAlgorithm
2122
import com.adobe.testing.s3mock.dto.Delete
2223
import com.adobe.testing.s3mock.dto.Mode
2324
import com.adobe.testing.s3mock.dto.Retention
2425
import com.adobe.testing.s3mock.dto.S3ObjectIdentifier
26+
import com.adobe.testing.s3mock.dto.Tag
2527
import com.adobe.testing.s3mock.store.BucketMetadata
2628
import com.adobe.testing.s3mock.store.MultipartStore
2729
import com.adobe.testing.s3mock.util.AwsHttpHeaders
@@ -308,6 +310,90 @@ internal class ObjectServiceTest : ServiceTestBase() {
308310
assertThat(tempFileAndChecksum.right).contains("Y8S4/uAGut7vjdFZQjLKZ7P28V9EPWb4BIoeniuM0mY=")
309311
}
310312

313+
@Test
314+
fun `store tags succeeds`() {
315+
val tags = listOf(Tag("key1", "value1"), Tag("key2", "value2"))
316+
iut.verifyObjectTags(tags)
317+
}
318+
319+
@Test
320+
fun `store tags succeeds with min key and value length`() {
321+
val tags = listOf(Tag("1", ""), Tag("2", ""))
322+
iut.verifyObjectTags(tags)
323+
}
324+
325+
@Test
326+
fun `store tags succeeds with all allowed characters`() {
327+
val tags = listOf(Tag("key1+-=._:/@ ", "value1"), Tag("key2", "value2"))
328+
iut.verifyObjectTags(tags)
329+
}
330+
331+
@Test
332+
fun `store tags fails with too many tags`() {
333+
val tags = mutableListOf<Tag>()
334+
for (i in 0..60) {
335+
tags.add(Tag("key$i", "value$i"))
336+
}
337+
assertThatThrownBy {
338+
iut.verifyObjectTags(tags)
339+
}.isInstanceOf(S3Exception::class.java)
340+
.hasMessage(INVALID_TAG.message)
341+
}
342+
343+
@Test
344+
fun `store tags fails with duplicate keys`() {
345+
val tags = listOf(Tag("key1", "value1"), Tag("key1", "value2"))
346+
assertThatThrownBy {
347+
iut.verifyObjectTags(tags)
348+
}.isInstanceOf(S3Exception::class.java)
349+
.hasMessage(INVALID_TAG.message)
350+
}
351+
352+
@Test
353+
fun `store tags fails with illegal characters`() {
354+
val tags = listOf(Tag("key1%()", "value1"))
355+
assertThatThrownBy {
356+
iut.verifyObjectTags(tags)
357+
}.isInstanceOf(S3Exception::class.java)
358+
.hasMessage(INVALID_TAG.message)
359+
}
360+
361+
@Test
362+
fun `store tags fails with key gt 127 characters`() {
363+
val tags = listOf(Tag("Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Aenean commodo ligula eget dolor. Aenean massa. Cum sociis natoque pena", "value1"))
364+
assertThatThrownBy {
365+
iut.verifyObjectTags(tags)
366+
}.isInstanceOf(S3Exception::class.java)
367+
.hasMessage(INVALID_TAG.message)
368+
}
369+
370+
@Test
371+
fun `store tags fails with value gt 255 characters`() {
372+
val tags = listOf(Tag("key1", "Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Aenean commodo ligula eget dolor. Aenean massa. Cum sociis natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Donec quam felis, ultricies nec, pellentesque eu, pretium quis, s"))
373+
assertThatThrownBy {
374+
iut.verifyObjectTags(tags)
375+
}.isInstanceOf(S3Exception::class.java)
376+
.hasMessage(INVALID_TAG.message)
377+
}
378+
379+
@Test
380+
fun `store tags fails with invalid key prefix`() {
381+
val tags = listOf(Tag("aws:key1", "value1"))
382+
assertThatThrownBy {
383+
iut.verifyObjectTags(tags)
384+
}.isInstanceOf(S3Exception::class.java)
385+
.hasMessage(INVALID_TAG.message)
386+
}
387+
388+
@Test
389+
fun `store tags fails with invalid key length`() {
390+
val tags = listOf(Tag("", "value1"))
391+
assertThatThrownBy {
392+
iut.verifyObjectTags(tags)
393+
}.isInstanceOf(S3Exception::class.java)
394+
.hasMessage(INVALID_TAG.message)
395+
}
396+
311397
@Throws(IOException::class)
312398
private fun toTempFile(path: Path, algorithm: software.amazon.awssdk.checksums.spi.ChecksumAlgorithm): Path {
313399
val (inputStream, _) = ChecksumTestUtil.prepareInputStream(path.toFile(), false, algorithm)

0 commit comments

Comments
 (0)