Repository: solid/community-server
Pull request #24 (2020-07-13 08:43:35)

Some bugfixes
This fixes 2 specific issues as seen in the commits.

joachimvh: Will also have to find out why tests are currently failing on Node 10

Patch:
diff --git a/src/ldp/AuthenticatedLdpHandler.ts b/src/ldp/AuthenticatedLdpHandler.ts
index 57bcf5619..52aced3d8 100644
--- a/src/ldp/AuthenticatedLdpHandler.ts
+++ b/src/ldp/AuthenticatedLdpHandler.ts
@@ -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);
   }
 
   /**
diff --git a/src/ldp/http/SimpleBodyParser.ts b/src/ldp/http/SimpleBodyParser.ts
index 1ad22eef5..f0eddce2e 100644
--- a/src/ldp/http/SimpleBodyParser.ts
+++ b/src/ldp/http/SimpleBodyParser.ts
@@ -1,8 +1,10 @@
 import { BodyParser } from './BodyParser';
 import { HttpRequest } from '../../server/HttpRequest';
+import { PassThrough } from 'stream';
 import { QuadRepresentation } from '../representation/QuadRepresentation';
 import { RepresentationMetadata } from '../representation/RepresentationMetadata';
 import { StreamParser } from 'n3';
+import { UnsupportedHttpError } from '../../util/errors/UnsupportedHttpError';
 import { UnsupportedMediaTypeHttpError } from '../../util/errors/UnsupportedMediaTypeHttpError';
 
 /**
@@ -41,9 +43,16 @@ export class SimpleBodyParser extends BodyParser {
       contentType: mediaType,
     };
 
+    // Catch parsing errors and emit correct error
+    // Node 10 requires both writableObjectMode and readableObjectMode
+    const errorStream = new PassThrough({ writableObjectMode: true, readableObjectMode: true });
+    const data = input.pipe(new StreamParser());
+    data.pipe(errorStream);
+    data.on('error', (error): boolean => errorStream.emit('error', new UnsupportedHttpError(error.message)));
+
     return {
       dataType: 'quad',
-      data: input.pipe(new StreamParser()),
+      data: errorStream,
       metadata,
     };
   }
diff --git a/src/ldp/http/SimpleResponseWriter.ts b/src/ldp/http/SimpleResponseWriter.ts
index 3b504cd84..ba2187cba 100644
--- a/src/ldp/http/SimpleResponseWriter.ts
+++ b/src/ldp/http/SimpleResponseWriter.ts
@@ -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> {
diff --git a/src/server/ExpressHttpServer.ts b/src/server/ExpressHttpServer.ts
index 488f1f8b8..996c58e8c 100644
--- a/src/server/ExpressHttpServer.ts
+++ b/src/server/ExpressHttpServer.ts
@@ -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);
   }


Test patch:
diff --git a/test/integration/RequestParser.test.ts b/test/integration/RequestParser.test.ts
index dcc156cdf..469d2835b 100644
--- a/test/integration/RequestParser.test.ts
+++ b/test/integration/RequestParser.test.ts
@@ -1,11 +1,11 @@
 import { AcceptPreferenceParser } from '../../src/ldp/http/AcceptPreferenceParser';
 import arrayifyStream from 'arrayify-stream';
 import { HttpRequest } from '../../src/server/HttpRequest';
+import { Readable } from 'stream';
 import { SimpleBodyParser } from '../../src/ldp/http/SimpleBodyParser';
 import { SimpleRequestParser } from '../../src/ldp/http/SimpleRequestParser';
 import { SimpleTargetExtractor } from '../../src/ldp/http/SimpleTargetExtractor';
 import streamifyArray from 'streamify-array';
-import { StreamParser } from 'n3';
 import { namedNode, triple } from '@rdfjs/data-model';
 
 describe('A SimpleRequestParser with simple input parsers', (): void => {
@@ -34,7 +34,7 @@ describe('A SimpleRequestParser with simple input parsers', (): void => {
         language: [{ value: 'en-gb', weight: 1 }, { value: 'en', weight: 0.5 }],
       },
       body: {
-        data: expect.any(StreamParser),
+        data: expect.any(Readable),
         dataType: 'quad',
         metadata: {
           contentType: 'text/turtle',
diff --git a/test/unit/ldp/AuthenticatedLdpHandler.test.ts b/test/unit/ldp/AuthenticatedLdpHandler.test.ts
index efa1d61a5..f1e31b02a 100644
--- a/test/unit/ldp/AuthenticatedLdpHandler.test.ts
+++ b/test/unit/ldp/AuthenticatedLdpHandler.test.ts
@@ -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 });
   });
@@ -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);
   });
diff --git a/test/unit/ldp/http/SimpleBodyParser.test.ts b/test/unit/ldp/http/SimpleBodyParser.test.ts
index 6e141e1fe..e5a0d4300 100644
--- a/test/unit/ldp/http/SimpleBodyParser.test.ts
+++ b/test/unit/ldp/http/SimpleBodyParser.test.ts
@@ -1,8 +1,9 @@
 import arrayifyStream from 'arrayify-stream';
 import { HttpRequest } from '../../../../src/server/HttpRequest';
+import { Readable } from 'stream';
 import { SimpleBodyParser } from '../../../../src/ldp/http/SimpleBodyParser';
 import streamifyArray from 'streamify-array';
-import { StreamParser } from 'n3';
+import { UnsupportedHttpError } from '../../../../src/util/errors/UnsupportedHttpError';
 import { UnsupportedMediaTypeHttpError } from '../../../../src/util/errors/UnsupportedMediaTypeHttpError';
 import { namedNode, triple } from '@rdfjs/data-model';
 import 'jest-rdf';
@@ -42,7 +43,7 @@ describe('A SimpleBodyparser', (): void => {
     input.headers = { 'content-type': 'text/turtle' };
     const result = await bodyParser.handle(input);
     expect(result).toEqual({
-      data: expect.any(StreamParser),
+      data: expect.any(Readable),
       dataType: 'quad',
       metadata: {
         contentType: 'text/turtle',
@@ -56,4 +57,11 @@ describe('A SimpleBodyparser', (): void => {
       namedNode('http://test.com/o'),
     ) ]);
   });
+
+  it('throws an UnsupportedHttpError on invalid triple data when reading the stream.', 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' };
+    const result = await bodyParser.handle(input);
+    await expect(arrayifyStream(result.data)).rejects.toThrow(UnsupportedHttpError);
+  });
 });
diff --git a/test/unit/ldp/http/SimpleResponseWriter.test.ts b/test/unit/ldp/http/SimpleResponseWriter.test.ts
index c2679fdeb..13bc02d9e 100644
--- a/test/unit/ldp/http/SimpleResponseWriter.test.ts
+++ b/test/unit/ldp/http/SimpleResponseWriter.test.ts
@@ -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();
diff --git a/test/unit/server/ExpressHttpServer.test.ts b/test/unit/server/ExpressHttpServer.test.ts
index a48c9619b..183c0d113 100644
--- a/test/unit/server/ExpressHttpServer.test.ts
+++ b/test/unit/server/ExpressHttpServer.test.ts
@@ -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));
 
@@ -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');
+  });
 });