From 43ec8e69e51726bec4114026b55f66be94651f92 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Fri, 17 Jul 2026 11:31:44 +0200 Subject: [PATCH 1/2] docs(server): Add rules file for server middleware Introduce .claude/rules/server-middleware.md as a home for rules that apply to every middleware in packages/server/lib/middleware/. The first rule requires middlewares to use only the built-in Node.js HTTP API so the underlying HTTP framework (currently Express) can be swapped without rewriting any middleware. Future middleware rules can be added as sections in the same file. --- .claude/rules/server-middleware.md | 51 ++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 .claude/rules/server-middleware.md diff --git a/.claude/rules/server-middleware.md b/.claude/rules/server-middleware.md new file mode 100644 index 00000000000..851bee8a197 --- /dev/null +++ b/.claude/rules/server-middleware.md @@ -0,0 +1,51 @@ +--- +paths: + - "packages/server/lib/middleware/**/*.js" +--- + +# Server middleware + +Rules that apply to every middleware in `packages/server/lib/middleware/`. + +## Use only the built-in Node.js HTTP API + +`@ui5/server` runs on Express, but Express is an implementation detail wired up +exclusively in `packages/server/lib/server.js`. Every middleware **MUST** treat +the request and response as plain Node.js `http.IncomingMessage` / +`http.ServerResponse` objects, so the underlying HTTP framework can be swapped +without rewriting any middleware. + +### MUST use — sanctioned Node.js surface + +This is already the norm across the middleware directory: + +- Request: `req.method`, `req.url`, `req.headers` +- Response: `res.setHeader`, `res.getHeader`, `res.writeHead`, `res.statusCode`, `res.end` + +### MUST NOT use — Express-specific sugar + +- Response: `res.send`, `res.json`, `res.type`, `res.status()`, `res.redirect`, `res.sendFile`, `res.cookie` +- Request: `req.query`, `req.params`, `req.cookies`, `req.get()`, `req.body` + +Parse anything you need (query string, body) from the raw Node primitives +(`req.url`, the request stream) instead. + +#### Exception: `req.body` after your own body-parser middleware + +Reading `req.body` is fine **if you installed the body parser yourself** on your +own router — then `req.body` is the output of a middleware you control, not +Express-injected sugar. `csp.js` does this: it registers +`bodyParser.json(...)` on its router before the handler reads `req.body`. + +### Model to imitate + +`packages/server/lib/middleware/liveReloadClient.js` is the reference: it serves +its response using only `req.method`, `middlewareUtil.getPathname(req)`, +`res.setHeader`, `res.statusCode`, and `res.end`. + +### Known debt — do not treat as precedent + +One existing middleware violates this rule and should be migrated (not copied): + +- `serveResources.js` uses `res.send(...)` — replace with `res.end(...)`, + setting `Content-Length` where needed. From 206f94837fe6a3c0dc1fe2a949fd557d026d1dbd Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Fri, 17 Jul 2026 11:32:37 +0200 Subject: [PATCH 2/2] refactor(server): Use res.end instead of Express res.send in serveResources The serveResources middleware relied on Express's res.send, violating the rule that middlewares must use only the built-in Node.js HTTP API so the framework can be swapped. Replace both res.send calls with res.end and set Content-Length explicitly (byte length for the injected HTML string). Also drop the resolved 'Known debt' entry for serveResources from .claude/rules/server-middleware.md. --- .claude/rules/server-middleware.md | 7 ---- .../server/lib/middleware/serveResources.js | 8 +++- .../lib/server/middleware/serveResources.js | 39 ++++++++++--------- 3 files changed, 27 insertions(+), 27 deletions(-) diff --git a/.claude/rules/server-middleware.md b/.claude/rules/server-middleware.md index 851bee8a197..29494b9cb67 100644 --- a/.claude/rules/server-middleware.md +++ b/.claude/rules/server-middleware.md @@ -42,10 +42,3 @@ Express-injected sugar. `csp.js` does this: it registers `packages/server/lib/middleware/liveReloadClient.js` is the reference: it serves its response using only `req.method`, `middlewareUtil.getPathname(req)`, `res.setHeader`, `res.statusCode`, and `res.end`. - -### Known debt — do not treat as precedent - -One existing middleware violates this rule and should be migrated (not copied): - -- `serveResources.js` uses `res.send(...)` — replace with `res.end(...)`, - setting `Content-Length` where needed. diff --git a/packages/server/lib/middleware/serveResources.js b/packages/server/lib/middleware/serveResources.js index 9b32b275b58..13d3cc2a355 100644 --- a/packages/server/lib/middleware/serveResources.js +++ b/packages/server/lib/middleware/serveResources.js @@ -55,9 +55,13 @@ function createMiddleware({resources, middlewareUtil, injectLiveReloadClient = f // script runs. If a later script throws or hangs synchronously, a tag placed // further down would never execute and the page would stay stuck — exactly // when live reload is needed most. - res.send(injectHead(html, INJECT_SCRIPT_TAG)); + const body = injectHead(html, INJECT_SCRIPT_TAG); + res.setHeader("Content-Length", Buffer.byteLength(body)); + res.end(body); } else { - res.send(await resource.getBuffer()); + const body = await resource.getBuffer(); + res.setHeader("Content-Length", body.length); + res.end(body); } } catch (err) { next(err); diff --git a/packages/server/test/lib/server/middleware/serveResources.js b/packages/server/test/lib/server/middleware/serveResources.js index 98c45449925..a09c394d8ab 100644 --- a/packages/server/test/lib/server/middleware/serveResources.js +++ b/packages/server/test/lib/server/middleware/serveResources.js @@ -25,7 +25,6 @@ function createMockResponse({headers = {}} = {}) { setHeader: sinon.stub().callsFake((name, value) => { headers[name] = value; }), - send: sinon.stub(), end: sinon.stub(), statusCode: 200, }; @@ -52,8 +51,10 @@ test.serial("Serves resource with correct Content-Type and ETag", async (t) => { t.regex(res.setHeader.getCall(0).args[1], /javascript/); t.is(res.setHeader.getCall(1).args[0], "ETag"); t.is(res.setHeader.getCall(1).args[1], etag("sha256-xyz")); - t.is(res.send.callCount, 1); - t.is(res.send.getCall(0).args[0], buffer); + t.is(res.end.callCount, 1); + t.is(res.end.getCall(0).args[0], buffer); + const contentLengthCall = res.setHeader.getCalls().find((c) => c.args[0] === "Content-Length"); + t.is(contentLengthCall.args[1], buffer.length); }); test.serial("Injects liveReload tag into HTML right after ", async (t) => { @@ -78,8 +79,11 @@ test.serial("Injects liveReload tag into HTML right after ", async (t) => const etagCall = res.setHeader.getCalls().find((c) => c.args[0] === "ETag"); t.truthy(etagCall); t.is(etagCall.args[1], etag(integrity + ":" + INJECT_SCRIPT_TAG)); - t.is(res.send.callCount, 1); - t.is(res.send.getCall(0).args[0], html.replace("", "\n" + INJECT_SCRIPT_TAG)); + t.is(res.end.callCount, 1); + const expectedBody = html.replace("", "\n" + INJECT_SCRIPT_TAG); + t.is(res.end.getCall(0).args[0], expectedBody); + const contentLengthCall = res.setHeader.getCalls().find((c) => c.args[0] === "Content-Length"); + t.is(contentLengthCall.args[1], Buffer.byteLength(expectedBody)); }); test.serial("Injects liveReload tag after with attributes", async (t) => { @@ -100,8 +104,8 @@ test.serial("Injects liveReload tag after with attributes", async (t) => await middleware(req, res, next); - t.is(res.send.callCount, 1); - t.is(res.send.getCall(0).args[0], html.replace(``, `\n` + INJECT_SCRIPT_TAG)); + t.is(res.end.callCount, 1); + t.is(res.end.getCall(0).args[0], html.replace(``, `\n` + INJECT_SCRIPT_TAG)); }); test.serial("Injects liveReload tag after when is missing", async (t) => { @@ -122,8 +126,8 @@ test.serial("Injects liveReload tag after when is missing", async await middleware(req, res, next); - t.is(res.send.callCount, 1); - t.is(res.send.getCall(0).args[0], html.replace("", "\n" + INJECT_SCRIPT_TAG)); + t.is(res.end.callCount, 1); + t.is(res.end.getCall(0).args[0], html.replace("", "\n" + INJECT_SCRIPT_TAG)); }); test.serial("Prepends liveReload tag to HTML without or ", async (t) => { @@ -144,8 +148,8 @@ test.serial("Prepends liveReload tag to HTML without or ", async (t await middleware(req, res, next); - t.is(res.send.callCount, 1); - t.is(res.send.getCall(0).args[0], INJECT_SCRIPT_TAG + html); + t.is(res.end.callCount, 1); + t.is(res.end.getCall(0).args[0], INJECT_SCRIPT_TAG + html); }); test.serial("Calls next() when resource is not found", async (t) => { @@ -163,7 +167,7 @@ test.serial("Calls next() when resource is not found", async (t) => { t.is(next.callCount, 1); t.is(next.getCall(0).args.length, 0); - t.is(res.send.callCount, 0); + t.is(res.end.callCount, 0); }); test.serial("Returns 304 Not Modified when client cache is fresh", async (t) => { @@ -185,7 +189,7 @@ test.serial("Returns 304 Not Modified when client cache is fresh", async (t) => t.is(res.statusCode, 304); t.is(res.end.callCount, 1); - t.is(res.send.callCount, 0); + t.is(res.end.getCall(0).args.length, 0); t.is(next.callCount, 0); }); @@ -223,7 +227,7 @@ test.serial("Does not override existing Content-Type header", async (t) => { const contentTypeSetCalls = res.setHeader.getCalls().filter((c) => c.args[0] === "Content-Type"); t.is(contentTypeSetCalls.length, 0); - t.is(res.send.callCount, 1); + t.is(res.end.callCount, 1); }); test.serial("Uses resource integrity for ETag generation", async (t) => { @@ -267,7 +271,7 @@ test.serial("HTML: 304 when ETag matches injected ETag", async (t) => { t.is(res.statusCode, 304); t.is(res.end.callCount, 1); - t.is(res.send.callCount, 0); + t.is(res.end.getCall(0).args.length, 0); }); test.serial("HTML: 200 when INJECT_SCRIPT_TAG changes invalidates client ETag", async (t) => { @@ -298,9 +302,8 @@ test.serial("HTML: 200 when INJECT_SCRIPT_TAG changes invalidates client ETag", await middleware(req, res, next); t.not(res.statusCode, 304); - t.is(res.end.callCount, 0); - t.is(res.send.callCount, 1); - t.true(res.send.getCall(0).args[0].includes(newInjectTag)); + t.is(res.end.callCount, 1); + t.true(res.end.getCall(0).args[0].includes(newInjectTag)); const etagCall = res.setHeader.getCalls().find((c) => c.args[0] === "ETag"); t.is(etagCall.args[1], expectedServerEtag); });