fix(engine): always answer JSON from the generic action endpoint

.chachalog/Hv2mTn8p.md

@@ -0,0 +1,6 @@
+---
+# Allowed version bumps: patch, minor, major
+javascript-modules: patch
+---
+
+The generic action endpoint (`<page>.jsAction.do`) now answers its JSON envelope whatever the caller's `accept` header, instead of an empty `200` when `application/json` was not requested, and carries a meaningful status: `400` for input rejected by an action's schema, `404` for an unknown action, `500` when the function throws. Calls made through the generated client stubs are unaffected. (#707)

docs/2-guides/7-actions/README.md

@@ -68,6 +68,25 @@ On invalid input the client call rejects with an error carrying the validation `
 - The client stub POSTs to the current page URL (`<page>.jsAction.do`); calls execute with the visitor's session and permissions. **Guests can call your actions**: treat inputs as untrusted and enforce your own permission checks inside the function.
 - Requests carry a mandatory `X-JS-Action` header, which protects the endpoint against classic CSRF (HTML forms cannot set headers; cross-origin scripts would need a CORS preflight that Jahia does not grant).
 
+## Calling an action without the client stub
+
+Tests, server-to-server integrations and `curl` can call the endpoint directly. It always answers a JSON envelope — `{"data": "<devalue>"}` on success, `{"error": "…", "issues": [...]}` on failure — with a status describing the outcome:
+
+| Status | Meaning                                                             |
+| ------ | ------------------------------------------------------------------- |
+| `200`  | the function returned; `data` holds its devalue-encoded result      |
+| `400`  | the input was rejected by the action's schema; `issues` details why |
+| `404`  | no action is registered under that name                             |
+| `500`  | the function threw, or the runtime could not complete the call      |
+
+```sh
+curl -X POST "http://localhost:8080/sites/my-site/home.jsAction.do?name=my-module%2FgetExchangeRate" \
+  -H "X-JS-Action: 1" \
+  --data-binary '[[1],{"currency":2},"EUR"]'
+```
+
+Arguments travel as a [devalue](https://github.com/sveltejs/devalue)-encoded **array** of the function's parameters, and `data` comes back devalue-encoded too — that is what preserves `Date`, `Map`, `Set` and friends across the wire.
+
 ## When to use what
 
 | Need                                                                                   | Use                                                       |

javascript-modules-engine-java/src/main/java/org/jahia/modules/javascript/modules/engine/actions/GenericActionEndpoint.java

@@ -45,8 +45,15 @@
  *
  * <p>Exposed as the platform action {@code jsAction}: client stubs POST a devalue-serialized arguments
  * array to {@code <pageUrl>.jsAction.do?name=<module>/<export>} and receive an envelope
- * {@code {"data": "<devalue>"}} or {@code {"error": "...", "issues": [...]?}}. The envelope always
- * travels on HTTP 200 because the render servlet only writes JSON bodies for 2xx action results.
+ * {@code {"data": "<devalue>"}} or {@code {"error": "...", "issues": [...]?}}, with a status code
+ * describing the outcome (200, 400, 404 or 500).
+ *
+ * <p>The envelope is written straight to the response rather than returned as an {@link ActionResult}:
+ * {@code Render#doAction} only serializes an action's JSON when the caller asked for it (a
+ * {@code returnContentType=json} parameter or an {@code accept} header containing
+ * {@code application/json}), and silently produces an empty body otherwise. This endpoint has no
+ * other representation to offer, so it always answers JSON; returning {@code null} tells the render
+ * servlet the response is already complete.
  *
  * <p>The {@code X-JS-Action} header is required: HTML forms cannot set custom headers and cross-origin
  * scripts would need a CORS preflight that Jahia does not grant, so the endpoint is not exploitable as
@@ -83,44 +90,76 @@ public void activate() {
     public ActionResult doExecute(HttpServletRequest request, RenderContext renderContext, Resource resource,
             JCRSessionWrapper session, Map<String, List<String>> parameters, URLResolver urlResolver)
             throws Exception {
+        HttpServletResponse response = renderContext.getResponse();
+
         if (request.getHeader(REQUIRED_HEADER) == null) {
-            return error("Missing " + REQUIRED_HEADER + " header");
+            logger.warn("Rejecting a call to the JS action endpoint without the {} header", REQUIRED_HEADER);
+            return error(response, HttpServletResponse.SC_BAD_REQUEST, "Missing " + REQUIRED_HEADER + " header");
         }
         String name = getParameter(parameters, "name");
         if (name == null) {
-            return error("Missing action name");
+            logger.warn("Rejecting a call to the JS action endpoint without an action name");
+            return error(response, HttpServletResponse.SC_BAD_REQUEST, "Missing action name");
         }
         String body = IOUtils.toString(request.getReader());
 
         return graalVMEngine.doWithContext(contextProvider -> {
             Map<String, Object> entry = contextProvider.getRegistry().get(REGISTRY_TYPE, name);
             if (entry == null || entry.get("execute") == null) {
-                return error("Unknown action: " + name);
+                logger.warn("Rejecting a call to unknown JS action '{}'", name);
+                return error(response, HttpServletResponse.SC_NOT_FOUND, "Unknown action: " + name);
             }
             JSPromise.Outcome outcome;
             try {
                 outcome = JSPromise.settle(Value.asValue(entry.get("execute")).execute(body));
             } catch (Exception e) {
                 logger.error("JS action '{}' failed to execute", name, e);
-                return error("Action execution failed");
+                return error(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, "Action execution failed");
             }
             if (!outcome.isSettled()) {
                 logger.error("JS action '{}' returned a promise that did not settle; only microtask-based " +
                         "asynchronicity is supported on the server (no timers or async I/O)", name);
-                return error("Action did not settle synchronously");
+                return error(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
+                        "Action did not settle synchronously");
             }
             if (outcome.isRejected()) {
-                return error(readMessage(outcome.getError()), readIssues(outcome.getError()));
+                JSONArray issues = readIssues(outcome.getError());
+                if (issues != null) {
+                    // Input rejected by the action's schema: the caller sent something invalid
+                    return error(response, HttpServletResponse.SC_BAD_REQUEST, readMessage(outcome.getError()),
+                            issues);
+                }
+                // Anything else the function threw is a server-side failure, as in any other request
+                logger.error("JS action '{}' threw: {}", name, readMessage(outcome.getError()));
+                return error(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
+                        readMessage(outcome.getError()));
             }
             Value data = outcome.getValue();
             if (data == null || data.isNull() || !data.isString()) {
                 logger.error("JS action '{}' adapter did not return a serialized string", name);
-                return error("Action returned an unexpected result");
+                return error(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
+                        "Action returned an unexpected result");
             }
-            return new ActionResult(HttpServletResponse.SC_OK, null, new JSONObject().put("data", data.asString()));
+            return respond(response, HttpServletResponse.SC_OK, new JSONObject().put("data", data.asString()));
         });
     }
 
+    /**
+     * Writes the envelope to the response and returns {@code null}, which tells {@code Render#doAction}
+     * that the response is complete — see this class' javadoc for why the render servlet cannot do it.
+     */
+    private static ActionResult respond(HttpServletResponse response, int status, JSONObject json) {
+        response.setStatus(status);
+        response.setContentType("application/json; charset=UTF-8");
+        try {
+            json.write(response.getWriter());
+            response.getWriter().flush();
+        } catch (Exception e) {
+            logger.error("Failed to write the JS action response", e);
+        }
+        return null;
+    }
+
     private static String readMessage(Value errorValue) {
         if (errorValue != null && errorValue.hasMembers() && errorValue.hasMember("message")) {
             Value message = errorValue.getMember("message");
@@ -146,15 +185,15 @@ private static JSONArray readIssues(Value errorValue) {
         return null;
     }
 
-    private static ActionResult error(String message) {
-        return error(message, null);
+    private static ActionResult error(HttpServletResponse response, int status, String message) {
+        return error(response, status, message, null);
     }
 
-    private static ActionResult error(String message, JSONArray issues) {
+    private static ActionResult error(HttpServletResponse response, int status, String message, JSONArray issues) {
         JSONObject json = new JSONObject().put("error", message);
         if (issues != null) {
             json.put("issues", issues);
         }
-        return new ActionResult(HttpServletResponse.SC_OK, null, json);
+        return respond(response, status, json);
     }
 }

tests/cypress/e2e/ui/genericActionTest.cy.ts

@@ -11,12 +11,12 @@ const MODULE = "javascript-modules-engine-test-module";
 const callAction = (
   name: string,
   args: unknown[],
-  headers: Record<string, string> = { "X-JS-Action": "1" },
+  headers: Record<string, string> = { "X-JS-Action": "1", "accept": "application/json" },
 ) =>
   cy.request({
     method: "POST",
     url: `/cms/render/default/en${pagePath}.jsAction.do?name=${encodeURIComponent(name)}`,
-    headers: { accept: "application/json", ...headers },
+    headers,
     body: stringify(args),
     failOnStatusCode: false,
   });
@@ -70,30 +70,44 @@ describe("Actions (.action.ts files)", () => {
     });
   });
 
-  it("surfaces thrown errors", () => {
+  it("answers the envelope even when the caller does not ask for JSON", () => {
+    // The endpoint has no other representation to offer, so it must not depend on the accept header
+    callAction(`${MODULE}/add`, [20, 22], { "X-JS-Action": "1" }).then((response) => {
+      expect(response.status).to.eq(200);
+      expect(response.headers["content-type"]).to.contain("application/json");
+      expect(parse(response.body.data)).to.eq(42);
+    });
+  });
+
+  it("surfaces thrown errors as a server error", () => {
     callAction(`${MODULE}/failOnPurpose`, []).then((response) => {
+      expect(response.status).to.eq(500);
       expect(response.body.error).to.contain("Intentional failure");
     });
   });
 
   it("rejects invalid input of safe actions with validation issues", () => {
     callAction(`${MODULE}/safeDouble`, [{ n: -1 }]).then((response) => {
+      expect(response.status).to.eq(400);
       expect(response.body.error).to.contain("n must be a positive number");
       expect(response.body.issues[0].message).to.eq("n must be a positive number");
     });
     callAction(`${MODULE}/safeDouble`, [{ n: 21 }]).then((response) => {
+      expect(response.status).to.eq(200);
       expect(parse(response.body.data)).to.eq(42);
     });
   });
 
   it("requires the X-JS-Action header (CSRF hardening)", () => {
     callAction(`${MODULE}/add`, [1, 2], {}).then((response) => {
+      expect(response.status).to.eq(400);
       expect(response.body.error).to.contain("X-JS-Action");
     });
   });
 
   it("reports unknown actions", () => {
     callAction(`${MODULE}/doesNotExist`, []).then((response) => {
+      expect(response.status).to.eq(404);
       expect(response.body.error).to.contain("Unknown action");
     });
   });