apache / apache/gravitino

[Improvement] Possible NPE in revokeRolesFromUser error handling when request body is null

Open
#10,166 4 comments 0 reactions 1 assignee Claimed by @pithecuse527 View on GitHub
good first issue improvement
Dominant language
Java
Stars
3.2k
Forks
935
Avg merge
1d 16h
Merged PRs (30d)
298

Description

### What would you like to be improved?

PermissionOperations.revokeRolesFromUser can throw an unintended NullPointerException in its catch block. When the RoleRevokeRequest request is null (for example, due to an empty/invalid request body), the handler calls request.getRoleNames() while building the error response. This masks the original failure path and returns an unhelpful internal error.

### How should we improve?

Make catch-block argument construction null-safe before calling ExceptionHandlers:
- Build role-name string with a null guard, e.g. empty string when request == null (or when role names are null).
- Reuse that safe value in handleUserPermissionOperationException(...).
- If needed, apply the same pattern to similar endpoints (grant/revoke for user/group) to avoid equivalent NPEs.

Here's a unit test to help:
```
@Test
public void testRevokeRolesFromUserWithNullRequest() throws Exception {
PermissionOperations operations = new PermissionOperations();
HttpServletRequest request = mock(HttpServletRequest.class);
when(request.getAttribute(any())).thenReturn(null);
FieldUtils.writeField(operations, "httpRequest", request, true);

Response response = operations.revokeRolesFromUser("metalake1", "user1", null);
Assertions.assertEquals(
Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), response.getStatus());
}
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.