justinmclean opened a new issue, #10172:
URL: https://github.com/apache/gravitino/issues/10172
### What would you like to be improved?
Multiple REST handlers dereference `request` inside `catch (Exception e)`
(for example `request.getName()`, `request.getRoleNames()`,
`request.getUpdates()`, `request.getDrops()`,
`request.getJobTemplate().name()`). When request is `null`, catch-path code can
throw a second `NullPointerException`, masking the real failure.
Representative failure:
- Primary exception occurs in method body.
- Catch block dereferences `request.get*()`.
- Secondary `NullPointerException` in catch hides the original exception
context.
## Scope
- [ ]
`server/src/main/java/org/apache/gravitino/server/web/rest/TableOperations.java`
create catch path
- [ ]
`server/src/main/java/org/apache/gravitino/server/web/rest/FilesetOperations.java`
create catch path
- [ ]
`server/src/main/java/org/apache/gravitino/server/web/rest/FunctionOperations.java`
register catch path
- [ ]
`server/src/main/java/org/apache/gravitino/server/web/rest/ModelOperations.java`
register catch path
- [ ]
`server/src/main/java/org/apache/gravitino/server/web/rest/SchemaOperations.java`
create catch path
- [ ]
`server/src/main/java/org/apache/gravitino/server/web/rest/MetalakeOperations.java`
create catch path
- [ ]
`server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java`
create/testConnection catch paths
- [ ]
`server/src/main/java/org/apache/gravitino/server/web/rest/UserOperations.java`
add catch path
- [ ]
`server/src/main/java/org/apache/gravitino/server/web/rest/GroupOperations.java`
add catch path
- [ ]
`server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.java`
create catch path
- [ ]
`server/src/main/java/org/apache/gravitino/server/web/rest/PolicyOperations.java`
create catch path
- [ ]
`server/src/main/java/org/apache/gravitino/server/web/rest/TagOperations.java`
create catch path
- [ ]
`server/src/main/java/org/apache/gravitino/server/web/rest/TopicOperations.java`
create catch path
- [ ]
`server/src/main/java/org/apache/gravitino/server/web/rest/PermissionOperations.java`
grant/revoke catch paths
- [ ]
`server/src/main/java/org/apache/gravitino/server/web/rest/StatisticOperations.java`
update/drop (+partition update/drop) catch paths
- [ ]
`server/src/main/java/org/apache/gravitino/server/web/rest/JobOperations.java`
register-job-template catch path
### How should we improve?
## Expected behavior
- No `request.get*()` dereference inside affected `catch` blocks.
- Catch blocks should always preserve/marshal the primary error through
existing `ExceptionHandlers`.
- Endpoint APIs and response contracts remain unchanged.
## Definition of done
- [ ] Replace catch-path request dereferences with null-safe precomputed
identifiers or safe fallbacks.
- [ ] Keep existing `ExceptionHandlers` mapping behavior.
- [ ] Keep endpoint APIs unchanged.
- [ ] Add/adjust tests to ensure exception handling path is stable when
request is null.
- [ ] Run and pass: `./gradlew test -PskipITs`
## Example unit test
```java
@Test
public void testAddGroupWithNullRequestBodyDoesNotExposeNpe() {
Response resp =
target("/metalakes/metalake1/groups")
.request(MediaType.APPLICATION_JSON_TYPE)
.accept("application/vnd.gravitino.v1+json")
.post(Entity.entity("null", MediaType.APPLICATION_JSON_TYPE));
Assertions.assertEquals(Status.INTERNAL_SERVER_ERROR.getStatusCode(),
resp.getStatus());
ErrorResponse error = resp.readEntity(ErrorResponse.class);
Assertions.assertNotEquals(NullPointerException.class.getSimpleName(),
error.getType());
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
