Skip to content

Commit

Permalink
code review changes
Browse files Browse the repository at this point in the history
  • Loading branch information
philiplehmann committed Jan 6, 2025
1 parent 4975f97 commit bec8fe4
Show file tree
Hide file tree
Showing 10 changed files with 46 additions and 31 deletions.
8 changes: 2 additions & 6 deletions apps/puppeteer/src/test/puppeteer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,7 @@ describe('puppeteer', { timeout: 120_000 }, () => {
});

expect(response.statusCode).toBe(400);
expect(text).toBe(
'{"issues":[{"code":"invalid_type","expected":"string","received":"undefined","path":["content-type"],"message":"Required"}],"name":"ZodError"}',
);
expect(text).toBe('Invalid request headers');
});

it('should complain about missing url / html', async () => {
Expand All @@ -59,9 +57,7 @@ describe('puppeteer', { timeout: 120_000 }, () => {
});

expect(response.statusCode).toBe(400);
expect(text).toBe(
'{"issues":[{"code":"invalid_union","unionErrors":[{"issues":[{"code":"invalid_type","expected":"string","received":"undefined","path":["url"],"message":"Required"}],"name":"ZodError"},{"issues":[{"code":"invalid_type","expected":"string","received":"undefined","path":["html"],"message":"Required"}],"name":"ZodError"}],"path":[],"message":"Invalid input"}],"name":"ZodError"}',
);
expect(text).toBe('Invalid body');
});

it('should convert url to pdf with all properties', async () => {
Expand Down
2 changes: 1 addition & 1 deletion libs/http/body/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@ export * from './lib/request-to-buffer';
export * from './lib/request-to-json';
export * from './lib/request-to-text';
export * from './lib/streamable-file';
export * from './lib/check-content-type';
export * from './lib/validate-request-headers';
12 changes: 6 additions & 6 deletions libs/http/body/src/lib/request-to-json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,24 @@ import { z } from 'zod';
import { BadRequest } from '@container/http/error';
import { streamToJson } from '@container/stream';
import type { IncomingMessage } from 'node:http';
import { checkContentType } from './check-content-type';
import { validateRequestHeaders } from './validate-request-headers';

export const applicationJSON = z.object({
'content-type': z.string().startsWith('application/json'),
'content-type': z.string().regex(/^application\/json(?:;\s*charset=utf-8)?$/i),
});

export async function requestToJson<T = unknown>(req: IncomingMessage): Promise<T> {
checkContentType(req, applicationJSON);
validateRequestHeaders(req, applicationJSON);

try {
return await streamToJson(req);
} catch (error) {
if (error instanceof Error) {
throw new BadRequest(error.message);
throw new BadRequest('Invalid JSON format');
}
if (typeof error === 'string') {
throw new BadRequest(error);
throw new BadRequest('Invalid JSON format');
}
throw new BadRequest('Unknown error');
throw new BadRequest('Error processing JSON request');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,24 @@ import { BadRequest } from '@container/http/error';
import type { IncomingMessage } from 'node:http';

export function getBoundary(req: IncomingMessage): string {
const boundary = req.headers['content-type']
?.split('; ')
const contentType = req.headers['content-type'];
if (!contentType) {
throw new BadRequest('Missing Content-Type header');
}

const boundary = contentType
.split('; ')
.find((v) => v.startsWith('boundary='))
?.split('boundary=')?.[1];

if (boundary === undefined) {
throw new BadRequest('Missing boundary');
}

// RFC 2046 states boundary can't be longer than 70 characters
if (boundary.length > 70) {
throw new BadRequest('Invalid boundary length');
}

return boundary;
}
Original file line number Diff line number Diff line change
@@ -1,13 +1,20 @@
export function getContentDispositionName(header: Headers): { filename?: string; name: string } {
const contentDisposition = header.get('content-disposition');
if (!contentDisposition) {
throw new Error('Missing content-disposition header');
}

const { filename, name } = Object.fromEntries(
header
.get('content-disposition')
?.split(';')
.map((part) => {
const [key, value = ''] = part.trim().split('=');
return [key, value.replace(/^"(.*)"$/, '$1')];
}) ?? [],
contentDisposition.split(';').map((part) => {
const [key, value = ''] = part.trim().split('=');
return [key, value.replace(/^"(.*)"$/, '$1')];
}),
);

if (!name) {
throw new Error('Missing name in content-disposition header');
}

return {
filename,
name,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { z } from 'zod';
import type { IncomingMessage } from 'node:http';
import { Readable, Transform } from 'node:stream';
import { checkContentType } from '../check-content-type';
import { validateRequestHeaders } from '../validate-request-headers';
import { getBoundary } from './get-boundary';

enum State {
Expand All @@ -22,7 +22,7 @@ export async function requestToMultipartFormData(
req: IncomingMessage,
partCallback: (header: Headers, stream: Readable) => Promise<void>,
): Promise<void> {
checkContentType(req, multipartFormData);
validateRequestHeaders(req, multipartFormData);

const boundaryLine = `--${getBoundary(req)}`;
let lastline = '';
Expand Down
6 changes: 3 additions & 3 deletions libs/http/body/src/lib/request-to-text.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import { z } from 'zod';
import { streamToString } from '@container/stream';
import type { IncomingMessage } from 'node:http';
import { checkContentType } from './check-content-type';
import { validateRequestHeaders } from './validate-request-headers';

export const textPlain = z.object({
'content-type': z.string().startsWith('text/plain').optional(),
'content-type': z.string().regex(/^text\/plain(?:;\s*charset=(?:utf-8|UTF-8))?$/),
});

export async function requestToText(req: IncomingMessage): Promise<string> {
checkContentType(req, textPlain);
validateRequestHeaders(req, textPlain);

return streamToString(req);
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ import { BadRequest } from '@container/http/error';
import type { IncomingMessage } from 'node:http';
import type { ZodType } from 'zod';

export const checkContentType = (req: IncomingMessage, type: ZodType): void => {
export const validateRequestHeaders = (req: IncomingMessage, type: ZodType): void => {
const test = type.safeParse(req.headers);
if (test.success === false) {
throw new BadRequest(JSON.stringify(test.error));
throw new BadRequest('Invalid request headers');
}
};
2 changes: 1 addition & 1 deletion libs/http/validate/src/lib/body.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export function validateBody<RQ extends ReqRes, BodySchema extends ZodSchema>(
if (body.success) {
return body.data;
}
throw new BadRequest(JSON.stringify(body.error));
throw new BadRequest('Invalid body');
};
}

Expand Down
2 changes: 1 addition & 1 deletion libs/http/validate/src/lib/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export function validateQuery<RQ extends ReqRes, QuerySchema extends ZodSchema>(
if (query.success) {
return query.data;
}
throw new BadRequest(JSON.stringify(query.error));
throw new BadRequest('Invalid query');
};
}

Expand Down

0 comments on commit bec8fe4

Please sign in to comment.