Skip to content

Commit cad71cc

Browse files
DaveCTurnerandreidan
authored andcommitted
Fix CommonPrefixes rendering in S3HttpHandler (elastic#126147)
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.
1 parent cd99c70 commit cad71cc

File tree

2 files changed

+21
-6
lines changed

2 files changed

+21
-6
lines changed

test/fixtures/s3-fixture/src/main/java/fixture/s3/S3HttpHandler.java

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -241,12 +241,9 @@ public void handle(final HttpExchange exchange) throws IOException {
241241
list.append("<Size>").append(blob.getValue().length()).append("</Size>");
242242
list.append("</Contents>");
243243
}
244-
if (commonPrefixes.isEmpty() == false) {
245-
list.append("<CommonPrefixes>");
246-
commonPrefixes.forEach(commonPrefix -> list.append("<Prefix>").append(commonPrefix).append("</Prefix>"));
247-
list.append("</CommonPrefixes>");
248-
249-
}
244+
commonPrefixes.forEach(
245+
commonPrefix -> list.append("<CommonPrefixes><Prefix>").append(commonPrefix).append("</Prefix></CommonPrefixes>")
246+
);
250247
list.append("</ListBucketResult>");
251248

252249
byte[] response = list.toString().getBytes(StandardCharsets.UTF_8);

test/fixtures/s3-fixture/src/test/java/fixture/s3/S3HttpHandlerTests.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,27 @@ public void testSimpleObjectOperations() {
7676
handleRequest(handler, "GET", "/bucket/?prefix=path/other")
7777
);
7878

79+
assertEquals(RestStatus.OK, handleRequest(handler, "PUT", "/bucket/path/subpath1/blob", randomAlphaOfLength(50)).status());
80+
assertEquals(RestStatus.OK, handleRequest(handler, "PUT", "/bucket/path/subpath2/blob", randomAlphaOfLength(50)).status());
81+
assertEquals(new TestHttpResponse(RestStatus.OK, """
82+
<?xml version="1.0" encoding="UTF-8"?><ListBucketResult><Prefix>path/</Prefix>\
83+
<Delimiter>/</Delimiter><Contents><Key>path/blob</Key><Size>50</Size></Contents>\
84+
<CommonPrefixes><Prefix>path/subpath1/</Prefix></CommonPrefixes>\
85+
<CommonPrefixes><Prefix>path/subpath2/</Prefix></CommonPrefixes>\
86+
</ListBucketResult>"""), handleRequest(handler, "GET", "/bucket/?delimiter=/&prefix=path/"));
87+
7988
assertEquals(RestStatus.OK, handleRequest(handler, "DELETE", "/bucket/path/blob").status());
8089
assertEquals(RestStatus.NO_CONTENT, handleRequest(handler, "DELETE", "/bucket/path/blob").status());
8190

91+
assertEquals(new TestHttpResponse(RestStatus.OK, """
92+
<?xml version="1.0" encoding="UTF-8"?><ListBucketResult><Prefix></Prefix>\
93+
<Contents><Key>path/subpath1/blob</Key><Size>50</Size></Contents>\
94+
<Contents><Key>path/subpath2/blob</Key><Size>50</Size></Contents>\
95+
</ListBucketResult>"""), handleRequest(handler, "GET", "/bucket/?prefix="));
96+
97+
assertEquals(RestStatus.OK, handleRequest(handler, "DELETE", "/bucket/path/subpath1/blob").status());
98+
assertEquals(RestStatus.OK, handleRequest(handler, "DELETE", "/bucket/path/subpath2/blob").status());
99+
82100
assertEquals(
83101
new TestHttpResponse(RestStatus.OK, """
84102
<?xml version="1.0" encoding="UTF-8"?><ListBucketResult><Prefix></Prefix></ListBucketResult>"""),

0 commit comments

Comments
 (0)