Skip to content

HDDS-15600. Fix ListObjects response for encoding-type, empty delimiter, and control-character prefix #10586

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Gargi-jais11 wants to merge 4 commits into
base: master
Choose a base branch
from
Open

HDDS-15600. Fix ListObjects response for encoding-type, empty delimiter, and control-character prefix #10586

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cf2f8b2
Fix ListObjects response for encoding-type, empty delimiter, and cont…
Gargi-jais11 Jun 19, 2026
45fdef1
Merge branch 'master' of github.com:apache/ozone into HDDS-15600
Gargi-jais11 Jun 29, 2026
1f720b5
fix checkstyle
Gargi-jais11 Jun 29, 2026
2852242
address review comments
Gargi-jais11 Jun 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ...p-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1095,11 +1095,11 @@ public ListKeysResult listKeys(String volumeName, String bucketName,
} else {
// This allows us to seek directly to the first key with the right prefix.
seekKey = getOzoneKey(volumeName, bucketName,
StringUtils.isNotBlank(keyPrefix) ? keyPrefix : OM_KEY_PREFIX);
(keyPrefix != null && !keyPrefix.isEmpty()) ? keyPrefix : OM_KEY_PREFIX);
Comment thread
Gargi-jais11 marked this conversation as resolved.
Outdated
}

