From 4fd7d965f0a3f2fc53093450656bb44aee71964a Mon Sep 17 00:00:00 2001 From: Romain Gauthier Date: Fri, 24 Jul 2026 21:41:45 +0200 Subject: [PATCH] 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); };