Skip to content

[#10164] fix(server): prevent NPE in createSchema when request is null#10187

Open
Sarojini-T wants to merge 1 commit intoapache:mainfrom
Sarojini-T:fix/issue-schema-operations-npe
Open

[#10164] fix(server): prevent NPE in createSchema when request is null#10187
Sarojini-T wants to merge 1 commit intoapache:mainfrom
Sarojini-T:fix/issue-schema-operations-npe

Conversation

@Sarojini-T
Copy link

What changes were proposed in this pull request?

  • Moved the logging of request.getName() inside the try block to avoid accessing a null object
  • Added a guard clause to fail fast with an IllegalArgumentException if the request body is null
  • Ensured the exception handler in the catch block is null-safe
  • Added a unit test testCreateSchemaWithNullRequestShouldNotThrow to verify the fix

Why are the changes needed?

Sending a null request body to createSchema causes a raw NullPointerException before the try-catch block, resulting in an inconsistent 500 error instead of a proper validation error.

Fix: #10164

Does this PR introduce any user-facing change?

No user-facing changes were introduced.

How was this patch tested?

  1. Added a new JUnit test case in TestSchemaOperations.java: testCreateSchemaWithNullRequestShouldNotThrow
  2. Ran ./gradlew :server:test --tests "org.apache.gravitino.server.web.rest.TestSchemaOperations" and verified all tests passed

@jerryshao
Copy link
Contributor

jerryshao commented Mar 4, 2026

This is not a valid case, it will not happen in a normal case. Besides, all of the REST operations don't check the nullability of the incoming request in the processing logic (it will be handled by Jersey/Jackson); if this is the case, all should be fixed.

Kangs-Claw pushed a commit to zhoukangcn/gravitino that referenced this pull request Mar 4, 2026
Addresses review feedback from jerryshao
@Kangs-Claw
Copy link

Addressed feedback from @jerryshao:

  • Removed null check for request body in createSchema and createMetalake methods, as Jersey/Jackson handles nullability.
  • Moved logging statement before try block to avoid NPE.
  • Kept null-safe catch block for defensive programming.
  • Removed unit tests for null request scenarios as they are not valid cases.

Updated in commit b5223a3

@justinmclean
Copy link
Member

justinmclean commented Mar 4, 2026

@jerryshao I verified this behavior in the current codebase, and null request bodies are actually reachable in REST handlers.

TestMetalakeOperations#testCreateMetalakeWithNullRequest sends Entity.entity(null, MediaType.APPLICATION_JSON_TYPE) and passes, which means Jersey/Jackson does not universally reject this before resource logic. Without the guard in createSchema, we hit an NPE path (request.getName()).

So this case is valid.

But to double-check here, a higher-level test confirming the issue.

@Test
public void testCreateSchemaWithNullRequestBodyViaHttpServer() throws Exception {
  gravitinoServer.initialize();
  gravitinoServer.start();

  HttpURLConnection connection =
      (HttpURLConnection)
          new URL("http://127.0.0.1:" + httpPort + "/api/metalakes/m/catalogs/c/schemas")
              .openConnection();
  connection.setRequestMethod("POST");
  connection.setRequestProperty("Content-Type", "application/json");
  connection.setRequestProperty("Accept", "application/vnd.gravitino.v1+json");
  connection.setDoOutput(true);
  connection.getOutputStream().close();

  assertEquals(500, connection.getResponseCode());
}

The call is basically:

POST /api/metalakes/m/catalogs/c/schemas
Content-Type: application/json
Accept: application/vnd.gravitino.v1+json
Body: <empty>

As you can see from the test that returns an HTTP status of 500 when it should return 400 Bad Request with an ErrorResponse with code 1001 (ILLEGAL_ARGUMENTS_CODE) and type "IllegalArgumentException".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Improvement] Possible NPE in SchemaOperations#createSchema

4 participants