From 3e1e7aa1ed2b6e959f143f474f323ad4d5c6b90d Mon Sep 17 00:00:00 2001 From: Fabian Morgan Date: Wed, 18 Mar 2026 14:09:41 -0700 Subject: [PATCH] fix latent S3 API issue when ListBuckets missing a required permission --- .../ozone/s3/endpoint/EndpointBase.java | 43 +++++++++++------- .../ozone/s3/endpoint/TestEndpointBase.java | 45 +++++++++++++++++++ 2 files changed, 71 insertions(+), 17 deletions(-) diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/EndpointBase.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/EndpointBase.java index 53bba420e67e..20ad21e23f11 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/EndpointBase.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/EndpointBase.java @@ -379,24 +379,33 @@ private Iterator iterateBuckets( OzoneVolume volume = getVolume(); ownerSetter.accept(volume); return query.apply(volume); - } catch (OMException e) { - if (e.getResult() == ResultCodes.VOLUME_NOT_FOUND) { - return Collections.emptyIterator(); - } else if (e.getResult() == ResultCodes.PERMISSION_DENIED) { - throw newError(S3ErrorTable.ACCESS_DENIED, - "listBuckets", e); - } else if (isExpiredToken(e)) { - throw newError(S3ErrorTable.EXPIRED_TOKEN, s3Auth.getAccessID(), e); - } else if (e.getResult() == ResultCodes.INVALID_TOKEN) { - throw newError(S3ErrorTable.ACCESS_DENIED, - s3Auth.getAccessID(), e); - } else if (e.getResult() == ResultCodes.TIMEOUT || - e.getResult() == ResultCodes.INTERNAL_ERROR) { - throw newError(S3ErrorTable.INTERNAL_ERROR, - "listBuckets", e); - } else { - throw e; + } catch (RuntimeException e) { + if (e.getCause() instanceof OMException) { + return handleOMException((OMException) e.getCause()); } + throw e; + } catch (OMException e) { + return handleOMException(e); + } + } + + private Iterator handleOMException(OMException e) throws OMException { + if (e.getResult() == ResultCodes.VOLUME_NOT_FOUND) { + return Collections.emptyIterator(); + } else if (e.getResult() == ResultCodes.PERMISSION_DENIED) { + throw newError(S3ErrorTable.ACCESS_DENIED, + "listBuckets", e); + } else if (isExpiredToken(e)) { + throw newError(S3ErrorTable.EXPIRED_TOKEN, s3Auth.getAccessID(), e); + } else if (e.getResult() == ResultCodes.INVALID_TOKEN) { + throw newError(S3ErrorTable.ACCESS_DENIED, + s3Auth.getAccessID(), e); + } else if (e.getResult() == ResultCodes.TIMEOUT || + e.getResult() == ResultCodes.INTERNAL_ERROR) { + throw newError(S3ErrorTable.INTERNAL_ERROR, + "listBuckets", e); + } else { + throw e; } } diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestEndpointBase.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestEndpointBase.java index 89d5a26f21a5..c5042ab5b248 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestEndpointBase.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestEndpointBase.java @@ -24,6 +24,9 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import java.nio.charset.StandardCharsets; import java.util.Locale; @@ -31,6 +34,7 @@ import javax.ws.rs.core.MultivaluedHashMap; import javax.ws.rs.core.MultivaluedMap; import org.apache.hadoop.ozone.OzoneConsts; +import org.apache.hadoop.ozone.client.OzoneVolume; import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.s3.exception.OS3Exception; import org.junit.jupiter.api.Test; @@ -137,4 +141,45 @@ public void init() { } assertFalse(endpointBase.isExpiredToken(new OMException(ResultCodes.INVALID_TOKEN))); } + @Test + public void testListS3BucketsHandlesRuntimeExceptionWrappingOMException() throws Exception { + final EndpointBase endpointBase = new EndpointBase() { + @Override + public void init() { } + + @Override + protected OzoneVolume getVolume() { + final OzoneVolume volume = mock(OzoneVolume.class); + when(volume.listBuckets(anyString())).thenThrow( + new RuntimeException(new OMException("Permission Denied", ResultCodes.PERMISSION_DENIED))); + return volume; + } + }; + + final OS3Exception e = assertThrows( + OS3Exception.class, () -> endpointBase.listS3Buckets( + "prefix", volume -> { }), "listS3Buckets should fail."); + + // Ensure we get the correct code + assertEquals("AccessDenied", e.getCode()); + } + + @Test + public void testListS3BucketsHandlesRuntimeExceptionWrappingOMExceptionVolumeNotFound() throws Exception { + final EndpointBase endpointBase = new EndpointBase() { + @Override + public void init() { } + + @Override + protected OzoneVolume getVolume() { + final OzoneVolume volume = mock(OzoneVolume.class); + when(volume.listBuckets(anyString())).thenThrow( + new RuntimeException(new OMException("Volume Not Found", ResultCodes.VOLUME_NOT_FOUND))); + return volume; + } + }; + + // Ensure we get an empty iterator + assertFalse(endpointBase.listS3Buckets("prefix", volume -> { }).hasNext()); + } }