From 43aec90704b45f481756dcdec33094360b97448f Mon Sep 17 00:00:00 2001
From: David Turner <david.turner@elastic.co>
Date: Wed, 2 Apr 2025 16:48:26 +0100
Subject: [PATCH] Fix `CommonPrefixes` rendering in `S3HttpHandler`

Today the `ListObjects` implementation in `S3HttpHandler` will put all
the common prefixes in a single `<CommonPrefixes>` container, but in
fact the real S3 gives each one its own container. The v1 SDK is lenient
and accepts either, but the v2 SDK requires us to do this correctly.
This commit fixes the test fixture to match the behaviour of the real
S3.
---
 .../main/java/fixture/s3/S3HttpHandler.java    |  9 +++------
 .../java/fixture/s3/S3HttpHandlerTests.java    | 18 ++++++++++++++++++
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/test/fixtures/s3-fixture/src/main/java/fixture/s3/S3HttpHandler.java b/test/fixtures/s3-fixture/src/main/java/fixture/s3/S3HttpHandler.java
index 444c4627ffa8a..91bb1597fccd4 100644
--- a/test/fixtures/s3-fixture/src/main/java/fixture/s3/S3HttpHandler.java
+++ b/test/fixtures/s3-fixture/src/main/java/fixture/s3/S3HttpHandler.java
@@ -241,12 +241,9 @@ public void handle(final HttpExchange exchange) throws IOException {
                     list.append("<Size>").append(blob.getValue().length()).append("</Size>");
                     list.append("</Contents>");
                 }
-                if (commonPrefixes.isEmpty() == false) {
-                    list.append("<CommonPrefixes>");
-                    commonPrefixes.forEach(commonPrefix -> list.append("<Prefix>").append(commonPrefix).append("</Prefix>"));
-                    list.append("</CommonPrefixes>");
-
-                }
+                commonPrefixes.forEach(
+                    commonPrefix -> list.append("<CommonPrefixes><Prefix>").append(commonPrefix).append("</Prefix></CommonPrefixes>")
+                );
                 list.append("</ListBucketResult>");
 
                 byte[] response = list.toString().getBytes(StandardCharsets.UTF_8);
diff --git a/test/fixtures/s3-fixture/src/test/java/fixture/s3/S3HttpHandlerTests.java b/test/fixtures/s3-fixture/src/test/java/fixture/s3/S3HttpHandlerTests.java
index 58f32292fa91c..e26ee9c0da589 100644
--- a/test/fixtures/s3-fixture/src/test/java/fixture/s3/S3HttpHandlerTests.java
+++ b/test/fixtures/s3-fixture/src/test/java/fixture/s3/S3HttpHandlerTests.java
@@ -76,9 +76,27 @@ public void testSimpleObjectOperations() {
             handleRequest(handler, "GET", "/bucket/?prefix=path/other")
         );
 
+        assertEquals(RestStatus.OK, handleRequest(handler, "PUT", "/bucket/path/subpath1/blob", randomAlphaOfLength(50)).status());
+        assertEquals(RestStatus.OK, handleRequest(handler, "PUT", "/bucket/path/subpath2/blob", randomAlphaOfLength(50)).status());
+        assertEquals(new TestHttpResponse(RestStatus.OK, """
+            <?xml version="1.0" encoding="UTF-8"?><ListBucketResult><Prefix>path/</Prefix>\
+            <Delimiter>/</Delimiter><Contents><Key>path/blob</Key><Size>50</Size></Contents>\
+            <CommonPrefixes><Prefix>path/subpath1/</Prefix></CommonPrefixes>\
+            <CommonPrefixes><Prefix>path/subpath2/</Prefix></CommonPrefixes>\
+            </ListBucketResult>"""), handleRequest(handler, "GET", "/bucket/?delimiter=/&prefix=path/"));
+
         assertEquals(RestStatus.OK, handleRequest(handler, "DELETE", "/bucket/path/blob").status());
         assertEquals(RestStatus.NO_CONTENT, handleRequest(handler, "DELETE", "/bucket/path/blob").status());
 
+        assertEquals(new TestHttpResponse(RestStatus.OK, """
+            <?xml version="1.0" encoding="UTF-8"?><ListBucketResult><Prefix></Prefix>\
+            <Contents><Key>path/subpath1/blob</Key><Size>50</Size></Contents>\
+            <Contents><Key>path/subpath2/blob</Key><Size>50</Size></Contents>\
+            </ListBucketResult>"""), handleRequest(handler, "GET", "/bucket/?prefix="));
+
+        assertEquals(RestStatus.OK, handleRequest(handler, "DELETE", "/bucket/path/subpath1/blob").status());
+        assertEquals(RestStatus.OK, handleRequest(handler, "DELETE", "/bucket/path/subpath2/blob").status());
+
         assertEquals(
             new TestHttpResponse(RestStatus.OK, """
                 <?xml version="1.0" encoding="UTF-8"?><ListBucketResult><Prefix></Prefix></ListBucketResult>"""),