fix: Have AsyncHandlers only check what is necessary

This commit is contained in:
Joachim Van Herwegen 2020-10-05 10:56:35 +02:00
parent 10723bb6b8
commit 4d34cdd12f
11 changed files with 45 additions and 55 deletions

View File

@ -28,20 +28,18 @@ export class BasicRequestParser extends RequestParser {
Object.assign(this, args); Object.assign(this, args);
} }
public async canHandle(input: HttpRequest): Promise<void> { public async canHandle(): Promise<void> {
if (!input.url) { // Can handle all requests
throw new Error('Missing URL.');
}
if (!input.method) {
throw new Error('Missing method.');
}
} }
public async handle(input: HttpRequest): Promise<Operation> { public async handle(input: HttpRequest): Promise<Operation> {
if (!input.method) {
throw new Error('Missing method.');
}
const target = await this.targetExtractor.handleSafe(input); const target = await this.targetExtractor.handleSafe(input);
const preferences = await this.preferenceParser.handleSafe(input); const preferences = await this.preferenceParser.handleSafe(input);
const body = await this.bodyParser.handleSafe(input); const body = await this.bodyParser.handleSafe(input);
return { method: input.method!, target, preferences, body }; return { method: input.method, target, preferences, body };
} }
} }

View File

@ -10,10 +10,8 @@ import { ResponseWriter } from './ResponseWriter';
*/ */
export class BasicResponseWriter extends ResponseWriter { export class BasicResponseWriter extends ResponseWriter {
public async canHandle(input: { response: HttpResponse; result: ResponseDescription | Error }): Promise<void> { public async canHandle(input: { response: HttpResponse; result: ResponseDescription | Error }): Promise<void> {
if (!(input.result instanceof Error)) { if (!(input.result instanceof Error) && input.result.body && !input.result.body.binary) {
if (input.result.body && !input.result.body.binary) { throw new UnsupportedHttpError('Only binary results and errors are supported.');
throw new UnsupportedHttpError('Only binary results are supported.');
}
} }
} }

View File

@ -10,16 +10,17 @@ import { TargetExtractor } from './TargetExtractor';
* TODO: input requires more extensive cleaning/parsing based on headers (see #22). * TODO: input requires more extensive cleaning/parsing based on headers (see #22).
*/ */
export class BasicTargetExtractor extends TargetExtractor { export class BasicTargetExtractor extends TargetExtractor {
public async canHandle(input: HttpRequest): Promise<void> { public async canHandle(): Promise<void> {
// Can handle all URLs
}
public async handle(input: HttpRequest): Promise<ResourceIdentifier> {
if (!input.url) { if (!input.url) {
throw new Error('Missing URL.'); throw new Error('Missing URL.');
} }
if (!input.headers.host) { if (!input.headers.host) {
throw new Error('Missing host.'); throw new Error('Missing host.');
} }
}
public async handle(input: HttpRequest): Promise<ResourceIdentifier> {
const isHttps = input.connection && (input.connection as TLSSocket).encrypted; const isHttps = input.connection && (input.connection as TLSSocket).encrypted;
const url = format({ const url = format({
protocol: `http${isHttps ? 's' : ''}`, protocol: `http${isHttps ? 's' : ''}`,

View File

@ -27,7 +27,7 @@ export class RawBodyParser extends BodyParser {
// While RFC7231 allows treating a body without content type as an octet stream, // While RFC7231 allows treating a body without content type as an octet stream,
// such an omission likely signals a mistake, so force clients to make this explicit. // such an omission likely signals a mistake, so force clients to make this explicit.
if (!input.headers['content-type']) { if (!input.headers['content-type']) {
throw new Error('An HTTP request body was passed without Content-Type header'); throw new UnsupportedHttpError('An HTTP request body was passed without Content-Type header');
} }
return { return {

View File

@ -1,6 +1,7 @@
import { PassThrough } from 'stream'; import { PassThrough } from 'stream';
import { translate } from 'sparqlalgebrajs'; import { translate } from 'sparqlalgebrajs';
import type { HttpRequest } from '../../server/HttpRequest'; import type { HttpRequest } from '../../server/HttpRequest';
import { APPLICATION_SPARQL_UPDATE } from '../../util/ContentTypes';
import { UnsupportedHttpError } from '../../util/errors/UnsupportedHttpError'; import { UnsupportedHttpError } from '../../util/errors/UnsupportedHttpError';
import { UnsupportedMediaTypeHttpError } from '../../util/errors/UnsupportedMediaTypeHttpError'; import { UnsupportedMediaTypeHttpError } from '../../util/errors/UnsupportedMediaTypeHttpError';
import { CONTENT_TYPE } from '../../util/UriConstants'; import { CONTENT_TYPE } from '../../util/UriConstants';
@ -16,9 +17,7 @@ import type { SparqlUpdatePatch } from './SparqlUpdatePatch';
*/ */
export class SparqlUpdateBodyParser extends BodyParser { export class SparqlUpdateBodyParser extends BodyParser {
public async canHandle(input: HttpRequest): Promise<void> { public async canHandle(input: HttpRequest): Promise<void> {
const contentType = input.headers['content-type']; if (input.headers['content-type'] !== APPLICATION_SPARQL_UPDATE) {
if (!contentType || contentType !== 'application/sparql-update') {
throw new UnsupportedMediaTypeHttpError('This parser only supports SPARQL UPDATE data.'); throw new UnsupportedMediaTypeHttpError('This parser only supports SPARQL UPDATE data.');
} }
} }
@ -35,7 +34,7 @@ export class SparqlUpdateBodyParser extends BodyParser {
const sparql = await readableToString(toAlgebraStream); const sparql = await readableToString(toAlgebraStream);
const algebra = translate(sparql, { quads: true }); const algebra = translate(sparql, { quads: true });
const metadata = new RepresentationMetadata({ [CONTENT_TYPE]: 'application/sparql-update' }); const metadata = new RepresentationMetadata({ [CONTENT_TYPE]: APPLICATION_SPARQL_UPDATE });
// Prevent body from being requested again // Prevent body from being requested again
return { return {

View File

@ -20,13 +20,13 @@ export class PostOperationHandler extends OperationHandler {
if (input.method !== 'POST') { if (input.method !== 'POST') {
throw new UnsupportedHttpError('This handler only supports POST operations.'); throw new UnsupportedHttpError('This handler only supports POST operations.');
} }
if (!input.body) {
throw new UnsupportedHttpError('POST operations require a body.');
}
} }
public async handle(input: Operation): Promise<ResponseDescription> { public async handle(input: Operation): Promise<ResponseDescription> {
const identifier = await this.store.addResource(input.target, input.body!); if (!input.body) {
throw new UnsupportedHttpError('POST operations require a body.');
}
const identifier = await this.store.addResource(input.target, input.body);
return { identifier }; return { identifier };
} }
} }

View File

@ -1,8 +1,6 @@
import type { Representation } from '../../ldp/representation/Representation'; import type { Representation } from '../../ldp/representation/Representation';
import { RepresentationMetadata } from '../../ldp/representation/RepresentationMetadata';
import type { RepresentationPreferences } from '../../ldp/representation/RepresentationPreferences';
import { CONTENT_TYPE } from '../../util/UriConstants';
import { matchingMediaType } from '../../util/Util'; import { matchingMediaType } from '../../util/Util';
import { checkRequest } from './ConversionUtil';
import type { RepresentationConverterArgs } from './RepresentationConverter'; import type { RepresentationConverterArgs } from './RepresentationConverter';
import { TypedRepresentationConverter } from './TypedRepresentationConverter'; import { TypedRepresentationConverter } from './TypedRepresentationConverter';
@ -44,18 +42,14 @@ export class ChainedConverter extends TypedRepresentationConverter {
public async canHandle(input: RepresentationConverterArgs): Promise<void> { public async canHandle(input: RepresentationConverterArgs): Promise<void> {
// We assume a chain can be constructed, otherwise there would be a configuration issue // We assume a chain can be constructed, otherwise there would be a configuration issue
// Check if the first converter can handle the input // So we only check if the input can be parsed and the preferred type can be written
const firstChain = await this.getMatchingType(this.converters[0], this.converters[1]); const inTypes = this.filterTypes(await this.first.getInputTypes());
const preferences: RepresentationPreferences = { type: [{ value: firstChain, weight: 1 }]}; const outTypes = this.filterTypes(await this.last.getOutputTypes());
await this.first.canHandle({ ...input, preferences }); checkRequest(input, inTypes, outTypes);
}
// Check if the last converter can produce the output private filterTypes(typeVals: { [contentType: string]: number }): string[] {
const idx = this.converters.length - 1; return Object.keys(typeVals).filter((name): boolean => typeVals[name] > 0);
const lastChain = await this.getMatchingType(this.converters[idx - 1], this.converters[idx]);
const oldMeta = input.representation.metadata;
const metadata = new RepresentationMetadata(oldMeta, { [CONTENT_TYPE]: lastChain });
const representation: Representation = { ...input.representation, metadata };
await this.last.canHandle({ ...input, representation });
} }
public async handle(input: RepresentationConverterArgs): Promise<Representation> { public async handle(input: RepresentationConverterArgs): Promise<Representation> {

View File

@ -1,6 +1,7 @@
// Well-known content types // Well-known content types
export const TEXT_TURTLE = 'text/turtle'; export const TEXT_TURTLE = 'text/turtle';
export const APPLICATION_OCTET_STREAM = 'application/octet-stream'; export const APPLICATION_OCTET_STREAM = 'application/octet-stream';
export const APPLICATION_SPARQL_UPDATE = 'application/sparql-update';
// Internal (non-exposed) content types // Internal (non-exposed) content types
export const INTERNAL_QUADS = 'internal/quads'; export const INTERNAL_QUADS = 'internal/quads';

View File

@ -17,16 +17,12 @@ describe('A BasicRequestParser', (): void => {
requestParser = new BasicRequestParser({ targetExtractor, bodyParser, preferenceParser }); requestParser = new BasicRequestParser({ targetExtractor, bodyParser, preferenceParser });
}); });
it('can handle input with both a URL and a method.', async(): Promise<void> => { it('can handle any input.', async(): Promise<void> => {
await expect(requestParser.canHandle({ url: 'url', method: 'GET' } as any)).resolves.toBeUndefined(); await expect(requestParser.canHandle()).resolves.toBeUndefined();
}); });
it('rejects input with no URL.', async(): Promise<void> => { it('errors if there is no input.', async(): Promise<void> => {
await expect(requestParser.canHandle({ method: 'GET' } as any)).rejects.toThrow('Missing URL.'); await expect(requestParser.handle({ url: 'url' } as any)).rejects.toThrow('Missing method.');
});
it('rejects input with no method.', async(): Promise<void> => {
await expect(requestParser.canHandle({ url: 'url' } as any)).rejects.toThrow('Missing method.');
}); });
it('returns the output of all input parsers after calling handle.', async(): Promise<void> => { it('returns the output of all input parsers after calling handle.', async(): Promise<void> => {

View File

@ -3,16 +3,16 @@ import { BasicTargetExtractor } from '../../../../src/ldp/http/BasicTargetExtrac
describe('A BasicTargetExtractor', (): void => { describe('A BasicTargetExtractor', (): void => {
const extractor = new BasicTargetExtractor(); const extractor = new BasicTargetExtractor();
it('can handle input with an URL and host.', async(): Promise<void> => { it('can handle any input.', async(): Promise<void> => {
await expect(extractor.canHandle({ url: 'url', headers: { host: 'test.com' }} as any)).resolves.toBeUndefined(); await expect(extractor.canHandle()).resolves.toBeUndefined();
}); });
it('rejects input without URL.', async(): Promise<void> => { it('errors if there is no URL.', async(): Promise<void> => {
await expect(extractor.canHandle({ headers: { host: 'test.com' }} as any)).rejects.toThrow('Missing URL.'); await expect(extractor.handle({ headers: { host: 'test.com' }} as any)).rejects.toThrow('Missing URL.');
}); });
it('rejects input without host.', async(): Promise<void> => { it('errors if there is no host.', async(): Promise<void> => {
await expect(extractor.canHandle({ url: 'url', headers: {}} as any)).rejects.toThrow('Missing host.'); await expect(extractor.handle({ url: 'url', headers: {}} as any)).rejects.toThrow('Missing host.');
}); });
it('returns the input URL.', async(): Promise<void> => { it('returns the input URL.', async(): Promise<void> => {

View File

@ -10,12 +10,15 @@ describe('A PostOperationHandler', (): void => {
} as unknown as ResourceStore; } as unknown as ResourceStore;
const handler = new PostOperationHandler(store); const handler = new PostOperationHandler(store);
it('only supports POST operations with a body.', async(): Promise<void> => { it('only supports POST operations.', async(): Promise<void> => {
await expect(handler.canHandle({ method: 'POST', body: { }} as Operation)) await expect(handler.canHandle({ method: 'POST', body: { }} as Operation))
.resolves.toBeUndefined(); .resolves.toBeUndefined();
await expect(handler.canHandle({ method: 'GET', body: { }} as Operation)) await expect(handler.canHandle({ method: 'GET', body: { }} as Operation))
.rejects.toThrow(UnsupportedHttpError); .rejects.toThrow(UnsupportedHttpError);
await expect(handler.canHandle({ method: 'POST' } as Operation)).rejects.toThrow(UnsupportedHttpError); });
it('errors if there is no body.', async(): Promise<void> => {
await expect(handler.handle({ method: 'POST' } as Operation)).rejects.toThrow(UnsupportedHttpError);
}); });
it('adds the given representation to the store and returns the new identifier.', async(): Promise<void> => { it('adds the given representation to the store and returns the new identifier.', async(): Promise<void> => {