From 7d566cf80a9291794efee8dfe1528070a927a149 Mon Sep 17 00:00:00 2001 From: Ruben Verborgh Date: Thu, 3 Sep 2020 00:51:40 +0200 Subject: [PATCH] fix: Determine body presence according to RFC7230 Fixes https://github.com/solid/community-server/issues/121 --- src/ldp/http/RawBodyParser.ts | 14 ++++++-- .../AuthenticatedLdpHandler.test.ts | 6 ++-- test/integration/Authorization.test.ts | 8 ++--- test/integration/RequestParser.test.ts | 3 +- test/unit/ldp/http/RawBodyParser.test.ts | 35 +++++++++++++++---- 5 files changed, 48 insertions(+), 18 deletions(-) diff --git a/src/ldp/http/RawBodyParser.ts b/src/ldp/http/RawBodyParser.ts index bab6f44d53..d90b9c48ec 100644 --- a/src/ldp/http/RawBodyParser.ts +++ b/src/ldp/http/RawBodyParser.ts @@ -17,10 +17,18 @@ export class RawBodyParser extends BodyParser { // Note that the only reason this is a union is in case the body is empty. // If this check gets moved away from the BodyParsers this union could be removed public async handle(input: HttpRequest): Promise { - if (!input.headers['content-type']) { + // RFC7230, ยง3.3: The presence of a message body in a request + // is signaled by a Content-Length or Transfer-Encoding header field. + if (!input.headers['content-length'] && !input.headers['transfer-encoding']) { return; } + // 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. + if (!input.headers['content-type']) { + throw new Error('An HTTP request body was passed without Content-Type header'); + } + return { binary: true, data: input, @@ -29,11 +37,11 @@ export class RawBodyParser extends BodyParser { } private parseMetadata(input: HttpRequest): RepresentationMetadata { - const mediaType = input.headers['content-type']!.split(';')[0]; + const contentType = /^[^;]*/u.exec(input.headers['content-type']!)![0]; const metadata: RepresentationMetadata = { raw: [], - contentType: mediaType, + contentType, }; const { link, slug } = input.headers; diff --git a/test/integration/AuthenticatedLdpHandler.test.ts b/test/integration/AuthenticatedLdpHandler.test.ts index b2fe388153..a07daf0a3f 100644 --- a/test/integration/AuthenticatedLdpHandler.test.ts +++ b/test/integration/AuthenticatedLdpHandler.test.ts @@ -70,7 +70,7 @@ describe('An integrated AuthenticatedLdpHandler', (): void => { handler, requestUrl, 'POST', - { 'content-type': 'text/turtle' }, + { 'content-type': 'text/turtle', 'transfer-encoding': 'chunked' }, [ ' .' ], ); expect(response.statusCode).toBe(200); @@ -151,7 +151,7 @@ describe('An integrated AuthenticatedLdpHandler', (): void => { handler, requestUrl, 'POST', - { 'content-type': 'text/turtle' }, + { 'content-type': 'text/turtle', 'transfer-encoding': 'chunked' }, [ ' .', ' .' ], ); @@ -166,7 +166,7 @@ describe('An integrated AuthenticatedLdpHandler', (): void => { handler, requestUrl, 'PATCH', - { 'content-type': 'application/sparql-update' }, + { 'content-type': 'application/sparql-update', 'transfer-encoding': 'chunked' }, [ 'DELETE { }', 'INSERT { }', 'WHERE {}' ], diff --git a/test/integration/Authorization.test.ts b/test/integration/Authorization.test.ts index 3c65ae6ffa..6e764ecd84 100644 --- a/test/integration/Authorization.test.ts +++ b/test/integration/Authorization.test.ts @@ -129,7 +129,7 @@ describe('A server with authorization', (): void => { handler, requestUrl, 'POST', - { 'content-type': 'text/turtle' }, + { 'content-type': 'text/turtle', 'transfer-encoding': 'chunked' }, [ ' .' ], ); expect(response.statusCode).toBe(200); @@ -140,7 +140,7 @@ describe('A server with authorization', (): void => { handler, requestUrl, 'PUT', - { 'content-type': 'text/turtle' }, + { 'content-type': 'text/turtle', 'transfer-encoding': 'chunked' }, [ ' .' ], ); expect(response.statusCode).toBe(200); @@ -162,7 +162,7 @@ describe('A server with authorization', (): void => { handler, requestUrl, 'POST', - { 'content-type': 'text/turtle' }, + { 'content-type': 'text/turtle', 'transfer-encoding': 'chunked' }, [ ' .' ], ); expect(response.statusCode).toBe(401); @@ -173,7 +173,7 @@ describe('A server with authorization', (): void => { handler, requestUrl, 'PUT', - { 'content-type': 'text/turtle' }, + { 'content-type': 'text/turtle', 'transfer-encoding': 'chunked' }, [ ' .' ], ); expect(response.statusCode).toBe(401); diff --git a/test/integration/RequestParser.test.ts b/test/integration/RequestParser.test.ts index aa39023adb..e9897e30d1 100644 --- a/test/integration/RequestParser.test.ts +++ b/test/integration/RequestParser.test.ts @@ -7,7 +7,7 @@ import { BasicTargetExtractor } from '../../src/ldp/http/BasicTargetExtractor'; import { RawBodyParser } from '../../src/ldp/http/RawBodyParser'; import { HttpRequest } from '../../src/server/HttpRequest'; -describe('A SimpleRequestParser with simple input parsers', (): void => { +describe('A BasicRequestParser with simple input parsers', (): void => { const targetExtractor = new BasicTargetExtractor(); const bodyParser = new RawBodyParser(); const preferenceParser = new AcceptPreferenceParser(); @@ -21,6 +21,7 @@ describe('A SimpleRequestParser with simple input parsers', (): void => { accept: 'text/turtle; q=0.8', 'accept-language': 'en-gb, en;q=0.5', 'content-type': 'text/turtle', + 'transfer-encoding': 'chunked', host: 'test.com', }; diff --git a/test/unit/ldp/http/RawBodyParser.test.ts b/test/unit/ldp/http/RawBodyParser.test.ts index 15230f9278..e911bea353 100644 --- a/test/unit/ldp/http/RawBodyParser.test.ts +++ b/test/unit/ldp/http/RawBodyParser.test.ts @@ -12,13 +12,29 @@ describe('A RawBodyparser', (): void => { await expect(bodyParser.canHandle()).resolves.toBeUndefined(); }); - it('returns empty output if there was no content-type.', async(): Promise => { - await expect(bodyParser.handle({ headers: { }} as HttpRequest)).resolves.toBeUndefined(); + it('returns empty output if there was no content length or transfer encoding.', async(): Promise => { + const input = streamifyArray([ '' ]) as HttpRequest; + input.headers = {}; + await expect(bodyParser.handle(input)).resolves.toBeUndefined(); + }); + + it('errors when a content length was specified without content type.', async(): Promise => { + const input = streamifyArray([ 'abc' ]) as HttpRequest; + input.headers = { 'content-length': '0' }; + await expect(bodyParser.handle(input)).rejects + .toThrow('An HTTP request body was passed without Content-Type header'); + }); + + it('errors when a transfer encoding was specified without content type.', async(): Promise => { + const input = streamifyArray([ 'abc' ]) as HttpRequest; + input.headers = { 'transfer-encoding': 'chunked' }; + await expect(bodyParser.handle(input)).rejects + .toThrow('An HTTP request body was passed without Content-Type header'); }); it('returns a Representation if there was data.', async(): Promise => { const input = streamifyArray([ ' .' ]) as HttpRequest; - input.headers = { 'content-type': 'text/turtle' }; + input.headers = { 'transfer-encoding': 'chunked', 'content-type': 'text/turtle' }; const result = (await bodyParser.handle(input))!; expect(result).toEqual({ binary: true, @@ -35,7 +51,7 @@ describe('A RawBodyparser', (): void => { it('adds the slug header to the metadata.', async(): Promise => { const input = {} as HttpRequest; - input.headers = { 'content-type': 'text/turtle', slug: 'slugText' }; + input.headers = { 'transfer-encoding': 'chunked', 'content-type': 'text/turtle', slug: 'slugText' }; const result = (await bodyParser.handle(input))!; expect(result.metadata).toEqual({ contentType: 'text/turtle', @@ -46,13 +62,17 @@ describe('A RawBodyparser', (): void => { it('errors if there are multiple slugs.', async(): Promise => { const input = {} as HttpRequest; - input.headers = { 'content-type': 'text/turtle', slug: [ 'slugTextA', 'slugTextB' ]}; + input.headers = { 'transfer-encoding': 'chunked', + 'content-type': 'text/turtle', + slug: [ 'slugTextA', 'slugTextB' ]}; await expect(bodyParser.handle(input)).rejects.toThrow(UnsupportedHttpError); }); it('adds the link headers to the metadata.', async(): Promise => { const input = {} as HttpRequest; - input.headers = { 'content-type': 'text/turtle', link: '; rel="type"' }; + input.headers = { 'transfer-encoding': 'chunked', + 'content-type': 'text/turtle', + link: '; rel="type"' }; const result = (await bodyParser.handle(input))!; expect(result.metadata).toEqual({ contentType: 'text/turtle', @@ -63,7 +83,8 @@ describe('A RawBodyparser', (): void => { it('supports multiple link headers.', async(): Promise => { const input = {} as HttpRequest; - input.headers = { 'content-type': 'text/turtle', + input.headers = { 'transfer-encoding': 'chunked', + 'content-type': 'text/turtle', link: [ '; rel="type"', '; rel="type"', '',