This is an automated email from the ASF dual-hosted git repository.
mchades pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 364e145c80 [#12834] fix(server): reject null request bodies in
remaining REST operations (#12866)
364e145c80 is described below
commit 364e145c804f6c753ed3b5ad0bd8478e170d99b3
Author: Shuai Liu <[email protected]>
AuthorDate: Thu Sep 3 18:03:54 2026 +0800
[#12834] fix(server): reject null request bodies in remaining REST
operations (#12866)
### What changes were proposed in this pull request?
- Reject a null request body in the 20 operations listed in #12834,
using operation-level guards in 9 resource classes.
- Move the pre-existing `updateStatistics` guard out of the `Utils.doAs`
lambda to the front of the method and drop its private message constant,
so all four operations in `StatisticOperations` report the same message.
- Return the 400 for `testConnection` directly through
`Utils.illegalArguments(...)` instead of
`handleTestConnectionException`, which by design reports connection-test
outcomes inside an HTTP 200 body.
- Follow the existing pattern of `MetalakeOperations.createMetalake`.
- Simplify the null-safe field extractions used for logging and are now
unreachable, in `testConnection`, `PermissionOperations` and
`StatisticOperations`.
- Add regression coverage for every fixed operation, plus a
malformed-JSON test and a normal-path `setMetalake` test.
### Why are the changes needed?
An omitted body, an empty body, or a JSON literal `null` is a client
error, but these endpoints answer it with HTTP 500.
Fix: #12834
### Does this PR introduce _any_ user-facing change?
Yes. These endpoints now return HTTP 400 with error code `1001`, error
type `IllegalArgumentException`, and a message stating that the request
body cannot be null, instead of HTTP 500.
One existing message also changes: a null body on `PUT
/api/metalakes/{metalake}/objects/{type}/{fullName}/statistics`
previously returned `Statistics update request body cannot be null` and
now returns the canonical `Request body cannot be null`. Its status code
and error code are unchanged.
There are no API schema or configuration changes, valid requests behave
exactly as before, and malformed-JSON handling is unchanged.
### How was this patch tested?
Added 21 `testXxxWithNullRequest()` tests, one per fixed operation plus
one for the deprecated tag route, using the shared
`assertNullRequestBodyRejected` helper introduced by #12770.
`TestStatisticOperations` and `TestMetadataObjectPolicyOperations` now
extend `BaseOperationsTest` to use it, and the pre-existing
`testUpdateTableStatisticsWithNullRequestBody` was converged onto it.
Also added `testSetMetalakeWithMalformedJson`, which sends `{` and
asserts the response still comes from the JSON exception mappers, and
`testSetMetalake` for the normal enable/disable path, which had no
coverage before.
With the production changes reverted and only the tests applied, all 21
null-body tests fail with:
```text
expected: <400> but was: <500>
```
With the fix applied:
```shell
./gradlew :server:test -PskipITs --no-daemon # 396 tests, 0
failures, 0 errors
./gradlew :server:spotlessCheck --no-daemon
git diff --check
```
### Note for reviewers
With `gravitino.authorization.enable=true` the two `associate*`
endpoints never reach the new guard: `AssociateTagAuthorizationExecutor`
/ `AssociatePolicyAuthorizationExecutor` return `false` for a null
request, so `GravitinoInterceptionService` answers 403 before the
resource method runs. The guard is still correct for the default
configuration, where that service is not registered at all. I left the
executors alone as changing an authorization decision felt out of scope;
a follow-up would have to `return true` there, so that a null body
reaches the guard in the resource method and is rejected with the 400.
Happy to file that issue if you would like it handled.
---
.../gravitino/server/web/rest/BulkOperations.java | 16 +++++
.../server/web/rest/CatalogOperations.java | 18 ++++-
.../web/rest/MetadataObjectPolicyOperations.java | 9 ++-
.../web/rest/MetadataObjectTagOperations.java | 18 +++++
.../server/web/rest/MetalakeOperations.java | 7 ++
.../gravitino/server/web/rest/OwnerOperations.java | 8 +++
.../server/web/rest/PermissionOperations.java | 64 ++++++++++++++++--
.../server/web/rest/PolicyOperations.java | 9 ++-
.../server/web/rest/StatisticOperations.java | 50 ++++++++++----
.../server/web/rest/TestBulkOperations.java | 22 ++++++
.../server/web/rest/TestCatalogOperations.java | 28 ++++++++
.../rest/TestMetadataObjectPolicyOperations.java | 19 +++++-
.../web/rest/TestMetadataObjectTagOperations.java | 34 ++++++++++
.../server/web/rest/TestMetalakeOperations.java | 63 +++++++++++++++++
.../server/web/rest/TestOwnerOperations.java | 11 +++
.../server/web/rest/TestPermissionOperations.java | 77 +++++++++++++++++++++
.../server/web/rest/TestPolicyOperations.java | 13 ++++
.../server/web/rest/TestStatisticOperations.java | 79 ++++++++++++++++++++--
.../server/web/rest/TestTagOperations.java | 16 +++++
19 files changed, 532 insertions(+), 29 deletions(-)
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/BulkOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/BulkOperations.java
index 9c9eb8796f..58be4935a3 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/BulkOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/BulkOperations.java
@@ -158,6 +158,14 @@ public class BulkOperations {
@PathParam("metalake") @AuthorizationMetadata(type =
Entity.EntityType.METALAKE)
String metalake,
BulkRemoveRequest request) {
+ if (request == null) {
+ return ExceptionHandlers.handleUserException(
+ OperationType.REMOVE,
+ "",
+ metalake,
+ new IllegalArgumentException("Request body cannot be null"));
+ }
+
try {
return Utils.doAs(
httpRequest,
@@ -264,6 +272,14 @@ public class BulkOperations {
@PathParam("metalake") @AuthorizationMetadata(type =
Entity.EntityType.METALAKE)
String metalake,
BulkRemoveRequest request) {
+ if (request == null) {
+ return ExceptionHandlers.handleGroupException(
+ OperationType.REMOVE,
+ "",
+ metalake,
+ new IllegalArgumentException("Request body cannot be null"));
+ }
+
try {
return Utils.doAs(
httpRequest,
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java
index ea5d99cdbd..9aab629818 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java
@@ -192,7 +192,14 @@ public class CatalogOperations {
@PathParam("metalake") @AuthorizationMetadata(type =
Entity.EntityType.METALAKE)
String metalake,
CatalogCreateRequest request) {
- String catalogName = request == null ? "" : request.getName();
+ if (request == null) {
+ // Unlike a failed connection test, which
handleTestConnectionException() reports inside an
+ // HTTP 200 response by design, a missing request body never reaches the
connection test, so
+ // it is rejected with a regular HTTP 400.
+ return Utils.illegalArguments("Request body cannot be null");
+ }
+
+ String catalogName = request.getName();
LOG.info("Received test connection request for catalog: {}.{}", metalake,
catalogName);
try {
return Utils.doAs(
@@ -264,8 +271,15 @@ public class CatalogOperations {
@PathParam("catalog") @AuthorizationMetadata(type =
Entity.EntityType.CATALOG)
String catalogName,
CatalogSetRequest request) {
- LOG.info("Received set request for catalog: {}.{}", metalake, catalogName);
+ if (request == null) {
+ return ExceptionHandlers.handleCatalogException(
+ OperationType.SET,
+ catalogName,
+ metalake,
+ new IllegalArgumentException("Request body cannot be null"));
+ }
+ LOG.info("Received set request for catalog: {}.{}", metalake, catalogName);
OperationType op = request.isInUse() ? OperationType.ENABLE :
OperationType.DISABLE;
try {
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/MetadataObjectPolicyOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/MetadataObjectPolicyOperations.java
index 78a03570fb..19ea45ab96 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/MetadataObjectPolicyOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/MetadataObjectPolicyOperations.java
@@ -251,12 +251,19 @@ public class MetadataObjectPolicyOperations {
@PathParam("fullName") @AuthorizationFullName String fullName,
@AuthorizationRequest(type =
AuthorizationRequest.RequestType.ASSOCIATE_POLICY)
PoliciesAssociateRequest request) {
+ if (request == null) {
+ return ExceptionHandlers.handlePolicyException(
+ OperationType.ASSOCIATE,
+ "",
+ fullName,
+ new IllegalArgumentException("Request body cannot be null"));
+ }
+
LOG.info(
"Received associate policies request for object type: {}, full name:
{} under metalake: {}",
type,
fullName,
metalake);
-
try {
return Utils.doAs(
httpRequest,
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/MetadataObjectTagOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/MetadataObjectTagOperations.java
index a77584b3cb..072f3876cc 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/MetadataObjectTagOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/MetadataObjectTagOperations.java
@@ -289,6 +289,14 @@ public class MetadataObjectTagOperations {
private Response associateTagsForObjectInternal(
String metalake, String type, String fullName, TagsAssociateRequest
request) {
+ if (request == null) {
+ return ExceptionHandlers.handleTagException(
+ OperationType.ASSOCIATE,
+ "",
+ fullName,
+ new IllegalArgumentException("Request body cannot be null"));
+ }
+
LOG.info(
"Received associate tags request for object type: {}, full name: {}
under metalake: {}",
type,
@@ -314,6 +322,16 @@ public class MetadataObjectTagOperations {
private Response associateTagValuesForObjectInternal(
String metalake, String type, String fullName, TagValuesAssociateRequest
request) {
+ if (request == null) {
+ return withMediaType(
+ ExceptionHandlers.handleTagException(
+ OperationType.ASSOCIATE,
+ "",
+ fullName,
+ new IllegalArgumentException("Request body cannot be null")),
+ TAG_VALUES_MEDIA_TYPE);
+ }
+
LOG.info(
"Received associate tag values request for object type: {}, full name:
{} under metalake: {}",
type,
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/MetalakeOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/MetalakeOperations.java
index 8d511a01dd..5e6dd53b9b 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/MetalakeOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/MetalakeOperations.java
@@ -181,6 +181,13 @@ public class MetalakeOperations {
@PathParam("name") @AuthorizationMetadata(type =
Entity.EntityType.METALAKE)
String metalakeName,
MetalakeSetRequest request) {
+ if (request == null) {
+ return ExceptionHandlers.handleMetalakeException(
+ OperationType.SET,
+ metalakeName,
+ new IllegalArgumentException("Request body cannot be null"));
+ }
+
LOG.info("Received set request for metalake: {}", metalakeName);
try {
return Utils.doAs(
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/OwnerOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/OwnerOperations.java
index dbe91956b0..962dfde6a2 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/OwnerOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/OwnerOperations.java
@@ -119,6 +119,14 @@ public class OwnerOperations {
@PathParam("metadataObjectType") String metadataObjectType,
@PathParam("fullName") String fullName,
OwnerSetRequest request) {
+ if (request == null) {
+ return ExceptionHandlers.handleOwnerException(
+ OperationType.SET,
+ String.format("metadata object %s", fullName),
+ metalake,
+ new IllegalArgumentException("Request body cannot be null"));
+ }
+
try {
MetadataObject object =
MetadataObjects.parse(
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/PermissionOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/PermissionOperations.java
index b7d0690f5a..bce5335d9b 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/PermissionOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/PermissionOperations.java
@@ -88,7 +88,15 @@ public class PermissionOperations {
String metalake,
@PathParam("user") String user,
RoleGrantRequest request) {
- String roleNames = request == null ? "" :
StringUtils.join(request.getRoleNames(), ",");
+ if (request == null) {
+ return ExceptionHandlers.handleUserPermissionOperationException(
+ OperationType.GRANT,
+ "",
+ user,
+ new IllegalArgumentException("Request body cannot be null"));
+ }
+
+ String roleNames = StringUtils.join(request.getRoleNames(), ",");
try {
return Utils.doAs(
httpRequest,
@@ -118,7 +126,15 @@ public class PermissionOperations {
String metalake,
@PathParam("group") String group,
RoleGrantRequest request) {
- String roleNames = request == null ? "" :
StringUtils.join(request.getRoleNames(), ",");
+ if (request == null) {
+ return ExceptionHandlers.handleGroupPermissionOperationException(
+ OperationType.GRANT,
+ "",
+ group,
+ new IllegalArgumentException("Request body cannot be null"));
+ }
+
+ String roleNames = StringUtils.join(request.getRoleNames(), ",");
try {
return Utils.doAs(
httpRequest,
@@ -148,7 +164,15 @@ public class PermissionOperations {
String metalake,
@PathParam("user") String user,
RoleRevokeRequest request) {
- String roleNames = request == null ? "" :
StringUtils.join(request.getRoleNames(), ",");
+ if (request == null) {
+ return ExceptionHandlers.handleUserPermissionOperationException(
+ OperationType.REVOKE,
+ "",
+ user,
+ new IllegalArgumentException("Request body cannot be null"));
+ }
+
+ String roleNames = StringUtils.join(request.getRoleNames(), ",");
try {
return Utils.doAs(
httpRequest,
@@ -178,7 +202,15 @@ public class PermissionOperations {
String metalake,
@PathParam("group") String group,
RoleRevokeRequest request) {
- String roleNames = request == null ? "" :
StringUtils.join(request.getRoleNames(), ",");
+ if (request == null) {
+ return ExceptionHandlers.handleGroupPermissionOperationException(
+ OperationType.REVOKE,
+ "",
+ group,
+ new IllegalArgumentException("Request body cannot be null"));
+ }
+
+ String roleNames = StringUtils.join(request.getRoleNames(), ",");
try {
return Utils.doAs(
httpRequest,
@@ -212,6 +244,14 @@ public class PermissionOperations {
@PathParam("type") String type,
@PathParam("fullName") String fullName,
PrivilegeGrantRequest privilegeGrantRequest) {
+ if (privilegeGrantRequest == null) {
+ return ExceptionHandlers.handleRolePermissionOperationException(
+ OperationType.GRANT,
+ fullName,
+ role,
+ new IllegalArgumentException("Request body cannot be null"));
+ }
+
try {
MetadataObject object =
MetadataObjects.parse(
@@ -260,6 +300,14 @@ public class PermissionOperations {
@PathParam("type") String type,
@PathParam("fullName") String fullName,
PrivilegeRevokeRequest privilegeRevokeRequest) {
+ if (privilegeRevokeRequest == null) {
+ return ExceptionHandlers.handleRolePermissionOperationException(
+ OperationType.REVOKE,
+ fullName,
+ role,
+ new IllegalArgumentException("Request body cannot be null"));
+ }
+
try {
MetadataObject object =
MetadataObjects.parse(
@@ -304,6 +352,14 @@ public class PermissionOperations {
String metalake,
@PathParam("role") String role,
PrivilegeOverrideRequest request) {
+ if (request == null) {
+ return ExceptionHandlers.handleRolePermissionOperationException(
+ OperationType.UPDATE,
+ role,
+ metalake,
+ new IllegalArgumentException("Request body cannot be null"));
+ }
+
try {
return Utils.doAs(
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/PolicyOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/PolicyOperations.java
index ec336b74ae..1816356524 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/PolicyOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/PolicyOperations.java
@@ -253,8 +253,15 @@ public class PolicyOperations {
String metalake,
@PathParam("policy") @AuthorizationMetadata(type =
Entity.EntityType.POLICY) String name,
PolicySetRequest request) {
- LOG.info("Received set policy request for policy: {} under metalake: {}",
name, metalake);
+ if (request == null) {
+ return ExceptionHandlers.handlePolicyException(
+ OperationType.SET,
+ name,
+ metalake,
+ new IllegalArgumentException("Request body cannot be null"));
+ }
+ LOG.info("Received set policy request for policy: {} under metalake: {}",
name, metalake);
OperationType op = request.isEnable() ? OperationType.ENABLE :
OperationType.DISABLE;
try {
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/StatisticOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/StatisticOperations.java
index 5b99050d95..79887df7dd 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/StatisticOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/StatisticOperations.java
@@ -81,9 +81,6 @@ public class StatisticOperations {
private static final Logger LOG =
LoggerFactory.getLogger(StatisticOperations.class);
- private static final String NULL_STATS_UPDATE_REQUEST_BODY_ERROR =
- "Statistics update request body cannot be null";
-
@Context private HttpServletRequest httpRequest;
private final StatisticDispatcher statisticDispatcher;
@@ -154,6 +151,14 @@ public class StatisticOperations {
@PathParam("type") @AuthorizationObjectType String type,
@PathParam("fullName") @AuthorizationFullName String fullName,
StatisticsUpdateRequest request) {
+ if (request == null) {
+ return ExceptionHandlers.handleStatisticException(
+ OperationType.UPDATE,
+ "",
+ fullName,
+ new IllegalArgumentException("Request body cannot be null"));
+ }
+
String statisticNames = getStatisticNames(request);
try {
LOG.info(
@@ -164,9 +169,6 @@ public class StatisticOperations {
return Utils.doAs(
httpRequest,
() -> {
- if (request == null) {
- throw new
IllegalArgumentException(NULL_STATS_UPDATE_REQUEST_BODY_ERROR);
- }
request.validate();
MetadataObject object =
MetadataObjects.parse(
@@ -215,10 +217,16 @@ public class StatisticOperations {
@PathParam("type") @AuthorizationObjectType String type,
@PathParam("fullName") @AuthorizationFullName String fullName,
StatisticsDropRequest request) {
+ if (request == null) {
+ return ExceptionHandlers.handleStatisticException(
+ OperationType.DROP,
+ "",
+ fullName,
+ new IllegalArgumentException("Request body cannot be null"));
+ }
+
String statisticNames =
- request == null || request.getNames() == null
- ? ""
- : StringUtils.join(request.getNames(), ",");
+ request.getNames() == null ? "" : StringUtils.join(request.getNames(),
",");
try {
LOG.info(
"Received drop statistics request for object full name: {} type: {}
in the metalake {}",
@@ -358,8 +366,16 @@ public class StatisticOperations {
@PathParam("type") @AuthorizationObjectType String type,
@PathParam("fullName") @AuthorizationFullName String fullName,
PartitionStatisticsUpdateRequest request) {
- String partitions = getPartitionNames(request);
+ if (request == null) {
+ return ExceptionHandlers.handlePartitionStatsException(
+ OperationType.UPDATE,
+ "",
+ fullName,
+ new IllegalArgumentException("Request body cannot be null"));
+ }
+
LOG.info("Updating partition statistics for table: {} in the metalake {}",
fullName, metalake);
+ String partitions = getPartitionNames(request);
try {
return Utils.doAs(
httpRequest,
@@ -432,6 +448,14 @@ public class StatisticOperations {
@PathParam("type") @AuthorizationObjectType String type,
@PathParam("fullName") @AuthorizationFullName String fullName,
PartitionStatisticsDropRequest request) {
+ if (request == null) {
+ return ExceptionHandlers.handlePartitionStatsException(
+ OperationType.DROP,
+ "",
+ fullName,
+ new IllegalArgumentException("Request body cannot be null"));
+ }
+
String partitions = getDropPartitionNames(request);
try {
@@ -503,7 +527,7 @@ public class StatisticOperations {
}
private static String getStatisticNames(StatisticsUpdateRequest request) {
- if (request == null || request.getUpdates() == null) {
+ if (request.getUpdates() == null) {
return "";
}
@@ -511,7 +535,7 @@ public class StatisticOperations {
}
private static String getPartitionNames(PartitionStatisticsUpdateRequest
request) {
- if (request == null || request.getUpdates() == null) {
+ if (request.getUpdates() == null) {
return "";
}
@@ -523,7 +547,7 @@ public class StatisticOperations {
}
private static String getDropPartitionNames(PartitionStatisticsDropRequest
request) {
- if (request == null || request.getDrops() == null) {
+ if (request.getDrops() == null) {
return "";
}
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestBulkOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestBulkOperations.java
index d309b1567b..5c008c19c9 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestBulkOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestBulkOperations.java
@@ -303,6 +303,28 @@ public class TestBulkOperations extends BaseOperationsTest
{
Assertions.assertEquals(ErrorConstants.NOT_FOUND_CODE,
bulkResponse.getErrors()[0].getCode());
}
+ @Test
+ public void testRemoveUsersWithNullRequest() {
+ Response response =
+ target("/bulk/metalakes/metalake1/users/remove")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .post(Entity.entity("null", MediaType.APPLICATION_JSON_TYPE));
+
+ assertNullRequestBodyRejected(response);
+ }
+
+ @Test
+ public void testRemoveGroupsWithNullRequest() {
+ Response response =
+ target("/bulk/metalakes/metalake1/groups/remove")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .post(Entity.entity(new byte[0], MediaType.APPLICATION_JSON_TYPE));
+
+ assertNullRequestBodyRejected(response);
+ }
+
@Test
public void testBulkRejectsEmptyAndExceededRequest() {
Response emptyResponse =
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestCatalogOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestCatalogOperations.java
index ebbc7a52a1..363a03c322 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestCatalogOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestCatalogOperations.java
@@ -30,6 +30,8 @@ import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import com.google.common.collect.ImmutableList;
@@ -316,6 +318,20 @@ public class TestCatalogOperations extends
BaseOperationsTest {
Assertions.assertEquals(RuntimeException.class.getSimpleName(),
errorResponse2.getType());
}
+ @Test
+ public void testTestConnectionWithNullRequest() {
+ Response resp =
+ target("/metalakes/metalake1/catalogs/testConnection")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .post(Entity.entity("null", MediaType.APPLICATION_JSON_TYPE));
+
+ // A missing request body never reaches the connection test, so it is
reported as a regular
+ // HTTP 400 rather than through the HTTP 200 envelope used for
connection-test failures.
+ assertNullRequestBodyRejected(resp);
+ verify(manager, never()).testConnection(any(), any(), any(), any(), any());
+ }
+
@Test
public void testConnection() {
CatalogCreateRequest req =
@@ -693,6 +709,18 @@ public class TestCatalogOperations extends
BaseOperationsTest {
Assertions.assertEquals(RuntimeException.class.getSimpleName(),
errorResponse1.getType());
}
+ @Test
+ public void testSetCatalogWithNullRequest() {
+ Response resp =
+ target("/metalakes/metalake1/catalogs/catalog1")
+ .property(HttpUrlConnectorProvider.SET_METHOD_WORKAROUND, true)
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .method("PATCH", Entity.entity("null",
MediaType.APPLICATION_JSON_TYPE));
+
+ assertNullRequestBodyRejected(resp);
+ }
+
private static TestCatalog buildCatalogWithProperties(
String metalake, String catalogName, Map<String, String> properties) {
CatalogEntity entity =
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestMetadataObjectPolicyOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestMetadataObjectPolicyOperations.java
index e95b06fb88..6ff31e2203 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestMetadataObjectPolicyOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestMetadataObjectPolicyOperations.java
@@ -57,12 +57,11 @@ import org.apache.gravitino.policy.PolicyManager;
import org.apache.gravitino.rest.RESTUtils;
import org.glassfish.jersey.internal.inject.AbstractBinder;
import org.glassfish.jersey.server.ResourceConfig;
-import org.glassfish.jersey.test.JerseyTest;
import org.glassfish.jersey.test.TestProperties;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
-public class TestMetadataObjectPolicyOperations extends JerseyTest {
+public class TestMetadataObjectPolicyOperations extends BaseOperationsTest {
private static class MockServletRequestFactory extends
ServletRequestFactoryBase {
@@ -669,4 +668,20 @@ public class TestMetadataObjectPolicyOperations extends
JerseyTest {
.withAuditInfo(testAuditInfo1)
.build();
}
+
+ @Test
+ public void testAssociatePoliciesForObjectWithNullRequest() {
+ MetadataObject catalog = MetadataObjects.parse("object1",
MetadataObject.Type.CATALOG);
+
+ Response response =
+ target(basePath(metalake))
+ .path(catalog.type().toString())
+ .path(catalog.fullName())
+ .path("policies")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .post(Entity.entity("null", MediaType.APPLICATION_JSON_TYPE));
+
+ assertNullRequestBodyRejected(response);
+ }
}
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestMetadataObjectTagOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestMetadataObjectTagOperations.java
index 1a291c305e..82a38c2849 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestMetadataObjectTagOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestMetadataObjectTagOperations.java
@@ -1117,6 +1117,40 @@ public class TestMetadataObjectTagOperations extends
BaseOperationsTest {
response.readEntity(NameListResponse.class).getNames());
}
+ @Test
+ public void testAssociateTagsForObjectWithNullRequest() {
+ MetadataObject catalog = MetadataObjects.parse("object1",
MetadataObject.Type.CATALOG);
+
+ Response response =
+ target(basePath(metalake))
+ .path(catalog.type().toString())
+ .path(catalog.fullName())
+ .path("tags")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .post(Entity.entity("null", MediaType.APPLICATION_JSON_TYPE));
+
+ assertNullRequestBodyRejected(response);
+ }
+
+ @Test
+ public void testAssociateTagValuesForObjectWithNullRequest() {
+ MetadataObject catalog = MetadataObjects.parse("object1",
MetadataObject.Type.CATALOG);
+
+ Response response =
+ target(basePath(metalake))
+ .path(catalog.type().toString())
+ .path(catalog.fullName())
+ .path("tags")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v2+json")
+ .post(Entity.entity("null", "application/vnd.gravitino.v2+json"));
+
+ assertNullRequestBodyRejected(response);
+ Assertions.assertEquals(
+ MediaType.valueOf("application/vnd.gravitino.v2+json"),
response.getMediaType());
+ }
+
@Test
public void testV2ErrorMediaType() {
MetadataObject catalog = MetadataObjects.parse("object1",
MetadataObject.Type.CATALOG);
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestMetalakeOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestMetalakeOperations.java
index ae7eb39133..a2cbd0b251 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestMetalakeOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestMetalakeOperations.java
@@ -51,6 +51,7 @@ import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.MetalakeChange;
import org.apache.gravitino.dto.MetalakeDTO;
import org.apache.gravitino.dto.requests.MetalakeCreateRequest;
+import org.apache.gravitino.dto.requests.MetalakeSetRequest;
import org.apache.gravitino.dto.requests.MetalakeUpdateRequest;
import org.apache.gravitino.dto.requests.MetalakeUpdatesRequest;
import org.apache.gravitino.dto.responses.DropResponse;
@@ -70,6 +71,7 @@ import
org.apache.gravitino.server.web.mapper.JsonMappingExceptionMapper;
import org.apache.gravitino.server.web.mapper.JsonParseExceptionMapper;
import org.apache.gravitino.server.web.mapper.JsonProcessingExceptionMapper;
import org.glassfish.hk2.utilities.binding.AbstractBinder;
+import org.glassfish.jersey.client.HttpUrlConnectorProvider;
import org.glassfish.jersey.server.ResourceConfig;
import org.glassfish.jersey.test.TestProperties;
import org.junit.jupiter.api.Assertions;
@@ -263,6 +265,67 @@ public class TestMetalakeOperations extends
BaseOperationsTest {
assertNullRequestBodyRejected(resp);
}
+ @Test
+ public void testSetMetalake() {
+ Mockito.doNothing().when(metalakeManager).enableMetalake(any());
+ Mockito.doNothing().when(metalakeManager).disableMetalake(any());
+
+ Response enableResp =
+ target("/metalakes/test")
+ .property(HttpUrlConnectorProvider.SET_METHOD_WORKAROUND, true)
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .method(
+ "PATCH",
+ Entity.entity(new MetalakeSetRequest(true),
MediaType.APPLICATION_JSON_TYPE));
+
+ Assertions.assertEquals(Response.Status.OK.getStatusCode(),
enableResp.getStatus());
+ Mockito.verify(metalakeManager).enableMetalake(any());
+
+ Response disableResp =
+ target("/metalakes/test")
+ .property(HttpUrlConnectorProvider.SET_METHOD_WORKAROUND, true)
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .method(
+ "PATCH",
+ Entity.entity(new MetalakeSetRequest(false),
MediaType.APPLICATION_JSON_TYPE));
+
+ Assertions.assertEquals(Response.Status.OK.getStatusCode(),
disableResp.getStatus());
+ Mockito.verify(metalakeManager).disableMetalake(any());
+ }
+
+ @Test
+ public void testSetMetalakeWithNullRequest() {
+ Response resp =
+ target("/metalakes/test")
+ .property(HttpUrlConnectorProvider.SET_METHOD_WORKAROUND, true)
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .method("PATCH", Entity.entity("null",
MediaType.APPLICATION_JSON_TYPE));
+
+ assertNullRequestBodyRejected(resp);
+ }
+
+ @Test
+ public void testSetMetalakeWithMalformedJson() {
+ // The null-body guard must not take over malformed JSON, which keeps
using the registered
+ // Jackson exception mappers.
+ Response resp =
+ target("/metalakes/test")
+ .property(HttpUrlConnectorProvider.SET_METHOD_WORKAROUND, true)
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .method("PATCH", Entity.entity("{",
MediaType.APPLICATION_JSON_TYPE));
+
+ Assertions.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(),
resp.getStatus());
+ ErrorResponse errorResponse = resp.readEntity(ErrorResponse.class);
+ Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE,
errorResponse.getCode());
+ Assertions.assertEquals(
+ IllegalArgumentException.class.getSimpleName(),
errorResponse.getType());
+ Assertions.assertTrue(errorResponse.getMessage().contains("Malformed json
request"));
+ }
+
@Test
public void testLoadMetalake() {
String metalakeName = "test";
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestOwnerOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestOwnerOperations.java
index 97eb522374..b342f3cfda 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestOwnerOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestOwnerOperations.java
@@ -316,6 +316,17 @@ class TestOwnerOperations extends BaseOperationsTest {
Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE,
errorResponse3.getCode());
}
+ @Test
+ public void testSetOwnerForObjectWithNullRequest() {
+ Response resp =
+ target("/metalakes/metalake1/owners/metalake/metalake1")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .put(Entity.entity("null", MediaType.APPLICATION_JSON_TYPE));
+
+ assertNullRequestBodyRejected(resp);
+ }
+
@Test
public void testRoleObject() {
MetadataObject role = MetadataObjects.of(null, "role",
MetadataObject.Type.ROLE);
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestPermissionOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestPermissionOperations.java
index 6a18db9af7..037490a407 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestPermissionOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestPermissionOperations.java
@@ -812,4 +812,81 @@ public class TestPermissionOperations extends
BaseOperationsTest {
Assertions.assertEquals(ErrorConstants.INTERNAL_ERROR_CODE,
errorResponse2.getCode());
Assertions.assertEquals(RuntimeException.class.getSimpleName(),
errorResponse2.getType());
}
+
+ @Test
+ public void testGrantRolesToUserWithNullRequest() {
+ Response resp =
+ target("/metalakes/metalake1/permissions/users/user1/grant")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .put(Entity.entity("null", MediaType.APPLICATION_JSON_TYPE));
+
+ assertNullRequestBodyRejected(resp);
+ }
+
+ @Test
+ public void testGrantRolesToGroupWithNullRequest() {
+ Response resp =
+ target("/metalakes/metalake1/permissions/groups/group1/grant")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .put(Entity.entity("null", MediaType.APPLICATION_JSON_TYPE));
+
+ assertNullRequestBodyRejected(resp);
+ }
+
+ @Test
+ public void testRevokeRolesFromUserWithNullRequest() {
+ Response resp =
+ target("/metalakes/metalake1/permissions/users/user1/revoke")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .put(Entity.entity("null", MediaType.APPLICATION_JSON_TYPE));
+
+ assertNullRequestBodyRejected(resp);
+ }
+
+ @Test
+ public void testRevokeRolesFromGroupWithNullRequest() {
+ Response resp =
+ target("/metalakes/metalake1/permissions/groups/group1/revoke")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .put(Entity.entity("null", MediaType.APPLICATION_JSON_TYPE));
+
+ assertNullRequestBodyRejected(resp);
+ }
+
+ @Test
+ public void testGrantPrivilegeToRoleWithNullRequest() {
+ Response resp =
+
target("/metalakes/metalake1/permissions/roles/role1/metalake/metalake1/grant")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .put(Entity.entity("null", MediaType.APPLICATION_JSON_TYPE));
+
+ assertNullRequestBodyRejected(resp);
+ }
+
+ @Test
+ public void testRevokePrivilegeFromRoleWithNullRequest() {
+ Response resp =
+
target("/metalakes/metalake1/permissions/roles/role1/metalake/metalake1/revoke")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .put(Entity.entity("null", MediaType.APPLICATION_JSON_TYPE));
+
+ assertNullRequestBodyRejected(resp);
+ }
+
+ @Test
+ public void testOverrideRolePrivilegesWithNullRequest() {
+ Response resp =
+ target("/metalakes/metalake1/permissions/roles/role1/")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .put(Entity.entity(new byte[0], MediaType.APPLICATION_JSON_TYPE));
+
+ assertNullRequestBodyRejected(resp);
+ }
}
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestPolicyOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestPolicyOperations.java
index 3095512cac..85e45239fe 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestPolicyOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestPolicyOperations.java
@@ -671,6 +671,19 @@ public class TestPolicyOperations extends
BaseOperationsTest {
Assertions.assertEquals(RuntimeException.class.getSimpleName(),
errorResp.getType());
}
+ @Test
+ public void testSetPolicyWithNullRequest() {
+ Response resp =
+ target(policyPath(metalake))
+ .path("policy1")
+ .property(HttpUrlConnectorProvider.SET_METHOD_WORKAROUND, true)
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .method("PATCH", Entity.entity("null",
MediaType.APPLICATION_JSON_TYPE));
+
+ assertNullRequestBodyRejected(resp);
+ }
+
@Test
public void testDeletePolicy() {
when(policyManager.deletePolicy(metalake, "policy1")).thenReturn(true);
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestStatisticOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestStatisticOperations.java
index a970af8214..da591459de 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestStatisticOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestStatisticOperations.java
@@ -73,14 +73,13 @@ import org.apache.gravitino.stats.StatisticValue;
import org.apache.gravitino.stats.StatisticValues;
import org.glassfish.jersey.internal.inject.AbstractBinder;
import org.glassfish.jersey.server.ResourceConfig;
-import org.glassfish.jersey.test.JerseyTest;
import org.glassfish.jersey.test.TestProperties;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
-public class TestStatisticOperations extends JerseyTest {
+public class TestStatisticOperations extends BaseOperationsTest {
private static class MockServletRequestFactory extends
ServletRequestFactoryBase {
@Override
@@ -392,12 +391,80 @@ public class TestStatisticOperations extends JerseyTest {
.accept("application/vnd.gravitino.v1+json")
.put(entity("null", MediaType.APPLICATION_JSON_TYPE));
- Assertions.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(),
resp.getStatus());
Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE,
resp.getMediaType());
+ assertNullRequestBodyRejected(resp);
+ }
- ErrorResponse errorResp = resp.readEntity(ErrorResponse.class);
- Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE,
errorResp.getCode());
- Assertions.assertEquals(IllegalArgumentException.class.getSimpleName(),
errorResp.getType());
+ @Test
+ public void testDropStatisticsWithNullRequest() {
+ when(tableDispatcher.tableExists(any())).thenReturn(true);
+
+ MetadataObject tableObject =
+ MetadataObjects.parse(
+ String.format("%s.%s.%s", catalog, schema, table),
MetadataObject.Type.TABLE);
+
+ Response resp =
+ target(
+ "/metalakes/"
+ + metalake
+ + "/objects/"
+ + tableObject.type()
+ + "/"
+ + tableObject.fullName()
+ + "/statistics")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .post(entity("null", MediaType.APPLICATION_JSON_TYPE));
+
+ assertNullRequestBodyRejected(resp);
+ }
+
+ @Test
+ public void testUpdatePartitionStatisticsWithNullRequest() {
+ when(tableDispatcher.tableExists(any())).thenReturn(true);
+
+ MetadataObject tableObject =
+ MetadataObjects.parse(
+ String.format("%s.%s.%s", catalog, schema, table),
MetadataObject.Type.TABLE);
+
+ Response resp =
+ target(
+ "/metalakes/"
+ + metalake
+ + "/objects/"
+ + tableObject.type()
+ + "/"
+ + tableObject.fullName()
+ + "/statistics/partitions")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .put(entity("null", MediaType.APPLICATION_JSON_TYPE));
+
+ assertNullRequestBodyRejected(resp);
+ }
+
+ @Test
+ public void testDropPartitionStatisticsWithNullRequest() {
+ when(tableDispatcher.tableExists(any())).thenReturn(true);
+
+ MetadataObject tableObject =
+ MetadataObjects.parse(
+ String.format("%s.%s.%s", catalog, schema, table),
MetadataObject.Type.TABLE);
+
+ Response resp =
+ target(
+ "/metalakes/"
+ + metalake
+ + "/objects/"
+ + tableObject.type()
+ + "/"
+ + tableObject.fullName()
+ + "/statistics/partitions")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .post(entity(new byte[0], MediaType.APPLICATION_JSON_TYPE));
+
+ assertNullRequestBodyRejected(resp);
}
@Test
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestTagOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestTagOperations.java
index 9a9fe3785f..c4672a1114 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestTagOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestTagOperations.java
@@ -1167,6 +1167,22 @@ public class TestTagOperations extends
BaseOperationsTest {
Assertions.assertEquals(RuntimeException.class.getSimpleName(),
errorResponse1.getType());
}
+ @Test
+ public void testAssociateTagsForObjectWithNullRequest() {
+ MetadataObject catalog = MetadataObjects.parse("object1",
MetadataObject.Type.CATALOG);
+
+ // The deprecated route delegates to MetadataObjectTagOperations, so it
inherits the same guard.
+ Response response =
+ target(tagPath(metalake))
+ .path(catalog.type().toString())
+ .path(catalog.fullName())
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .post(Entity.entity("null", MediaType.APPLICATION_JSON_TYPE));
+
+ assertNullRequestBodyRejected(response);
+ }
+
private String tagPath(String metalake) {
return "/metalakes/" + metalake + "/tags";
}
