Skip to content

Commit 7b1608e

Browse files
Refactor request handling by consolidating prechecks
- Removed individual protocol version and authentication checks from the request handling flow. - Introduced a new `performRequestPrechecks` function to streamline the validation process for incoming requests. - Updated the `authenticate` and `checkProtocolVersion` functions to accept request bodies directly, enhancing modularity. - Improved error handling by ensuring consistent response structures across precheck validations.
1 parent d9125df commit 7b1608e

File tree

4 files changed

+53
-70
lines changed

4 files changed

+53
-70
lines changed

react_on_rails_pro/packages/node-renderer/src/worker.ts

Lines changed: 15 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,8 @@ import log, { sharedLoggerOptions } from './shared/log';
1313
import packageJson from './shared/packageJson';
1414
import { buildConfig, Config, getConfig } from './shared/configBuilder';
1515
import fileExistsAsync from './shared/fileExistsAsync';
16-
import type { FastifyInstance, FastifyReply, FastifyRequest } from './worker/types';
17-
import checkProtocolVersion from './worker/checkProtocolVersionHandler';
18-
import authenticate from './worker/authHandler';
16+
import type { FastifyInstance, FastifyReply } from './worker/types';
17+
import { performRequestPrechecks } from './worker/requestPrechecks';
1918
import { handleRenderRequest, type ProvidedNewBundle } from './worker/handleRenderRequest';
2019
import handleGracefulShutdown from './worker/handleGracefulShutdown';
2120
import {
@@ -174,42 +173,6 @@ export default function run(config: Partial<Config>) {
174173
done(null, payload);
175174
});
176175

