Skip to content

Commit

Permalink
fix: Prevent server from crashing if requested data can't be handled
Browse files Browse the repository at this point in the history
  • Loading branch information
joachimvh committed Jul 17, 2020
1 parent 225afba commit dc09b54
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 5 deletions.
2 changes: 1 addition & 1 deletion src/ldp/AuthenticatedLdpHandler.ts
Expand Up @@ -98,7 +98,7 @@ export class AuthenticatedLdpHandler extends HttpHandler {

const writeData = { response: input.response, description, error: err };

return this.responseWriter.handleSafe(writeData);
await this.responseWriter.handleSafe(writeData);
}

/**
Expand Down
5 changes: 5 additions & 0 deletions src/ldp/http/SimpleResponseWriter.ts
Expand Up @@ -12,6 +12,11 @@ export class SimpleResponseWriter extends ResponseWriter {
if (!input.description && !input.error) {
throw new UnsupportedHttpError('Either a description or an error is required for output.');
}
if (input.description && input.description.body) {
if (input.description.body.dataType !== 'binary' && input.description.body.dataType !== 'string') {
throw new UnsupportedHttpError('Only string or binary results are supported.');
}
}
}

public async handle(input: { response: HttpResponse; description?: ResponseDescription; error?: Error }): Promise<void> {
Expand Down
8 changes: 7 additions & 1 deletion src/server/ExpressHttpServer.ts
Expand Up @@ -21,7 +21,13 @@ export class ExpressHttpServer {
}));

app.use(async(request, response): Promise<void> => {
await this.handler.handleSafe({ request, response });
try {
await this.handler.handleSafe({ request, response });
} catch (error) {
const errMsg = `${error.name}: ${error.message}\n${error.stack}`;
process.stderr.write(errMsg);
response.status(500).send(errMsg);
}
});
return app.listen(port);
}
Expand Down
4 changes: 2 additions & 2 deletions test/unit/ldp/AuthenticatedLdpHandler.test.ts
Expand Up @@ -42,7 +42,7 @@ describe('An AuthenticatedLdpHandler', (): void => {
it('can handle input.', async(): Promise<void> => {
const handler = new AuthenticatedLdpHandler(args);

await expect(handler.handle({ request: 'request' as any, response: 'response' as any })).resolves.toEqual('response');
await expect(handler.handle({ request: 'request' as any, response: 'response' as any })).resolves.toEqual(undefined);
expect(responseFn).toHaveBeenCalledTimes(1);
expect(responseFn).toHaveBeenLastCalledWith({ response: 'response', description: 'operation' as any });
});
Expand All @@ -51,7 +51,7 @@ describe('An AuthenticatedLdpHandler', (): void => {
args.requestParser = new StaticAsyncHandler(false, null);
const handler = new AuthenticatedLdpHandler(args);

await expect(handler.handle({ request: 'request' as any, response: null })).resolves.toEqual('response');
await expect(handler.handle({ request: 'request' as any, response: null })).resolves.toEqual(undefined);
expect(responseFn).toHaveBeenCalledTimes(1);
expect(responseFn.mock.calls[0][0].error).toBeInstanceOf(Error);
});
Expand Down
6 changes: 6 additions & 0 deletions test/unit/ldp/http/SimpleResponseWriter.test.ts
Expand Up @@ -20,6 +20,12 @@ describe('A SimpleResponseWriter', (): void => {
await expect(writer.canHandle({ response })).rejects.toThrow(UnsupportedHttpError);
});

it('requires the description body to be a string or binary stream if present.', async(): Promise<void> => {
await expect(writer.canHandle({ response, description: { body: { dataType: 'quad' }} as ResponseDescription })).rejects.toThrow(UnsupportedHttpError);
await expect(writer.canHandle({ response, description: { body: { dataType: 'string' }} as ResponseDescription })).resolves.toBeUndefined();
await expect(writer.canHandle({ response, description: { body: { dataType: 'binary' }} as ResponseDescription })).resolves.toBeUndefined();
});

it('responds with status code 200 and a location header if there is a description.', async(): Promise<void> => {
await writer.handle({ response, description: { identifier: { path: 'path' }}});
expect(response._isEndCalled()).toBeTruthy();
Expand Down
11 changes: 10 additions & 1 deletion test/unit/server/ExpressHttpServer.test.ts
Expand Up @@ -24,8 +24,9 @@ describe('ExpressHttpServer', (): void => {
let server: Server;
let canHandleJest: jest.Mock<Promise<void>, []>;
let handleJest: jest.Mock<Promise<void>, [any]>;
let handler: SimpleHttpHandler;
beforeEach(async(): Promise<void> => {
const handler = new SimpleHttpHandler();
handler = new SimpleHttpHandler();
canHandleJest = jest.fn(async(): Promise<void> => undefined);
handleJest = jest.fn(async(input): Promise<void> => handle(input));

Expand Down Expand Up @@ -74,4 +75,12 @@ describe('ExpressHttpServer', (): void => {
response: expect.objectContaining({}),
});
});

it('catches errors thrown by its handler.', async(): Promise<void> => {
handler.handle = async(): Promise<void> => {
throw new Error('dummyError');
};
const res = await request(server).get('/').expect(500);
expect(res.text).toContain('dummyError');
});
});

0 comments on commit dc09b54

Please sign in to comment.