String seekPrefix;
if (StringUtils.isNotBlank(keyPrefix)) {
if (keyPrefix != null && !keyPrefix.isEmpty()) {
Comment thread
Gargi-jais11 marked this conversation as resolved.
Outdated
seekPrefix = getOzoneKey(volumeName, bucketName, keyPrefix);
} else {
seekPrefix = getBucketKey(volumeName, bucketName) + OM_KEY_PREFIX;
Expand Down
3 changes: 2 additions & 1 deletion .../s3gateway/src/main/java/org/apache/hadoop/ozone/s3/commontypes/ObjectKeyNameAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ public EncodingTypeObject unmarshal(String s) {
public String marshal(EncodingTypeObject s)
throws UnsupportedEncodingException {
if (s.getEncodingType() != null && s.getEncodingType().equals("url")) {
return StringUtils.replaceEach(S3Utils.urlEncode(s.getName()), URL_ENCODE_SEARCH_CHARS, URL_ENCODE_REPLACE_CHARS);
return StringUtils.replaceEach(S3Utils.s3urlEncode(s.getName()),
URL_ENCODE_SEARCH_CHARS, URL_ENCODE_REPLACE_CHARS);
}
return s.getName();
}
Expand Down
11 changes: 11 additions & 0 deletions ...one/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/commontypes/RequestParameters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ default int getInt(String key, int defaultValue) {
}
}

/**
* @return true if the query parameter is present, even when its value is
* an empty string (eg. {@code delimiter=}).
*/
boolean containsKey(String key);

/** Additional methods for tests. */
interface Mutable extends RequestParameters {

Expand All @@ -72,6 +78,11 @@ public String get(String key) {
return params.getFirst(key);
}

@Override
public boolean containsKey(String key) {
return params.containsKey(key);
}

@Override
public void set(String key, String value) {
params.putSingle(key, value);
Expand Down
20 changes: 13 additions & 7 deletions hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,13 @@ public Response get(
@Override
Response handleGetRequest(S3RequestContext context, String bucketName) throws IOException, OS3Exception {
final String continueToken = queryParams().get(QueryParams.CONTINUATION_TOKEN);
final String delimiter = queryParams().get(QueryParams.DELIMITER);
final String delimiter = queryParams().containsKey(QueryParams.DELIMITER) ?
queryParams().get(QueryParams.DELIMITER) : null;
final String encodingType = queryParams().get(QueryParams.ENCODING_TYPE);
final String marker = queryParams().get(QueryParams.MARKER);
int maxKeys = queryParams().getInt(QueryParams.MAX_KEYS, 1000);
String prefix = queryParams().get(QueryParams.PREFIX, "");
final boolean prefixSpecified = queryParams().containsKey(QueryParams.PREFIX);
final String prefix = prefixSpecified ? queryParams().get(QueryParams.PREFIX) : "";
String startAfter = queryParams().get(QueryParams.START_AFTER);

Iterator<? extends OzoneKey> ozoneKeyIterator = null;
Expand Down Expand Up @@ -157,17 +159,21 @@ Response handleGetRequest(S3RequestContext context, String bucketName) throws IO
if (encodingType != null && !encodingType.equals(ENCODING_TYPE)) {
throw S3ErrorTable.newError(S3ErrorTable.INVALID_ARGUMENT, encodingType);
}

// If you specify the encoding-type request parameter,should return
// encoded key name values in the following response elements:
// Delimiter, Prefix, Key, and StartAfter.
// encoded key name values in the following response elements: Delimiter,
// Key, and StartAfter. The echoed Prefix request parameter is returned without URL encoding.
//
// For detail refer:
// https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListObjectsV2.html#AmazonS3-ListObjectsV2-response-EncodingType
ListObjectResponse response = new ListObjectResponse();
response.setDelimiter(EncodingTypeObject.createNullable(delimiter, encodingType));
// AWS omits Delimiter from the response when the client passes delimiter= or does not specify delimiter at all.
if (delimiter != null && !delimiter.isEmpty()) {
Comment thread
Gargi-jais11 marked this conversation as resolved.
Outdated
response.setDelimiter(EncodingTypeObject.createNullable(delimiter, encodingType));
}
response.setName(bucketName);
response.setPrefix(EncodingTypeObject.createNullable(prefix, encodingType));
if (prefixSpecified) {
response.setPrefix(EncodingTypeObject.createNullable(prefix, null));
}
Comment on lines +174 to +176

@Russole Russole Jun 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefix should also be encoded when encoding-type=url is specified.

AWS docs say EncodingType applies to Delimiter, Prefix, Key, and StartAfter:
https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListObjectsV2.html#AmazonS3-ListObjectsV2-response-EncodingType

Local test:

aws s3api list-objects \
  --bucket buck1 \
  --prefix $'\n' \
  --encoding-type url \
  --endpoint-url http://localhost:9878/

Actual:

"Prefix": "\n"

Expected:

"Prefix": "%0A"

Looks like Prefix is created with a null encoding type here. Should this use encodingType instead?

@Gargi-jais11 Gargi-jais11 Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Russole thanks for bringing this in view. Will revert the changes for this but now the problem is test_bucket_list_prefix_unreadable this test since the encoding type is not there so it should not encode it.

With original :

if (prefixSpecified) {
      response.setPrefix(EncodingTypeObject.createNullable(prefix, encodingType));
    }

Local test:

bash-5.1$ aws s3api list-objects \
  --bucket buck1 \
  --prefix $'\n' \
  --endpoint-url http://s3g:9878/

Actual is:

{
    "RequestCharged": null,
    "Prefix": "%0A"            --------------------> it is encoded without encoding type mentioned in the request
}

Expected :

{
    "RequestCharged": null,
    "Prefix": "\n"
}

@Gargi-jais11 Gargi-jais11 Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder how to fix this part when encoding-type is not mentioned in the request.

response.setMarker(marker == null ? "" : marker);
response.setMaxKeys(maxKeys);
response.setEncodingType(encodingType);
Expand Down
11 changes: 11 additions & 0 deletions hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/util/S3Utils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,17 @@ public static String urlEncode(String str)
return URLEncoder.encode(str, UTF_8.name());
}

/**
* Percent-encode a string for S3 {@code encoding-type=url} responses.
*
* <p>Unlike {@link URLEncoder} (application/x-www-form-urlencoded), AWS S3
* uses percent-encoding where spaces are {@code %20}, not {@code +}.
*/
public static String s3urlEncode(String str)
throws UnsupportedEncodingException {
return urlEncode(str).replace("+", "%20");
}
Comment thread
adoroszlai marked this conversation as resolved.

private S3Utils() {
// no instances
}
Expand Down
2 changes: 1 addition & 1 deletion ...ateway/src/test/java/org/apache/hadoop/ozone/s3/commontypes/TestObjectKeyNameAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public class TestObjectKeyNameAdapter {
public void testEncodeResult() throws Exception {
assertEquals("abc/", getAdapter()
.marshal(EncodingTypeObject.createNullable("abc/", ENCODING_TYPE)));
assertEquals("a+b+c/", getAdapter()
assertEquals("a%20b%20c/", getAdapter()
.marshal(EncodingTypeObject.createNullable("a b c/", ENCODING_TYPE)));
assertEquals("a%2Bb%2Bc/", getAdapter()
.marshal(EncodingTypeObject.createNullable("a+b+c/", ENCODING_TYPE)));
Expand Down
62 changes: 57 additions & 5 deletions hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketList.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.apache.hadoop.ozone.client.OzoneClient;
import org.apache.hadoop.ozone.client.OzoneClientStub;
import org.apache.hadoop.ozone.s3.commontypes.EncodingTypeObject;
import org.apache.hadoop.ozone.s3.commontypes.ObjectKeyNameAdapter;
import org.apache.hadoop.ozone.s3.exception.OS3Exception;
import org.apache.hadoop.ozone.s3.exception.S3ErrorTable;
import org.apache.hadoop.ozone.s3.util.S3Consts.QueryParams;
Expand Down Expand Up @@ -214,6 +215,7 @@ public void listWithPrefixAndEmptyStrDelimiter()

assertEquals(0, getBucketResponse.getCommonPrefixes().size());
assertEquals(4, getBucketResponse.getContents().size());
assertNull(getBucketResponse.getDelimiter());
assertEquals("dir1/",
getBucketResponse.getContents().get(0).getKey().getName());
assertEquals("dir1/dir2/",
Expand Down Expand Up @@ -401,7 +403,7 @@ public void testEncodingType() throws IOException, OS3Exception {
<?xml version="1.0" encoding="UTF-8"?>
<ListBucketResult xmlns="http://s3.amazonaws.com/doc/2006-03-01/">
...
<Prefix>data%3D</Prefix>
<Prefix>data=</Prefix>
<StartAfter>data%3D</StartAfter>
<Delimiter>%3D</Delimiter>
<EncodingType>url</EncodingType>
Expand All @@ -415,7 +417,8 @@ public void testEncodingType() throws IOException, OS3Exception {
</CommonPrefixes>
</ListBucketResult>

if encodingType == null , the = will not be encoded to "%3D"
Echoed Prefix is not URL-encoded. Delimiter, StartAfter, Key, and
CommonPrefixes are encoded when encodingType == url.
* */

OzoneClient ozoneClient =
Expand All @@ -437,9 +440,8 @@ public void testEncodingType() throws IOException, OS3Exception {
// The Object name will be encoded by ObjectKeyNameAdapter
// if encodingType == url
assertEncodingTypeObject(delimiter, encodingType, response.getDelimiter());
assertEncodingTypeObject(prefix, encodingType, response.getPrefix());
assertEncodingTypeObject(startAfter, encodingType,
response.getStartAfter());
assertEncodingTypeObject(prefix, null, response.getPrefix());
assertEncodingTypeObject(startAfter, encodingType, response.getStartAfter());
assertNotNull(response.getCommonPrefixes());
assertNotNull(response.getContents());
assertEncodingTypeObject(prefix + delimiter, encodingType,
Expand Down Expand Up @@ -566,6 +568,56 @@ public void testListObjectsRespectsConfiguredMaxKeysLimit() throws Exception {
assertEquals(Integer.parseInt(configuredMaxKeysLimit), response.getContents().size());
}

@Test
public void testListObjectsUrlEncodingUsesPercentTwentyForSpaces()
throws Exception {
OzoneClient client = createClientWithKeys(
"foo+1/bar", "foo/bar/xyzzy", "quux ab/thud", "asdf+b");
BucketEndpoint endpoint = newBucketEndpointBuilder().setClient(client).build();

endpoint.queryParamsForTest().set(QueryParams.DELIMITER, "/");
endpoint.queryParamsForTest().set(QueryParams.ENCODING_TYPE, ENCODING_TYPE);
ListObjectResponse response = (ListObjectResponse) endpoint.get("b1").getEntity();

ObjectKeyNameAdapter adapter = new ObjectKeyNameAdapter();
assertEquals("asdf%2Bb", adapter.marshal(response.getContents().get(0).getKey()));
assertEquals(3, response.getCommonPrefixes().size());
assertEquals("foo%2B1/", adapter.marshal(response.getCommonPrefixes().get(0).getPrefix()));
assertEquals("foo/", adapter.marshal(response.getCommonPrefixes().get(1).getPrefix()));
assertEquals("quux%20ab/", adapter.marshal(response.getCommonPrefixes().get(2).getPrefix()));
}

@Test
public void testListObjectsOmitsDelimiterWhenEmpty() throws Exception {
OzoneClient client = createClientWithKeys("bar", "baz", "cab", "foo");
BucketEndpoint endpoint = newBucketEndpointBuilder().setClient(client).build();

endpoint.queryParamsForTest().set(QueryParams.DELIMITER, "");
ListObjectResponse response = (ListObjectResponse) endpoint.get("b1").getEntity();

assertNull(response.getDelimiter());
assertEquals(4, response.getContents().size());
assertEquals(0, response.getCommonPrefixes().size());
}

@Test
public void testListObjectsPrefixWithNewline() throws Exception {
OzoneClient client = createClientWithKeys("foo/bar", "foo/baz", "quux");
BucketEndpoint endpoint = newBucketEndpointBuilder().setClient(client).build();

String prefix = String.valueOf((char) 0x0a);
endpoint.queryParamsForTest().set(QueryParams.PREFIX, prefix);
endpoint.queryParamsForTest().set(QueryParams.ENCODING_TYPE, ENCODING_TYPE);
ListObjectResponse response = (ListObjectResponse) endpoint.get("b1").getEntity();

assertNotNull(response.getPrefix());
assertEquals(prefix, response.getPrefix().getName());
assertNull(response.getPrefix().getEncodingType());
assertEquals(prefix, new ObjectKeyNameAdapter().marshal(response.getPrefix()));
assertEquals(0, response.getContents().size());
assertEquals(0, response.getCommonPrefixes().size());
}

private void assertEncodingTypeObject(
String exceptName, String exceptEncodingType, EncodingTypeObject object) {
assertEquals(exceptName, object.getName());
Expand Down