[#10171] fix(server): Add null request body checks to REST create/register endpoints

[shaojunying]

What changes were proposed in this pull request?

Add null checks for request body parameters before accessing request fields in REST endpoints. Previously, if request deserialization yielded null (e.g., empty body, literal JSON null, or binding edge cases), accessing request.getName() or similar methods before the try block would trigger an uncaught NullPointerException, bypassing the expected ExceptionHandlers error handling path.

Why are the changes needed?

Several REST endpoints dereference request fields (e.g., request.getName(), request.getJobTemplateName()) before entering their try block. This causes uncaught NPEs that bypass the structured error response path, returning unhelpful 500 errors instead of proper 400 Bad Request responses.

Fix #10171

Does this PR introduce any user-facing change?

No API changes. Null request bodies now return a structured 400 Bad Request error response instead of an unhandled 500 Internal Server Error.

How was this patch tested?

Added unit tests for null request body behavior in all matching REST test classes:

• TestTableOperations.testCreateTableWithNullRequestBody
• TestFilesetOperations.testCreateFilesetWithNullRequestBody
• TestFunctionOperations.testRegisterFunctionWithNullRequestBody
• TestModelOperations.testRegisterModelWithNullRequestBody
• TestSchemaOperations.testCreateSchemaWithNullRequestBody
• TestJobOperations.testRegisterJobTemplateWithNullRequestBody
• TestJobOperations.testRunJobWithNullRequestBody

Affected endpoints

File	Method
TableOperations.java	createTable
FilesetOperations.java	createFileset
FunctionOperations.java	registerFunction
ModelOperations.java	registerModel
SchemaOperations.java	createSchema
JobOperations.java	registerJobTemplate
JobOperations.java	runJob

[justinmclean]

Looks good, but a few minor improvements needed:

• Null-body fix in registerJobTemplate is incomplete; it can still trigger an unhandled NPE before try/catch
• PR contains unrelated changes in TestCompactionStrategyHandler.java?
• Minor style inconsistency in tests with FQN used inline

[jerryshao]

Just show me a real case on how to reproduce this issue. AFAIK, this will not happen in the real environment.

[shaojunying]

Thanks for the review @justinmclean! Here's my plan for addressing your feedback:

1. registerJobTemplate NPE in catch block: Good catch — the catch block still dereferences request.getJobTemplate().name(), which can NPE if getJobTemplate() returns null (even after the null request body check). I'll fix this by moving the name extraction inside the try block or adding a safe fallback in the catch block.

2. Unrelated TestCompactionStrategyHandler.java changes: Will remove those from this PR.

3. FQN style inconsistency in tests: I noticed TestSchemaOperations.java uses javax.ws.rs.client.Entity.entity(...) inline instead of the imported Entity. Will fix this to be consistent with the other test files.

I'll push the fixes shortly.

[shaojunying]

Hi @jerryshao, thanks for the question. Here's a concrete reproduction case:

Steps to reproduce:

# Start a Gravitino server, then send a null JSON body to the createTable endpoint:
curl -X POST "http://localhost:8090/api/metalakes/my_metalake/catalogs/my_catalog/schemas/my_schema/tables" \
  -H "Content-Type: application/json" \
  -H "Accept: application/vnd.gravitino.v1+json" \
  -d "null"

When JAX-RS (Jersey) deserializes the literal JSON null, the request parameter becomes null in Java. The current code then does:

LOG.info("Received create table request: {}.{}.{}.{}", metalake, catalog, schema, request.getName());
//                                                                                  ^^^^^^^^^^^^^^^ NPE here
try {

This happens before the try/catch block, so the NPE is not caught by ExceptionHandlers and the client gets an unhelpful HTTP 500 instead of a structured 400 Bad Request.

You're right that normal Gravitino clients won't send null bodies, but this can happen with:

• Direct REST API calls (curl, custom HTTP clients)
• Malformed client implementations
• Security scanning / fuzzing tools

This is essentially a defensive programming improvement to ensure all REST endpoints return proper error responses at system boundaries, consistent with the existing ExceptionHandlers error handling pattern.

[jerryshao]

If this is the case, this will affect all the requests, is there a centralized place that can fix all the related issues, not just do the null check in every place?

I think we need to fix it in the Jackson / Jersey object mapper configuration.

[justinmclean]

Not how we currently have this set up as far as I'm aware. ObjectMapper can enforce many field-level rules, but not null method arguments at JAX-RS entry.