This is an automated email from the ASF dual-hosted git repository.
yuqi1129 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 79527d7669 [#10166] fix(server): reject null request bodies in bulk
role and run-job operations (#13670)
79527d7669 is described below
commit 79527d7669f0f25714d11e302c9e8c8a6cda5234
Author: dennismdejong <[email protected]>
AuthorDate: Fri Oct 9 15:10:32 2026 +0200
[#10166] fix(server): reject null request bodies in bulk role and run-job
operations (#13670)
### What changes were proposed in this pull request?
Guard the REST operations that could answer a null request body with
HTTP 500 through an unintended NullPointerException:
- `BulkOperations.addRoles` / `removeRoles`: reject a null body the same
way as the sibling bulk user/group operations (`handleRoleException`,
HTTP 400).
- `JobOperations.runJob`: guard the null body before the log statement
that dereferences the request.
- `CatalogOperations.setCatalog` / `MetalakeOperations.setMetalake`:
behavior-preserving refactor — compute the operation type once and reuse
it in the catch blocks and log statements instead of re-reading the
request. These two already reject null bodies at the top of the handler,
so this is not an additional null-body fix.
Note: the permission endpoints named in the issue (`revokeRolesFromUser`
and siblings) are already guarded on main (handled by
https://github.com/apache/gravitino/pull/12590 /
https://github.com/apache/gravitino/pull/12866), so no change is needed
there; this PR completes the same pattern for the operations that were
still missing it.
### Why are the changes needed?
An omitted, empty, or JSON `null` request body is a client error, but
these endpoints answered it with HTTP 500 through an unintended
NullPointerException (in the handler, or before the `try` block for
`runJob`), masking the original failure.
Caveat on the `runJob` guarantee: the HTTP 400 applies only when the
resource method is reached. With authorization enabled, a null request
is rejected with HTTP 403 by the authorization filter before the handler
(pre-existing behavior, unchanged by this PR).
Fix: https://github.com/apache/gravitino/issues/10166
### Does this PR introduce _any_ user-facing change?
Yes, for `BulkOperations.addRoles`/`removeRoles` a null body now returns
HTTP 400 with error code `1001` (`IllegalArgumentException`, message
"Request body cannot be null") instead of HTTP 500. For
`JobOperations.runJob`, the same HTTP 400 is returned when the resource
method is reached; with authorization enabled the existing filter
rejects a null body with HTTP 403 first (pre-existing). The
`setCatalog`/`setMetalake` changes are behavior-preserving refactors.
Valid requests are unchanged.
### How was this patch tested?
- New regression tests (`testBulkAddRolesWithNullRequest`,
`testBulkRemoveRolesWithNullRequest`, `testRunJobWithNullRequest`) fail
before the change (expected 400, got 500) and pass after.
- `JAVA_HOME=/opt/homebrew/opt/openjdk@17 ./gradlew :server:test
-PskipITs`: 64 test classes, 559 tests, 0 failures.
- `./gradlew :server:spotlessCheck` passes.
---
.../gravitino/server/web/rest/BulkOperations.java | 16 ++++++++++++++++
.../server/web/rest/CatalogOperations.java | 4 ++--
.../gravitino/server/web/rest/JobOperations.java | 8 ++++++++
.../server/web/rest/MetalakeOperations.java | 14 +++++++++-----
.../server/web/rest/TestBulkOperations.java | 22 ++++++++++++++++++++++
.../server/web/rest/TestJobOperations.java | 16 ++++++++++++++++
6 files changed, 73 insertions(+), 7 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 23e0bda28e..a73dd907d2 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
@@ -355,6 +355,14 @@ public class BulkOperations {
@PathParam("metalake") @AuthorizationMetadata(type =
Entity.EntityType.METALAKE)
String metalake,
BulkRoleAddRequest request) {
+ if (request == null) {
+ return ExceptionHandlers.handleRoleException(
+ OperationType.CREATE,
+ "",
+ metalake,
+ new IllegalArgumentException("Request body cannot be null"));
+ }
+
try {
return Utils.doAs(
httpRequest,
@@ -417,6 +425,14 @@ public class BulkOperations {
@PathParam("metalake") @AuthorizationMetadata(type =
Entity.EntityType.METALAKE)
String metalake,
BulkRemoveRequest request) {
+ if (request == null) {
+ return ExceptionHandlers.handleRoleException(
+ OperationType.DELETE,
+ "",
+ 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 09679b7515..d47dc71b30 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
@@ -313,7 +313,7 @@ public class CatalogOperations {
Response response = Utils.ok(new BaseResponse());
LOG.info(
"Successfully {} catalog: {}.{}",
- request.isInUse() ? "enable" : "disable",
+ op == OperationType.ENABLE ? "enable" : "disable",
metalake,
catalogName);
return response;
@@ -322,7 +322,7 @@ public class CatalogOperations {
} catch (Exception e) {
LOG.info(
"Failed to {} catalog: {}.{}",
- request.isInUse() ? "enable" : "disable",
+ op == OperationType.ENABLE ? "enable" : "disable",
metalake,
catalogName);
return ExceptionHandlers.handleCatalogException(op, catalogName,
metalake, e);
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/JobOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/JobOperations.java
index 863e619d4c..4a185ab4f6 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/JobOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/JobOperations.java
@@ -416,6 +416,14 @@ public class JobOperations {
String metalake,
@AuthorizationRequest(type = AuthorizationRequest.RequestType.RUN_JOB)
JobRunRequest request) {
+ if (request == null) {
+ return ExceptionHandlers.handleJobException(
+ OperationType.RUN,
+ "",
+ metalake,
+ new IllegalArgumentException("Request body cannot be null"));
+ }
+
LOG.info(
"Received request to run job {} in metalake: {}",
request.getJobTemplateName(), metalake);
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 5e6dd53b9b..c577bd3298 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
@@ -189,12 +189,14 @@ public class MetalakeOperations {
}
LOG.info("Received set request for metalake: {}", metalakeName);
+ OperationType op = request.isInUse() ? OperationType.ENABLE :
OperationType.DISABLE;
+
try {
return Utils.doAs(
httpRequest,
() -> {
NameIdentifier identifier =
NameIdentifierUtil.ofMetalake(metalakeName);
- if (request.isInUse()) {
+ if (op == OperationType.ENABLE) {
metalakeDispatcher.enableMetalake(identifier);
} else {
metalakeDispatcher.disableMetalake(identifier);
@@ -202,15 +204,17 @@ public class MetalakeOperations {
Response response = Utils.ok(new BaseResponse());
LOG.info(
"Successfully {} metalake: {}",
- request.isInUse() ? "enable" : "disable",
+ op == OperationType.ENABLE ? "enable" : "disable",
metalakeName);
return response;
});
} catch (Exception e) {
- LOG.info("Failed to {} metalake: {}", request.isInUse() ? "enable" :
"disable", metalakeName);
- return ExceptionHandlers.handleMetalakeException(
- request.isInUse() ? OperationType.ENABLE : OperationType.DISABLE,
metalakeName, e);
+ LOG.info(
+ "Failed to {} metalake: {}",
+ op == OperationType.ENABLE ? "enable" : "disable",
+ metalakeName);
+ return ExceptionHandlers.handleMetalakeException(op, metalakeName, e);
}
}
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 30458b689d..718e6ef6e2 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
@@ -401,6 +401,28 @@ public class TestBulkOperations extends BaseOperationsTest
{
assertNullRequestBodyRejected(response);
}
+ @Test
+ public void testBulkAddRolesWithNullRequest() {
+ Response response =
+ target("/bulk/metalakes/metalake1/roles/add")
+ .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 testBulkRemoveRolesWithNullRequest() {
+ Response response =
+ target("/bulk/metalakes/metalake1/roles/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 testBulkRejectsEmptyAndExceededRequest() {
Response emptyResponse =
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestJobOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestJobOperations.java
index 5714ec0d20..da19b642fa 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestJobOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestJobOperations.java
@@ -263,6 +263,22 @@ public class TestJobOperations extends JerseyTest {
Assertions.assertTrue(errorResponse.getMessage().contains("Request body
cannot be null"));
}
+ @Test
+ public void testRunJobWithNullRequest() {
+ Response resp =
+ target(jobRunPath())
+ .request(APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .post(Entity.entity(new byte[0], 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("Request body
cannot be null"));
+ }
+
@Test
public void testRegisterJobTemplate() {
JobTemplateEntity template =