177-
const isProtocolVersionMatch = async (req: FastifyRequest, res: FastifyReply) => {
178-
// Check protocol version
179-
const protocolVersionCheckingResult = checkProtocolVersion(req);
180-
181-
if (typeof protocolVersionCheckingResult === 'object') {
182-
await setResponse(protocolVersionCheckingResult, res);
183-
return false;
184-
}
185-
186-
return true;
187-
};
188-
189-
const isAuthenticated = async (req: FastifyRequest, res: FastifyReply) => {
190-
// Authenticate Ruby client
191-
const authResult = authenticate(req);
192-
193-
if (typeof authResult === 'object') {
194-
await setResponse(authResult, res);
195-
return false;
196-
}
197-
198-
return true;
199-
};
200-
201-
const requestPrechecks = async (req: FastifyRequest, res: FastifyReply) => {
202-
if (!(await isProtocolVersionMatch(req, res))) {
203-
return false;
204-
}
205-
206-
if (!(await isAuthenticated(req, res))) {
207-
return false;
208-
}
209-
210-
return true;
211-
};
212-
213176
// See https://github.com/shakacode/react_on_rails_pro/issues/119 for why
214177
// the digest is part of the request URL. Yes, it's not used here, but the
215178
// server logs might show it to distinguish different requests.
@@ -223,7 +186,9 @@ export default function run(config: Partial<Config>) {
223186
// Can't infer from the route like Express can
224187
Params: { bundleTimestamp: string; renderRequestDigest: string };
225188
}>('/bundles/:bundleTimestamp/render/:renderRequestDigest', async (req, res) => {
226-
if (!(await requestPrechecks(req, res))) {
189+
const precheckResult = performRequestPrechecks(req.body);
190+
if (precheckResult) {
191+
await setResponse(precheckResult, res);
227192
return;
228193
}
229194

@@ -300,26 +265,11 @@ export default function run(config: Partial<Config>) {
300265
// Build a temporary FastifyRequest shape for protocol/auth check
301266
const tempReqBody = typeof obj === 'object' && obj !== null ? (obj as Record<string, unknown>) : {};
302267

303-
// Protocol check
304-
const protoResult = checkProtocolVersion({
305-
...req,
306-
body: tempReqBody,
307-
} as unknown as FastifyRequest);
308-
if (typeof protoResult === 'object') {
309-
return {
310-
response: protoResult,
311-
shouldContinue: false,
312-
};
313-
}
314-
315-
// Auth check
316-
const authResult = authenticate({
317-
...req,
318-
body: tempReqBody,
319-
} as unknown as FastifyRequest);
320-
if (typeof authResult === 'object') {
268+
// Perform request prechecks
269+
const precheckResult = performRequestPrechecks(tempReqBody);
270+
if (precheckResult) {
321271
return {
322-
response: authResult,
272+
response: precheckResult,
323273
shouldContinue: false,
324274
};
325275
}
@@ -401,7 +351,9 @@ export default function run(config: Partial<Config>) {
401351
app.post<{
402352
Body: WithBodyArrayField<Record<string, Asset>, 'targetBundles'>;
403353
}>('/upload-assets', async (req, res) => {
404-
if (!(await requestPrechecks(req, res))) {
354+
const precheckResult = performRequestPrechecks(req.body);
355+
if (precheckResult) {
356+
await setResponse(precheckResult, res);
405357
return;
406358
}
407359
let lockAcquired = false;
@@ -500,7 +452,9 @@ export default function run(config: Partial<Config>) {
500452
Querystring: { filename: string };
501453
Body: WithBodyArrayField<Record<string, unknown>, 'targetBundles'>;
502454
}>('/asset-exists', async (req, res) => {
503-
if (!(await isAuthenticated(req, res))) {
455+
const precheckResult = performRequestPrechecks(req.body);
456+
if (precheckResult) {
457+
await setResponse(precheckResult, res);
504458
return;
505459
}
506460

react_on_rails_pro/packages/node-renderer/src/worker/authHandler.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,16 @@
66
*/
77
// TODO: Replace with fastify-basic-auth per https://github.com/shakacode/react_on_rails_pro/issues/110
88

9-
import type { FastifyRequest } from './types';
109
import { getConfig } from '../shared/configBuilder';
1110

12-
export = function authenticate(req: FastifyRequest) {
11+
export interface AuthBody {
12+
password?: string;
13+
}
14+
15+
export function authenticate(body: AuthBody) {
1316
const { password } = getConfig();
1417

15-
if (password && password !== (req.body as { password?: string }).password) {
18+
if (password && password !== body.password) {
1619
return {
1720
headers: { 'Cache-Control': 'no-cache, no-store, max-age=0, must-revalidate' },
1821
status: 401,
@@ -21,4 +24,4 @@ export = function authenticate(req: FastifyRequest) {
2124
}
2225

2326
return undefined;
24-
};
27+
}

react_on_rails_pro/packages/node-renderer/src/worker/checkProtocolVersionHandler.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
* Logic for checking protocol version.
33
* @module worker/checkProtocVersionHandler
44
*/
5-
import type { FastifyRequest } from './types';
65
import packageJson from '../shared/packageJson';
76
import log from '../shared/log';
87

@@ -41,8 +40,8 @@ interface RequestBody {
4140
railsEnv?: string;
4241
}
4342

44-
export = function checkProtocolVersion(req: FastifyRequest) {
45-
const { protocolVersion: reqProtocolVersion, gemVersion, railsEnv } = req.body as RequestBody;
43+
export function checkProtocolVersion(body: RequestBody) {
44+
const { protocolVersion: reqProtocolVersion, gemVersion, railsEnv } = body;
4645

4746
// Check protocol version
4847
if (reqProtocolVersion !== packageJson.protocolVersion) {
@@ -52,7 +51,7 @@ export = function checkProtocolVersion(req: FastifyRequest) {
5251
data: `Unsupported renderer protocol version ${
5352
reqProtocolVersion
5453
? `request protocol ${reqProtocolVersion}`
55-
: `MISSING with body ${JSON.stringify(req.body)}`
54+
: `MISSING with body ${JSON.stringify(body)}`
5655
} does not match installed renderer protocol ${packageJson.protocolVersion} for version ${packageJson.version}.
5756
Update either the renderer or the Rails server`,
5857
};
@@ -108,4 +107,4 @@ Update either the gem or the node renderer package to match versions.`,
108107
}
109108

110109
return undefined;
111-
};
110+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/**
2+
* Request prechecks logic that is independent of the HTTP server framework.
3+
* @module worker/requestPrechecks
4+
*/
5+
import type { ResponseResult } from '../shared/utils';
6+
import { checkProtocolVersion, type ProtocolVersionBody } from './checkProtocolVersionHandler';
7+
import { authenticate, type AuthBody } from './authHandler';
8+
9+
export interface RequestPrechecksBody extends ProtocolVersionBody, AuthBody {
10+
[key: string]: unknown;
11+
}
12+
13+
export function performRequestPrechecks(body: RequestPrechecksBody): ResponseResult | undefined {
14+
// Check protocol version
15+
const protocolVersionCheckingResult = checkProtocolVersion(body);
16+
if (typeof protocolVersionCheckingResult === 'object') {
17+
return protocolVersionCheckingResult;
18+
}
19+
20+
// Authenticate Ruby client
21+
const authResult = authenticate(body);
22+
if (typeof authResult === 'object') {
23+
return authResult;
24+
}
25+
26+
return undefined;
27+
}

0 commit comments

Comments
 (0)