This is an automated email from the ASF dual-hosted git repository.
jongyoul pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git
The following commit(s) were added to refs/heads/master by this push:
new 07cf670875 [ZEPPELIN-6012] Fix NPE when the run-note request body
carries no params
07cf670875 is described below
commit 07cf67087516e16c3d1c6e31716c956f6654f33e
Author: dae won <[email protected]>
AuthorDate: Thu Aug 6 16:57:06 2026 +0900
[ZEPPELIN-6012] Fix NPE when the run-note request body carries no params
### What is this PR for?
`POST /api/notebook/job/{noteId}` accepts an optional request body carrying
form parameters. Sending a body that supplies no parameters, either `{}` or
`{"params":null}`, returns HTTP 500.
`ParametersRequest` declares its `params` field as `final` and assigns it
in the constructor, but Gson never invokes that constructor. It allocates the
instance and fills the fields reflectively, so a body without a `"params"`
entry leaves the field at its default value of `null`.
`NotebookRestApi.runNoteJobs` then hands that `null` straight to
`HashMap.putAll`:
```java
Map<String, Object> params = new HashMap<>();
if (!StringUtils.isEmpty(message)) {
ParametersRequest request = GSON.fromJson(message,
ParametersRequest.class);
params.putAll(request.getParams());
}
```
`{}` is not an empty string, so the guard passes and the call throws:
```
java.lang.NullPointerException: Cannot invoke "java.util.Map.size()"
because "m" is null
at java.util.HashMap.putMapEntries(HashMap.java:495)
at java.util.HashMap.putAll(HashMap.java:783)
at
org.apache.zeppelin.rest.NotebookRestApi.runNoteJobs(NotebookRestApi.java:850)
```
Nothing catches it, so `WebApplicationExceptionMapper` turns it into a
generic `Internal server error` with status 500. Running a note without form
parameters is a legitimate request, and an empty body already works, so both
spellings should behave the same.
This PR makes `ParametersRequest.getParams()` return an empty map instead
of `null`, which covers both an absent key and an explicit null value.
Scope note: two other call sites parse the same request object, at
`NotebookRestApi` lines 979 and 1018. Both assign the result to a local
variable rather than calling `putAll`, so they do not throw, and their
consumers already guard against null (`Note.runAllSync` and
`NotebookService.runParagraph` each check `params != null &&
!params.isEmpty()`). Fixing the accessor covers all three call sites without
changing their behavior.
### What type of PR is it?
Bug Fix
### Todos
* [x] - Return an empty map from `ParametersRequest.getParams()` when no
parameters were supplied
* [x] - Add a regression test covering both `{}` and `{"params":null}`
* [x] - Confirm the test fails without the fix and passes with it
### What is the Jira issue?
* [ZEPPELIN-6012](https://issues.apache.org/jira/browse/ZEPPELIN-6012)
### How should this be tested?
New test `NotebookRestApiTest#testRunNoteWithoutParamsInBody` creates a
note and posts both bodies to the run-note endpoint, asserting that each
returns status `OK`.
```bash
./mvnw package -pl zeppelin-server --am \
-Dtest='NotebookRestApiTest#testRunNoteWithoutParamsInBody'
-DfailIfNoTests=false
```
Reverting only the production change makes the new test fail with
`Expected: HTTP response <200> but: got <500>`, and the server log shows the
stack trace above. With the fix it passes.
Also verified by hand against a locally running server, posting each body
to `/api/notebook/job/{noteId}`:
| Request body | Before | After |
|---|---|---|
| `{}` | HTTP 500 | HTTP 200 |
| `{"params":null}` | HTTP 500 | HTTP 200 |
| empty body | HTTP 200 | HTTP 200 |
| `{"params":{"name":"zeppelin"}}` | HTTP 200 | HTTP 200 |
### Screenshots (if appropriate)
N/A
### Questions:
* Does the license files need to update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No
Closes #5385 from big-cir/ZEPPELIN-6012.
Signed-off-by: Jongyoul Lee <[email protected]>
---
.../zeppelin/rest/message/ParametersRequest.java | 8 ++++++-
.../apache/zeppelin/rest/NotebookRestApiTest.java | 27 ++++++++++++++++++++++
2 files changed, 34 insertions(+), 1 deletion(-)
diff --git
a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/message/ParametersRequest.java
b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/message/ParametersRequest.java
index 04e19a3772..828c36c474 100644
---
a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/message/ParametersRequest.java
+++
b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/message/ParametersRequest.java
@@ -16,6 +16,7 @@
*/
package org.apache.zeppelin.rest.message;
+import java.util.Collections;
import java.util.Map;
/**
@@ -29,7 +30,12 @@ public class ParametersRequest {
this.params = params;
}
+ /**
+ * Gson bypasses the constructor, so this field is null when the body
carries no "params" entry.
+ *
+ * @return the parameters, or an empty map when none were supplied
+ */
public Map<String, Object> getParams() {
- return params;
+ return params == null ? Collections.emptyMap() : params;
}
}
diff --git
a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookRestApiTest.java
b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookRestApiTest.java
index c93cc610d5..0257f8a69e 100644
---
a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookRestApiTest.java
+++
b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookRestApiTest.java
@@ -890,6 +890,33 @@ class NotebookRestApiTest extends AbstractTestRestApi {
}
}
+ @Test
+ void testRunNoteWithoutParamsInBody() throws IOException {
+ LOGGER.info("Running testRunNoteWithoutParamsInBody");
+ String note1Id = null;
+ try {
+ note1Id = notebook.createNote("note1", anonymous);
+
+ // Running a note without form parameters is valid. Gson leaves
ParametersRequest#params
+ // null both when the key is absent and when it is an explicit null, so
neither body may fail.
+ for (String body : new String[] {"{}", "{\"params\":null}"}) {
+ CloseableHttpResponse post =
+ httpPost("/notebook/job/" + note1Id +
"?blocking=true&isolated=true", body);
+ assertThat(post, isAllowed());
+ Map<String, Object> resp = gson.fromJson(
+ EntityUtils.toString(post.getEntity(), StandardCharsets.UTF_8),
+ new TypeToken<Map<String, Object>>() {}.getType());
+ assertEquals("OK", resp.get("status"), "Failed for request body: " +
body);
+ post.close();
+ }
+ } finally {
+ // cleanup
+ if (null != note1Id) {
+ notebook.removeNote(note1Id, anonymous);
+ }
+ }
+ }
+
@Test
void testRunAllParagraph_FirstFailed() throws IOException {
LOGGER.info("Running testRunAllParagraph_FirstFailed");
