From b642393a159889c50181bb093bada9cb564f2bb8 Mon Sep 17 00:00:00 2001 From: Joachim Van Herwegen Date: Thu, 14 Jan 2021 11:53:56 +0100 Subject: [PATCH] fix: Have PATCH/POST/PUT operations handlers check content-type --- src/ldp/operations/PatchOperationHandler.ts | 15 +++++++++++++++ src/ldp/operations/PostOperationHandler.ts | 9 ++++++--- src/ldp/operations/PutOperationHandler.ts | 9 ++++++--- .../ldp/operations/PatchOperationHandler.test.ts | 13 +++++++++++-- .../ldp/operations/PostOperationHandler.test.ts | 9 ++++++--- .../ldp/operations/PutOperationHandler.test.ts | 16 ++++++++++------ 6 files changed, 54 insertions(+), 17 deletions(-) diff --git a/src/ldp/operations/PatchOperationHandler.ts b/src/ldp/operations/PatchOperationHandler.ts index a4a1ee954..2b047483c 100644 --- a/src/ldp/operations/PatchOperationHandler.ts +++ b/src/ldp/operations/PatchOperationHandler.ts @@ -1,4 +1,6 @@ +import { getLoggerFor } from '../../logging/LogUtil'; import type { ResourceStore } from '../../storage/ResourceStore'; +import { BadRequestHttpError } from '../../util/errors/BadRequestHttpError'; import { NotImplementedHttpError } from '../../util/errors/NotImplementedHttpError'; import type { Patch } from '../http/Patch'; import { ResetResponseDescription } from '../http/response/ResetResponseDescription'; @@ -6,7 +8,13 @@ import type { ResponseDescription } from '../http/response/ResponseDescription'; import type { Operation } from './Operation'; import { OperationHandler } from './OperationHandler'; +/** + * Handles PATCH {@link Operation}s. + * Calls the modifyResource function from a {@link ResourceStore}. + */ export class PatchOperationHandler extends OperationHandler { + protected readonly logger = getLoggerFor(this); + private readonly store: ResourceStore; public constructor(store: ResourceStore) { @@ -21,6 +29,13 @@ export class PatchOperationHandler extends OperationHandler { } public async handle(input: Operation): Promise { + // Solid, §2.1: "A Solid server MUST reject PUT, POST and PATCH requests + // without the Content-Type header with a status code of 400." + // https://solid.github.io/specification/protocol#http-server + if (!input.body?.metadata.contentType) { + this.logger.warn('No Content-Type header specified on PATCH request'); + throw new BadRequestHttpError('No Content-Type header specified on PATCH request'); + } await this.store.modifyResource(input.target, input.body as Patch); return new ResetResponseDescription(); } diff --git a/src/ldp/operations/PostOperationHandler.ts b/src/ldp/operations/PostOperationHandler.ts index 9b9de16f3..1687849cd 100644 --- a/src/ldp/operations/PostOperationHandler.ts +++ b/src/ldp/operations/PostOperationHandler.ts @@ -28,9 +28,12 @@ export class PostOperationHandler extends OperationHandler { } public async handle(input: Operation): Promise { - if (!input.body) { - this.logger.warn('POST operations require a body'); - throw new BadRequestHttpError('POST operations require a body'); + // Solid, §2.1: "A Solid server MUST reject PUT, POST and PATCH requests + // without the Content-Type header with a status code of 400." + // https://solid.github.io/specification/protocol#http-server + if (!input.body?.metadata.contentType) { + this.logger.warn('No Content-Type header specified on POST request'); + throw new BadRequestHttpError('No Content-Type header specified on POST request'); } const identifier = await this.store.addResource(input.target, input.body); return new CreatedResponseDescription(identifier); diff --git a/src/ldp/operations/PutOperationHandler.ts b/src/ldp/operations/PutOperationHandler.ts index 7cdede505..94ec7a469 100644 --- a/src/ldp/operations/PutOperationHandler.ts +++ b/src/ldp/operations/PutOperationHandler.ts @@ -28,9 +28,12 @@ export class PutOperationHandler extends OperationHandler { } public async handle(input: Operation): Promise { - if (typeof input.body !== 'object') { - this.logger.warn('No body specified on PUT request'); - throw new BadRequestHttpError('PUT operations require a body'); + // Solid, §2.1: "A Solid server MUST reject PUT, POST and PATCH requests + // without the Content-Type header with a status code of 400." + // https://solid.github.io/specification/protocol#http-server + if (!input.body?.metadata.contentType) { + this.logger.warn('No Content-Type header specified on PUT request'); + throw new BadRequestHttpError('No Content-Type header specified on PUT request'); } await this.store.setRepresentation(input.target, input.body); return new ResetResponseDescription(); diff --git a/test/unit/ldp/operations/PatchOperationHandler.test.ts b/test/unit/ldp/operations/PatchOperationHandler.test.ts index b1362845d..6767d4218 100644 --- a/test/unit/ldp/operations/PatchOperationHandler.test.ts +++ b/test/unit/ldp/operations/PatchOperationHandler.test.ts @@ -1,6 +1,8 @@ import type { Operation } from '../../../../src/ldp/operations/Operation'; import { PatchOperationHandler } from '../../../../src/ldp/operations/PatchOperationHandler'; +import { RepresentationMetadata } from '../../../../src/ldp/representation/RepresentationMetadata'; import type { ResourceStore } from '../../../../src/storage/ResourceStore'; +import { BadRequestHttpError } from '../../../../src/util/errors/BadRequestHttpError'; import { NotImplementedHttpError } from '../../../../src/util/errors/NotImplementedHttpError'; describe('A PatchOperationHandler', (): void => { @@ -15,10 +17,17 @@ describe('A PatchOperationHandler', (): void => { await expect(handler.canHandle({ method: 'GET' } as Operation)).rejects.toThrow(NotImplementedHttpError); }); + it('errors if there is no body or content-type.', async(): Promise => { + await expect(handler.handle({ } as Operation)).rejects.toThrow(BadRequestHttpError); + await expect(handler.handle({ body: { metadata: new RepresentationMetadata() }} as Operation)) + .rejects.toThrow(BadRequestHttpError); + }); + it('deletes the resource from the store and returns the correct response.', async(): Promise => { - const result = await handler.handle({ target: { path: 'url' }, body: { binary: false }} as Operation); + const metadata = new RepresentationMetadata('text/turtle'); + const result = await handler.handle({ target: { path: 'url' }, body: { metadata }} as Operation); expect(store.modifyResource).toHaveBeenCalledTimes(1); - expect(store.modifyResource).toHaveBeenLastCalledWith({ path: 'url' }, { binary: false }); + expect(store.modifyResource).toHaveBeenLastCalledWith({ path: 'url' }, { metadata }); expect(result.statusCode).toBe(205); expect(result.metadata).toBeUndefined(); expect(result.data).toBeUndefined(); diff --git a/test/unit/ldp/operations/PostOperationHandler.test.ts b/test/unit/ldp/operations/PostOperationHandler.test.ts index 380b51d76..89d0e84ea 100644 --- a/test/unit/ldp/operations/PostOperationHandler.test.ts +++ b/test/unit/ldp/operations/PostOperationHandler.test.ts @@ -20,12 +20,15 @@ describe('A PostOperationHandler', (): void => { .rejects.toThrow(NotImplementedHttpError); }); - it('errors if there is no body.', async(): Promise => { - await expect(handler.handle({ method: 'POST' } as Operation)).rejects.toThrow(BadRequestHttpError); + it('errors if there is no body or content-type.', async(): Promise => { + await expect(handler.handle({ } as Operation)).rejects.toThrow(BadRequestHttpError); + await expect(handler.handle({ body: { metadata: new RepresentationMetadata() }} as Operation)) + .rejects.toThrow(BadRequestHttpError); }); it('adds the given representation to the store and returns the correct response.', async(): Promise => { - const result = await handler.handle({ method: 'POST', body: { }} as Operation); + const metadata = new RepresentationMetadata('text/turtle'); + const result = await handler.handle({ method: 'POST', body: { metadata }} as Operation); expect(result.statusCode).toBe(201); expect(result.metadata).toBeInstanceOf(RepresentationMetadata); expect(result.metadata?.get(HTTP.location)?.value).toBe('newPath'); diff --git a/test/unit/ldp/operations/PutOperationHandler.test.ts b/test/unit/ldp/operations/PutOperationHandler.test.ts index b23af1960..f667ce9f3 100644 --- a/test/unit/ldp/operations/PutOperationHandler.test.ts +++ b/test/unit/ldp/operations/PutOperationHandler.test.ts @@ -1,5 +1,6 @@ import type { Operation } from '../../../../src/ldp/operations/Operation'; import { PutOperationHandler } from '../../../../src/ldp/operations/PutOperationHandler'; +import { RepresentationMetadata } from '../../../../src/ldp/representation/RepresentationMetadata'; import type { ResourceStore } from '../../../../src/storage/ResourceStore'; import { BadRequestHttpError } from '../../../../src/util/errors/BadRequestHttpError'; import { NotImplementedHttpError } from '../../../../src/util/errors/NotImplementedHttpError'; @@ -17,16 +18,19 @@ describe('A PutOperationHandler', (): void => { await expect(handler.canHandle({ method: 'PUT' } as Operation)).resolves.toBeUndefined(); }); + it('errors if there is no body or content-type.', async(): Promise => { + await expect(handler.handle({ } as Operation)).rejects.toThrow(BadRequestHttpError); + await expect(handler.handle({ body: { metadata: new RepresentationMetadata() }} as Operation)) + .rejects.toThrow(BadRequestHttpError); + }); + it('sets the representation in the store and returns the correct response.', async(): Promise => { - const result = await handler.handle({ target: { path: 'url' }, body: {}} as Operation); + const metadata = new RepresentationMetadata('text/turtle'); + const result = await handler.handle({ target: { path: 'url' }, body: { metadata }} as Operation); expect(store.setRepresentation).toHaveBeenCalledTimes(1); - expect(store.setRepresentation).toHaveBeenLastCalledWith({ path: 'url' }, {}); + expect(store.setRepresentation).toHaveBeenLastCalledWith({ path: 'url' }, { metadata }); expect(result.statusCode).toBe(205); expect(result.metadata).toBeUndefined(); expect(result.data).toBeUndefined(); }); - - it('errors when there is no body.', async(): Promise => { - await expect(handler.handle({ method: 'PUT' } as Operation)).rejects.toThrow(BadRequestHttpError); - }); });