From 4fd7d965f0a3f2fc53093450656bb44aee71964a Mon Sep 17 00:00:00 2001 From: Romain Gauthier Date: Fri, 24 Jul 2026 21:41:45 +0200 Subject: [PATCH 1/5] fix(engine): always answer JSON from the generic action endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Render#doAction` only serializes an action's JSON when the caller asked for it — a `returnContentType=json` parameter, or an `accept` header containing `application/json` — and otherwise falls through to its redirect branch, which for a JSON-only action means an empty 200 with nothing logged. The generated client stubs always send the header, so this was invisible from the browser but hit every hand-rolled caller: curl, tests, server-to-server integrations, all silently getting a blank body, including on error paths. The endpoint now writes its envelope to the response itself and returns null, which tells the render servlet the response is complete. Freed from the 2xx-only constraint of that code path, failures also get meaningful statuses: 400 when an action's schema rejects the input (the envelope keeps its `issues`), 404 for an unknown action name, 500 when the function throws or the runtime cannot complete the call. Malformed calls are logged, which they were not before. The generated stub now reads the envelope before looking at the status, so a validation failure still rejects with the server's message and issues rather than a bare "failed with HTTP 400". Verified on a Jahia 8.2 instance, with and without the accept header, across success, schema rejection, thrown error, unknown action and missing-header cases; and from a browser island through the generated stubs. Closes #707 --- .chachalog/Hv2mTn8p.md | 6 ++ docs/2-guides/7-actions/README.md | 19 ++++++ .../engine/actions/GenericActionEndpoint.java | 67 +++++++++++++++---- tests/cypress/e2e/ui/genericActionTest.cy.ts | 20 +++++- .../expected/client/bar.client.tsx.js | 2 +- vite-plugin/src/actions.ts | 10 ++- 6 files changed, 103 insertions(+), 21 deletions(-) create mode 100644 .chachalog/Hv2mTn8p.md diff --git a/.chachalog/Hv2mTn8p.md b/.chachalog/Hv2mTn8p.md new file mode 100644 index 00000000..f1116711 --- /dev/null +++ b/.chachalog/Hv2mTn8p.md @@ -0,0 +1,6 @@ +--- +# Allowed version bumps: patch, minor, major +javascript-modules: patch +--- + +The generic action endpoint (`.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) diff --git a/docs/2-guides/7-actions/README.md b/docs/2-guides/7-actions/README.md index c7d5518a..6f9e9e41 100644 --- a/docs/2-guides/7-actions/README.md +++ b/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 (`.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": ""}` 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 | diff --git a/javascript-modules-engine-java/src/main/java/org/jahia/modules/javascript/modules/engine/actions/GenericActionEndpoint.java b/javascript-modules-engine-java/src/main/java/org/jahia/modules/javascript/modules/engine/actions/GenericActionEndpoint.java index d2b7c667..d92962e0 100644 --- a/javascript-modules-engine-java/src/main/java/org/jahia/modules/javascript/modules/engine/actions/GenericActionEndpoint.java +++ b/javascript-modules-engine-java/src/main/java/org/jahia/modules/javascript/modules/engine/actions/GenericActionEndpoint.java @@ -45,8 +45,15 @@ * *

