fix: Determine body presence according to RFC7230

Fixes https://github.com/solid/community-server/issues/121

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<Representation | undefined> {
-    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;

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' },
         [ '<http://test.com/s> <http://test.com/p> <http://test.com/o>.' ],
       );
       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' },
         [ '<http://test.com/s1> <http://test.com/p1> <http://test.com/o1>.',
           '<http://test.com/s2> <http://test.com/p2> <http://test.com/o2>.' ],
       );
@@ -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 { <http://test.com/s1> <http://test.com/p1> <http://test.com/o1> }',
           'INSERT {<http://test.com/s3> <http://test.com/p3> <http://test.com/o3>}',
           'WHERE {}' ],

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' },
       [ '<http://test.com/s> <http://test.com/p> <http://test.com/o>.' ],
     );
     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' },
       [ '<http://test.com/s> <http://test.com/p> <http://test.com/o>.' ],
     );
     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' },
       [ '<http://test.com/s> <http://test.com/p> <http://test.com/o>.' ],
     );
     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' },
       [ '<http://test.com/s> <http://test.com/p> <http://test.com/o>.' ],
     );
     expect(response.statusCode).toBe(401);

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',
     };
 

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<void> => {
-    await expect(bodyParser.handle({ headers: { }} as HttpRequest)).resolves.toBeUndefined();
+  it('returns empty output if there was no content length or transfer encoding.', async(): Promise<void> => {
+    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<void> => {
+    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<void> => {
+    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<void> => {
     const input = streamifyArray([ '<http://test.com/s> <http://test.com/p> <http://test.com/o>.' ]) 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<void> => {
     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<void> => {
     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<void> => {
     const input = {} as HttpRequest;
-    input.headers = { 'content-type': 'text/turtle', link: '<http://www.w3.org/ns/ldp#Container>; rel="type"' };
+    input.headers = { 'transfer-encoding': 'chunked',
+      'content-type': 'text/turtle',
+      link: '<http://www.w3.org/ns/ldp#Container>; 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<void> => {
     const input = {} as HttpRequest;
-    input.headers = { 'content-type': 'text/turtle',
+    input.headers = { 'transfer-encoding': 'chunked',
+      'content-type': 'text/turtle',
       link: [ '<http://www.w3.org/ns/ldp#Container>; rel="type"',
         '<http://www.w3.org/ns/ldp#Resource>; rel="type"',
         '<unrelatedLink>',