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); };