Exposed as the platform action {@code jsAction}: client stubs POST a devalue-serialized arguments * array to {@code .jsAction.do?name=/} and receive an envelope - * {@code {"data": ""}} 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": ""}} or {@code {"error": "...", "issues": [...]?}}, with a status code + * describing the outcome (200, 400, 404 or 500). + * + *

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. * *

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> 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 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); } } diff --git a/tests/cypress/e2e/ui/genericActionTest.cy.ts b/tests/cypress/e2e/ui/genericActionTest.cy.ts index 1eb47e6b..878cb61e 100644 --- a/tests/cypress/e2e/ui/genericActionTest.cy.ts +++ b/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 = { "X-JS-Action": "1" }, + headers: Record = { "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"); }); }); diff --git a/vite-plugin/fixtures/expected/client/bar.client.tsx.js b/vite-plugin/fixtures/expected/client/bar.client.tsx.js index 29367e04..4e2d97df 100644 --- a/vite-plugin/fixtures/expected/client/bar.client.tsx.js +++ b/vite-plugin/fixtures/expected/client/bar.client.tsx.js @@ -1,2 +1,2 @@ import{useState as e}from"react";import{jsx as t}from"react/jsx-runtime";var n=2**32-1,r=n-1,i=class extends Error{constructor(e,t,n,r){super(e),this.name=`DevalueError`,this.path=t.join(``),this.value=n,this.root=r}};function a(e){return e===null||typeof e!=`object`&&typeof e!=`function`}var o=Object.getOwnPropertyNames(Object.prototype).sort().join(`\0`);function s(e){let t=Object.getPrototypeOf(e);return t===Object.prototype||t===null||Object.getPrototypeOf(t)===null||Object.getOwnPropertyNames(t).sort().join(`\0`)===o}function c(e){return Object.prototype.toString.call(e).slice(8,-1)}function l(e){switch(e){case`"`:return`\\"`;case`<`:return`\\u003C`;case`\\`:return`\\\\`;case` -`:return`\\n`;case`\r`:return`\\r`;case` `:return`\\t`;case`\b`:return`\\b`;case`\f`:return`\\f`;case`\u2028`:return`\\u2028`;case`\u2029`:return`\\u2029`;default:return e<` `?`\\u${e.charCodeAt(0).toString(16).padStart(4,`0`)}`:``}}function u(e){let t=``,n=0,r=e.length;for(let i=0;iObject.getOwnPropertyDescriptor(e,t).enumerable)}var f=/^[a-zA-Z_$][a-zA-Z_$0-9]*$/;function p(e){return f.test(e)?`.`+e:`[`+JSON.stringify(e)+`]`}function m(e){return!(!Number.isInteger(e)||e<0||e>r)}function h(e){return!(!Number.isInteger(e)||e<0||e>n)}function g(e){if(e.length===0||e.length>1&&e.charCodeAt(0)===48)return!1;for(let t=0;t57)return!1}return m(+e)}function _(e){let t=Object.keys(e);for(var n=t.length-1;n>=0&&!g(t[n]);n--);return t.length=n+1,t}function v(e){return new Uint8Array(e).toBase64()}function y(e){return Uint8Array.fromBase64(e).buffer}function b(e){return Buffer.from(e).toString(`base64`)}function x(e){return Uint8Array.from(Buffer.from(e,`base64`)).buffer}function S(e){let t=new Uint8Array(e),n=``,r=32768;for(let e=0;e=t)throw Error(`Invalid input`);n[r]=o(c[e+1])}n.length=t}else{let t=Array(c.length);i[e]=t;for(let e=0;e{let t=h(e,g);t<0&&(r[g]=t)})}else{let e=c(n);switch(e){case`Number`:case`String`:case`Boolean`:case`BigInt`:v=`["Object",${h(n.valueOf())}]`;break;case`Date`:v=`["Date","${isNaN(n.getDate())?``:n.toISOString()}"]`;break;case`URL`:v=`["URL",${u(n.toString())}]`;break;case`URLSearchParams`:v=`["URLSearchParams",${u(n.toString())}]`;break;case`RegExp`:let{source:r,flags:o}=n;v=o?`["RegExp",${u(r)},"${o}"]`:`["RegExp",${u(r)}]`;break;case`Array`:{let e=!1;v=`[`;for(let t=0;t0&&(v+=`,`),Object.hasOwn(n,t))f.push(`[${t}]`),v+=h(n[t]),f.pop();else if(e)v+=-2;else{let t=_(n),r=t.length,i=String(n.length).length;if((n.length-r)*3>4+i+r*(i+1)){v=`[-7,`+n.length;for(let e=0;e0)throw new i(`Cannot stringify POJOs with symbolic keys`,f,n,t);if(Object.getPrototypeOf(n)===null){v=`["null"`;for(let e of Object.keys(n)){if(e===`__proto__`)throw new i(`Cannot stringify objects with __proto__ keys`,f,n,t);f.push(p(e)),v+=`,${u(e)},${h(n[e])}`,f.pop()}v+=`]`}else{v=`{`;let e=!1;for(let r of Object.keys(n)){if(r===`__proto__`)throw new i(`Cannot stringify objects with __proto__ keys`,f,n,t);e&&(v+=`,`),e=!0,f.push(p(r)),v+=`${u(r)}:${h(n[r])}`,f.pop()}v+=`}`}}}return r[g]=v,g}let g=h(t);return g<0?`${g}`:r}function M(e){let t=typeof e;return t===`string`?u(e):e===void 0?`-1`:e===0&&1/e<0?`-6`:t===`bigint`?`["BigInt","${e}"]`:String(e)}var N=e=>async(...t)=>{let n=`${location.pathname.replace(/\.html$/,``)}.jsAction.do?name=${encodeURIComponent(e)}`,r=await fetch(n,{method:`POST`,headers:{"X-JS-Action":`1`,accept:`application/json`},body:A(t)});if(!r.ok)throw Error(`Action ${e} failed with HTTP ${r.status}`);let i=await r.json();if(i.error){let e=Error(i.error);throw i.issues&&(e.issues=i.issues),e}return O(i.data)},P=N(`@jahia/vite-plugin-fixtures/greet`),F=N(`@jahia/vite-plugin-fixtures/sum`);function I(){let[n,r]=e(``);return t(`button`,{onClick:async()=>{r(`${await P(`world`)} (${await F(1,2,3)})`)},children:n||`Say hello`})}export{I as default}; \ No newline at end of file +`:return`\\n`;case`\r`:return`\\r`;case` `:return`\\t`;case`\b`:return`\\b`;case`\f`:return`\\f`;case`\u2028`:return`\\u2028`;case`\u2029`:return`\\u2029`;default:return e<` `?`\\u${e.charCodeAt(0).toString(16).padStart(4,`0`)}`:``}}function u(e){let t=``,n=0,r=e.length;for(let i=0;iObject.getOwnPropertyDescriptor(e,t).enumerable)}var f=/^[a-zA-Z_$][a-zA-Z_$0-9]*$/;function p(e){return f.test(e)?`.`+e:`[`+JSON.stringify(e)+`]`}function m(e){return!(!Number.isInteger(e)||e<0||e>r)}function h(e){return!(!Number.isInteger(e)||e<0||e>n)}function g(e){if(e.length===0||e.length>1&&e.charCodeAt(0)===48)return!1;for(let t=0;t57)return!1}return m(+e)}function _(e){let t=Object.keys(e);for(var n=t.length-1;n>=0&&!g(t[n]);n--);return t.length=n+1,t}function v(e){return new Uint8Array(e).toBase64()}function y(e){return Uint8Array.fromBase64(e).buffer}function b(e){return Buffer.from(e).toString(`base64`)}function x(e){return Uint8Array.from(Buffer.from(e,`base64`)).buffer}function S(e){let t=new Uint8Array(e),n=``,r=32768;for(let e=0;e=t)throw Error(`Invalid input`);n[r]=o(c[e+1])}n.length=t}else{let t=Array(c.length);i[e]=t;for(let e=0;e{let t=h(e,g);t<0&&(r[g]=t)})}else{let e=c(n);switch(e){case`Number`:case`String`:case`Boolean`:case`BigInt`:v=`["Object",${h(n.valueOf())}]`;break;case`Date`:v=`["Date","${isNaN(n.getDate())?``:n.toISOString()}"]`;break;case`URL`:v=`["URL",${u(n.toString())}]`;break;case`URLSearchParams`:v=`["URLSearchParams",${u(n.toString())}]`;break;case`RegExp`:let{source:r,flags:o}=n;v=o?`["RegExp",${u(r)},"${o}"]`:`["RegExp",${u(r)}]`;break;case`Array`:{let e=!1;v=`[`;for(let t=0;t0&&(v+=`,`),Object.hasOwn(n,t))f.push(`[${t}]`),v+=h(n[t]),f.pop();else if(e)v+=-2;else{let t=_(n),r=t.length,i=String(n.length).length;if((n.length-r)*3>4+i+r*(i+1)){v=`[-7,`+n.length;for(let e=0;e0)throw new i(`Cannot stringify POJOs with symbolic keys`,f,n,t);if(Object.getPrototypeOf(n)===null){v=`["null"`;for(let e of Object.keys(n)){if(e===`__proto__`)throw new i(`Cannot stringify objects with __proto__ keys`,f,n,t);f.push(p(e)),v+=`,${u(e)},${h(n[e])}`,f.pop()}v+=`]`}else{v=`{`;let e=!1;for(let r of Object.keys(n)){if(r===`__proto__`)throw new i(`Cannot stringify objects with __proto__ keys`,f,n,t);e&&(v+=`,`),e=!0,f.push(p(r)),v+=`${u(r)}:${h(n[r])}`,f.pop()}v+=`}`}}}return r[g]=v,g}let g=h(t);return g<0?`${g}`:r}function M(e){let t=typeof e;return t===`string`?u(e):e===void 0?`-1`:e===0&&1/e<0?`-6`:t===`bigint`?`["BigInt","${e}"]`:String(e)}var N=e=>async(...t)=>{let n=`${location.pathname.replace(/\.html$/,``)}.jsAction.do?name=${encodeURIComponent(e)}`,r=await fetch(n,{method:`POST`,headers:{"X-JS-Action":`1`,accept:`application/json`},body:A(t)}),i=await r.json().catch(()=>null);if(i?.error){let e=Error(i.error);throw i.issues&&(e.issues=i.issues),e}if(!r.ok||!i)throw Error(`Action ${e} failed with HTTP ${r.status}`);return O(i.data)},P=N(`@jahia/vite-plugin-fixtures/greet`),F=N(`@jahia/vite-plugin-fixtures/sum`);function I(){let[n,r]=e(``);return t(`button`,{onClick:async()=>{r(`${await P(`world`)} (${await F(1,2,3)})`)},children:n||`Say hello`})}export{I as default}; \ No newline at end of file diff --git a/vite-plugin/src/actions.ts b/vite-plugin/src/actions.ts index caf78283..9b0607c3 100644 --- a/vite-plugin/src/actions.ts +++ b/vite-plugin/src/actions.ts @@ -84,13 +84,17 @@ const __jsmCall = (name) => async (...args) => { headers: { "X-JS-Action": "1", accept: "application/json" }, body: __jsmStringify(args), }); - if (!response.ok) throw new Error(\`Action \${name} failed with HTTP \${response.status}\`); - const payload = await response.json(); - if (payload.error) { + // The endpoint answers with a JSON envelope whatever the outcome, so the body is read before the + // status is considered: failures carry their message — and their validation issues — in there + const payload = await response.json().catch(() => null); + if (payload?.error) { const error = new Error(payload.error); if (payload.issues) error.issues = payload.issues; throw error; } + if (!response.ok || !payload) { + throw new Error(\`Action \${name} failed with HTTP \${response.status}\`); + } return __jsmParse(payload.data); }; From ba9272c76d8eaa1268372394b8b3f799d722bc45 Mon Sep 17 00:00:00 2001 From: Romain Gauthier Date: Fri, 14 Aug 2026 14:38:04 +0200 Subject: [PATCH 2/5] fix(engine): answer 400 for malformed action request bodies An unparsable body was classified as a server failure (500 + ERROR log) though it is a caller mistake. The adapter flags parse failures as badRequest, and the endpoint maps flagged rejections to 400 with a warn. --- .../engine/actions/GenericActionEndpoint.java | 14 ++++++++++++++ .../framework/actions/registerActionsModule.ts | 18 ++++++++++++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/javascript-modules-engine-java/src/main/java/org/jahia/modules/javascript/modules/engine/actions/GenericActionEndpoint.java b/javascript-modules-engine-java/src/main/java/org/jahia/modules/javascript/modules/engine/actions/GenericActionEndpoint.java index d92962e0..7911f40f 100644 --- a/javascript-modules-engine-java/src/main/java/org/jahia/modules/javascript/modules/engine/actions/GenericActionEndpoint.java +++ b/javascript-modules-engine-java/src/main/java/org/jahia/modules/javascript/modules/engine/actions/GenericActionEndpoint.java @@ -129,6 +129,11 @@ public ActionResult doExecute(HttpServletRequest request, RenderContext renderCo return error(response, HttpServletResponse.SC_BAD_REQUEST, readMessage(outcome.getError()), issues); } + if (isBadRequest(outcome.getError())) { + // Malformed request body: a caller mistake, not a server failure + logger.warn("Rejecting a call to JS action '{}' with a malformed body", name); + return error(response, HttpServletResponse.SC_BAD_REQUEST, readMessage(outcome.getError())); + } // 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, @@ -160,6 +165,15 @@ private static ActionResult respond(HttpServletResponse response, int status, JS return null; } + /** Whether the adapter flagged the rejection as a caller mistake (e.g. an unparsable body). */ + private static boolean isBadRequest(Value errorValue) { + if (errorValue == null || !errorValue.hasMembers() || !errorValue.hasMember("badRequest")) { + return false; + } + Value flag = errorValue.getMember("badRequest"); + return flag != null && flag.isBoolean() && flag.asBoolean(); + } + private static String readMessage(Value errorValue) { if (errorValue != null && errorValue.hasMembers() && errorValue.hasMember("message")) { Value message = errorValue.getMember("message"); diff --git a/javascript-modules-library/src/framework/actions/registerActionsModule.ts b/javascript-modules-library/src/framework/actions/registerActionsModule.ts index e724a32d..f037929f 100644 --- a/javascript-modules-library/src/framework/actions/registerActionsModule.ts +++ b/javascript-modules-library/src/framework/actions/registerActionsModule.ts @@ -33,7 +33,16 @@ export const __registerActionsModule = ( server.registry.add("action", `${moduleName}/${name}`, { execute: (body: string) => Promise.resolve() - .then(() => fn(...(parse(body) as unknown[]))) + .then(() => { + let args: unknown[]; + try { + args = parse(body) as unknown[]; + } catch { + // a caller mistake, not a server failure: flagged for the endpoint to answer 400 + throw { message: "Malformed request body", badRequest: true }; + } + return fn(...args); + }) .then( (result) => stringify(result), (error: unknown) => { @@ -43,16 +52,21 @@ export const __registerActionsModule = ( // logged here and replaced by a generic message (unexpected messages can leak // implementation details, node paths or permissions). const issues = (error as { issues?: unknown } | undefined)?.issues; + const badRequest = (error as { badRequest?: unknown } | undefined)?.badRequest === true; const deliberate = + badRequest || Array.isArray(issues) || error instanceof ActionError || error instanceof ActionValidationError; - const shaped: { message: string; issues?: string } = { + const shaped: { message: string; issues?: string; badRequest?: boolean } = { message: deliberate ? messageOf(error) : "Action execution failed", }; if (Array.isArray(issues)) { shaped.issues = JSON.stringify(issues); } + if (badRequest) { + shaped.badRequest = true; + } if (!deliberate) { console.error(`Action ${moduleName}/${name} threw: ${messageOf(error)}`); } From af3341f55f829defccfe37bd76814e3af1e52f82 Mon Sep 17 00:00:00 2001 From: Romain Gauthier Date: Fri, 14 Aug 2026 14:38:04 +0200 Subject: [PATCH 3/5] test(e2e): cover every status of the action envelope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New cases: missing action name (400), malformed body (400), non-settling promise (500), masked unexpected error messages (500, generic) — and the deliberate-failure fixture now throws ActionError, whose message is the one that travels to callers. --- .../src/actions/calculator.action.ts | 13 +++++- tests/cypress/e2e/ui/genericActionTest.cy.ts | 43 ++++++++++++++++++- 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/jahia-test-module/src/actions/calculator.action.ts b/jahia-test-module/src/actions/calculator.action.ts index f8f813b0..65dbde63 100644 --- a/jahia-test-module/src/actions/calculator.action.ts +++ b/jahia-test-module/src/actions/calculator.action.ts @@ -1,4 +1,4 @@ -import { action, type StandardSchemaV1 } from "@jahia/javascript-modules-library"; +import { action, ActionError, type StandardSchemaV1 } from "@jahia/javascript-modules-library"; /** * Test fixtures for actions (.action.ts files): functions executed on the server, callable from the @@ -15,10 +15,19 @@ export const echoKinds = (input: { date: Date; map: Map; set: Se setHas: input.set.has("present"), }); +// an ActionError is deliberate: its message travels to the caller export const failOnPurpose = () => { - throw new Error("Intentional failure"); + throw new ActionError("Intentional failure"); }; +// any other exception is masked with a generic message (actions are guest-callable) +export const failInternally = () => { + throw new Error("secret implementation detail"); +}; + +// depends on an event that never happens: the runtime must detect it instead of hanging +export const neverSettles = () => new Promise(() => {}); + /** Hand-rolled Standard Schema, to avoid pulling a validation library into the test module. */ const positiveNumberInput: StandardSchemaV1<{ n: number }> = { "~standard": { diff --git a/tests/cypress/e2e/ui/genericActionTest.cy.ts b/tests/cypress/e2e/ui/genericActionTest.cy.ts index 878cb61e..ac249e4e 100644 --- a/tests/cypress/e2e/ui/genericActionTest.cy.ts +++ b/tests/cypress/e2e/ui/genericActionTest.cy.ts @@ -79,13 +79,54 @@ describe("Actions (.action.ts files)", () => { }); }); - it("surfaces thrown errors as a server error", () => { + it("surfaces ActionError messages as a server error", () => { callAction(`${MODULE}/failOnPurpose`, []).then((response) => { expect(response.status).to.eq(500); expect(response.body.error).to.contain("Intentional failure"); }); }); + it("masks unexpected error messages (guest-callable endpoint)", () => { + callAction(`${MODULE}/failInternally`, []).then((response) => { + expect(response.status).to.eq(500); + expect(response.body.error).to.not.contain("secret"); + expect(response.body.error).to.contain("Action execution failed"); + }); + }); + + it("answers 500 on a promise that cannot settle on the server", () => { + callAction(`${MODULE}/neverSettles`, []).then((response) => { + expect(response.status).to.eq(500); + expect(response.body.error).to.contain("did not settle"); + }); + }); + + it("rejects a missing action name as a caller mistake", () => { + cy.request({ + method: "POST", + url: `/cms/render/default/en${pagePath}.jsAction.do`, + headers: { "X-JS-Action": "1", "accept": "application/json" }, + body: stringify([]), + failOnStatusCode: false, + }).then((response) => { + expect(response.status).to.eq(400); + expect(response.body.error).to.contain("Missing action name"); + }); + }); + + it("rejects a malformed body as a caller mistake", () => { + cy.request({ + method: "POST", + url: `/cms/render/default/en${pagePath}.jsAction.do?name=${encodeURIComponent(`${MODULE}/add`)}`, + headers: { "X-JS-Action": "1", "accept": "application/json" }, + body: "not devalue at all", + failOnStatusCode: false, + }).then((response) => { + expect(response.status).to.eq(400); + expect(response.body.error).to.contain("Malformed request body"); + }); + }); + it("rejects invalid input of safe actions with validation issues", () => { callAction(`${MODULE}/safeDouble`, [{ n: -1 }]).then((response) => { expect(response.status).to.eq(400); From efdd5e37d964d35ede5685f111a71a5a316e0c21 Mon Sep 17 00:00:00 2001 From: Romain Gauthier Date: Fri, 14 Aug 2026 14:38:04 +0200 Subject: [PATCH 4/5] docs: align the status table, ADR-0008 and changelog with the envelope - the status table's 400 row now lists all the caller mistakes the endpoint answers with 400 - ADR-0008 still described the envelope as 'always on HTTP 200', the very behavior this branch removes: describe the shipped protocol and keep the history as context - the changelog no longer presents the empty-200 behavior as something a released version had --- .chachalog/Hv2mTn8p.md | 2 +- docs/2-guides/7-actions/README.md | 12 ++++++------ docs/adr/0008-client-callable-actions.md | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.chachalog/Hv2mTn8p.md b/.chachalog/Hv2mTn8p.md index f1116711..6454121f 100644 --- a/.chachalog/Hv2mTn8p.md +++ b/.chachalog/Hv2mTn8p.md @@ -3,4 +3,4 @@ javascript-modules: patch --- -The generic action endpoint (`.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) +The generic action endpoint (`.jsAction.do`) answers its JSON envelope whatever the caller's `accept` header, with a status describing the outcome: `400` for caller mistakes (missing header or name, malformed body, input rejected by an action's schema), `404` for an unknown action, `500` when the function throws. Plain HTTP callers (curl, server-to-server) can rely on the status; the generated client stubs discriminate on the envelope. (#707) diff --git a/docs/2-guides/7-actions/README.md b/docs/2-guides/7-actions/README.md index d3b9fce6..ca943758 100644 --- a/docs/2-guides/7-actions/README.md +++ b/docs/2-guides/7-actions/README.md @@ -72,12 +72,12 @@ On invalid input the client call rejects with an error carrying the validation ` Tests, server-to-server integrations and `curl` can call the endpoint directly. It always answers a JSON envelope — `{"data": ""}` 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 | +| Status | Meaning | +| ------ | ------------------------------------------------------------------------------------------------------------------------------------ | +| `200` | the function returned; `data` holds its devalue-encoded result | +| `400` | caller mistake: missing `X-JS-Action` header, missing `name` parameter, malformed body, or input rejected by the schema (`issues`) | +| `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" \ diff --git a/docs/adr/0008-client-callable-actions.md b/docs/adr/0008-client-callable-actions.md index a2a91cbc..9ab4b849 100644 --- a/docs/adr/0008-client-callable-actions.md +++ b/docs/adr/0008-client-callable-actions.md @@ -30,7 +30,7 @@ Export discovery is a deliberate v1 simplification: only top-level `export const One engine-owned platform action, `jsAction` (`GenericActionEndpoint`), dispatches to all registered actions: - URL: `.jsAction.do?name=/` — riding the Render servlet keeps Jahia's authentication valves (calls execute as the visitor, guest included) and requires no new servlet/HTTP-whiteboard surface. -- Request body: devalue-serialized arguments array. Response envelope: `{"data": ""}` on success, `{"error": "...", "issues": [...]?}` on failure — always on HTTP 200, because the render servlet only writes JSON bodies for 2xx action results; the stub discriminates on the envelope. +- Request body: devalue-serialized arguments array. Response envelope: `{"data": ""}` on success, `{"error": "...", "issues": [...]?}` on failure — written by the endpoint itself with a status describing the outcome (200/400/404/500). The initial decision rode the render servlet's body writing, which only writes JSON for 2xx results and forced the envelope onto HTTP 200; the endpoint now completes the response on its own, so plain HTTP callers can discriminate on the status while the stub keeps discriminating on the envelope. - The JS adapter (library) owns all serialization: the Java endpoint pipes opaque strings and never converts structured polyglot values. ### CSRF From 9b1d773ea36637cca7613bcad59da4383abde08b Mon Sep 17 00:00:00 2001 From: Romain Gauthier Date: Fri, 14 Aug 2026 14:47:39 +0200 Subject: [PATCH 5/5] test(vite-plugin): refresh the fixture snapshot after the stub hardening The client stub now discriminates errors with `"error" in payload` instead of `payload?.error` (picked up when #713 merged the base branch in); the expected snapshot still carried the old expression. --- vite-plugin/fixtures/expected/client/bar.client.tsx.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vite-plugin/fixtures/expected/client/bar.client.tsx.js b/vite-plugin/fixtures/expected/client/bar.client.tsx.js index 4e2d97df..cfcb4a98 100644 --- a/vite-plugin/fixtures/expected/client/bar.client.tsx.js +++ b/vite-plugin/fixtures/expected/client/bar.client.tsx.js @@ -1,2 +1,2 @@ import{useState as e}from"react";import{jsx as t}from"react/jsx-runtime";var n=2**32-1,r=n-1,i=class extends Error{constructor(e,t,n,r){super(e),this.name=`DevalueError`,this.path=t.join(``),this.value=n,this.root=r}};function a(e){return e===null||typeof e!=`object`&&typeof e!=`function`}var o=Object.getOwnPropertyNames(Object.prototype).sort().join(`\0`);function s(e){let t=Object.getPrototypeOf(e);return t===Object.prototype||t===null||Object.getPrototypeOf(t)===null||Object.getOwnPropertyNames(t).sort().join(`\0`)===o}function c(e){return Object.prototype.toString.call(e).slice(8,-1)}function l(e){switch(e){case`"`:return`\\"`;case`<`:return`\\u003C`;case`\\`:return`\\\\`;case` -`:return`\\n`;case`\r`:return`\\r`;case` `:return`\\t`;case`\b`:return`\\b`;case`\f`:return`\\f`;case`\u2028`:return`\\u2028`;case`\u2029`:return`\\u2029`;default:return e<` `?`\\u${e.charCodeAt(0).toString(16).padStart(4,`0`)}`:``}}function u(e){let t=``,n=0,r=e.length;for(let i=0;iObject.getOwnPropertyDescriptor(e,t).enumerable)}var f=/^[a-zA-Z_$][a-zA-Z_$0-9]*$/;function p(e){return f.test(e)?`.`+e:`[`+JSON.stringify(e)+`]`}function m(e){return!(!Number.isInteger(e)||e<0||e>r)}function h(e){return!(!Number.isInteger(e)||e<0||e>n)}function g(e){if(e.length===0||e.length>1&&e.charCodeAt(0)===48)return!1;for(let t=0;t57)return!1}return m(+e)}function _(e){let t=Object.keys(e);for(var n=t.length-1;n>=0&&!g(t[n]);n--);return t.length=n+1,t}function v(e){return new Uint8Array(e).toBase64()}function y(e){return Uint8Array.fromBase64(e).buffer}function b(e){return Buffer.from(e).toString(`base64`)}function x(e){return Uint8Array.from(Buffer.from(e,`base64`)).buffer}function S(e){let t=new Uint8Array(e),n=``,r=32768;for(let e=0;e=t)throw Error(`Invalid input`);n[r]=o(c[e+1])}n.length=t}else{let t=Array(c.length);i[e]=t;for(let e=0;e{let t=h(e,g);t<0&&(r[g]=t)})}else{let e=c(n);switch(e){case`Number`:case`String`:case`Boolean`:case`BigInt`:v=`["Object",${h(n.valueOf())}]`;break;case`Date`:v=`["Date","${isNaN(n.getDate())?``:n.toISOString()}"]`;break;case`URL`:v=`["URL",${u(n.toString())}]`;break;case`URLSearchParams`:v=`["URLSearchParams",${u(n.toString())}]`;break;case`RegExp`:let{source:r,flags:o}=n;v=o?`["RegExp",${u(r)},"${o}"]`:`["RegExp",${u(r)}]`;break;case`Array`:{let e=!1;v=`[`;for(let t=0;t0&&(v+=`,`),Object.hasOwn(n,t))f.push(`[${t}]`),v+=h(n[t]),f.pop();else if(e)v+=-2;else{let t=_(n),r=t.length,i=String(n.length).length;if((n.length-r)*3>4+i+r*(i+1)){v=`[-7,`+n.length;for(let e=0;e0)throw new i(`Cannot stringify POJOs with symbolic keys`,f,n,t);if(Object.getPrototypeOf(n)===null){v=`["null"`;for(let e of Object.keys(n)){if(e===`__proto__`)throw new i(`Cannot stringify objects with __proto__ keys`,f,n,t);f.push(p(e)),v+=`,${u(e)},${h(n[e])}`,f.pop()}v+=`]`}else{v=`{`;let e=!1;for(let r of Object.keys(n)){if(r===`__proto__`)throw new i(`Cannot stringify objects with __proto__ keys`,f,n,t);e&&(v+=`,`),e=!0,f.push(p(r)),v+=`${u(r)}:${h(n[r])}`,f.pop()}v+=`}`}}}return r[g]=v,g}let g=h(t);return g<0?`${g}`:r}function M(e){let t=typeof e;return t===`string`?u(e):e===void 0?`-1`:e===0&&1/e<0?`-6`:t===`bigint`?`["BigInt","${e}"]`:String(e)}var N=e=>async(...t)=>{let n=`${location.pathname.replace(/\.html$/,``)}.jsAction.do?name=${encodeURIComponent(e)}`,r=await fetch(n,{method:`POST`,headers:{"X-JS-Action":`1`,accept:`application/json`},body:A(t)}),i=await r.json().catch(()=>null);if(i?.error){let e=Error(i.error);throw i.issues&&(e.issues=i.issues),e}if(!r.ok||!i)throw Error(`Action ${e} failed with HTTP ${r.status}`);return O(i.data)},P=N(`@jahia/vite-plugin-fixtures/greet`),F=N(`@jahia/vite-plugin-fixtures/sum`);function I(){let[n,r]=e(``);return t(`button`,{onClick:async()=>{r(`${await P(`world`)} (${await F(1,2,3)})`)},children:n||`Say hello`})}export{I as default}; \ No newline at end of file +`:return`\\n`;case`\r`:return`\\r`;case` `:return`\\t`;case`\b`:return`\\b`;case`\f`:return`\\f`;case`\u2028`:return`\\u2028`;case`\u2029`:return`\\u2029`;default:return e<` `?`\\u${e.charCodeAt(0).toString(16).padStart(4,`0`)}`:``}}function u(e){let t=``,n=0,r=e.length;for(let i=0;iObject.getOwnPropertyDescriptor(e,t).enumerable)}var f=/^[a-zA-Z_$][a-zA-Z_$0-9]*$/;function p(e){return f.test(e)?`.`+e:`[`+JSON.stringify(e)+`]`}function m(e){return!(!Number.isInteger(e)||e<0||e>r)}function h(e){return!(!Number.isInteger(e)||e<0||e>n)}function g(e){if(e.length===0||e.length>1&&e.charCodeAt(0)===48)return!1;for(let t=0;t57)return!1}return m(+e)}function _(e){let t=Object.keys(e);for(var n=t.length-1;n>=0&&!g(t[n]);n--);return t.length=n+1,t}function v(e){return new Uint8Array(e).toBase64()}function y(e){return Uint8Array.fromBase64(e).buffer}function b(e){return Buffer.from(e).toString(`base64`)}function x(e){return Uint8Array.from(Buffer.from(e,`base64`)).buffer}function S(e){let t=new Uint8Array(e),n=``,r=32768;for(let e=0;e=t)throw Error(`Invalid input`);n[r]=o(c[e+1])}n.length=t}else{let t=Array(c.length);i[e]=t;for(let e=0;e{let t=h(e,g);t<0&&(r[g]=t)})}else{let e=c(n);switch(e){case`Number`:case`String`:case`Boolean`:case`BigInt`:v=`["Object",${h(n.valueOf())}]`;break;case`Date`:v=`["Date","${isNaN(n.getDate())?``:n.toISOString()}"]`;break;case`URL`:v=`["URL",${u(n.toString())}]`;break;case`URLSearchParams`:v=`["URLSearchParams",${u(n.toString())}]`;break;case`RegExp`:let{source:r,flags:o}=n;v=o?`["RegExp",${u(r)},"${o}"]`:`["RegExp",${u(r)}]`;break;case`Array`:{let e=!1;v=`[`;for(let t=0;t0&&(v+=`,`),Object.hasOwn(n,t))f.push(`[${t}]`),v+=h(n[t]),f.pop();else if(e)v+=-2;else{let t=_(n),r=t.length,i=String(n.length).length;if((n.length-r)*3>4+i+r*(i+1)){v=`[-7,`+n.length;for(let e=0;e0)throw new i(`Cannot stringify POJOs with symbolic keys`,f,n,t);if(Object.getPrototypeOf(n)===null){v=`["null"`;for(let e of Object.keys(n)){if(e===`__proto__`)throw new i(`Cannot stringify objects with __proto__ keys`,f,n,t);f.push(p(e)),v+=`,${u(e)},${h(n[e])}`,f.pop()}v+=`]`}else{v=`{`;let e=!1;for(let r of Object.keys(n)){if(r===`__proto__`)throw new i(`Cannot stringify objects with __proto__ keys`,f,n,t);e&&(v+=`,`),e=!0,f.push(p(r)),v+=`${u(r)}:${h(n[r])}`,f.pop()}v+=`}`}}}return r[g]=v,g}let g=h(t);return g<0?`${g}`:r}function M(e){let t=typeof e;return t===`string`?u(e):e===void 0?`-1`:e===0&&1/e<0?`-6`:t===`bigint`?`["BigInt","${e}"]`:String(e)}var N=e=>async(...t)=>{let n=`${location.pathname.replace(/\.html$/,``)}.jsAction.do?name=${encodeURIComponent(e)}`,r=await fetch(n,{method:`POST`,headers:{"X-JS-Action":`1`,accept:`application/json`},body:A(t)}),i=await r.json().catch(()=>null);if(i&&`error`in i){let e=Error(i.error);throw i.issues&&(e.issues=i.issues),e}if(!r.ok||!i)throw Error(`Action ${e} failed with HTTP ${r.status}`);return O(i.data)},P=N(`@jahia/vite-plugin-fixtures/greet`),F=N(`@jahia/vite-plugin-fixtures/sum`);function I(){let[n,r]=e(``);return t(`button`,{onClick:async()=>{r(`${await P(`world`)} (${await F(1,2,3)})`)},children:n||`Say hello`})}export{I as default}; \ No newline at end of file