CommunitySolidServer · joachimvh · Jan 9, 2025 · Jan 7, 2025 · Jan 8, 2025
diff --git a/config/ldp/handler/components/error-handler.json b/config/ldp/handler/components/error-handler.json
@@ -16,10 +16,16 @@
           "handlers": [
             {
               "comment": "Redirects are created internally by throwing a specific error; this handler converts them to the correct response.",
+              "@id": "urn:solid-server:default:RedirectingErrorHandler",
               "@type": "RedirectingErrorHandler"
             },
+            {
+              "@id": "urn:solid-server:default:EmptyErrorHandler",
+              "@type": "EmptyErrorHandler"
+            },
             {
               "comment": "Converts an Error object into a representation for an HTTP response.",
+              "@id": "urn:solid-server:default:ConvertingErrorHandler",
               "@type": "ConvertingErrorHandler",
               "converter": { "@id": "urn:solid-server:default:UiEnabledConverter" },
               "preferenceParser": { "@id": "urn:solid-server:default:PreferenceParser" },

diff --git a/src/http/output/error/EmptyErrorHandler.ts b/src/http/output/error/EmptyErrorHandler.ts
@@ -0,0 +1,44 @@
+import { NotImplementedHttpError } from '../../../util/errors/NotImplementedHttpError';
+import { SOLID_ERROR } from '../../../util/Vocabularies';
+import { ResponseDescription } from '../response/ResponseDescription';
+import type { ErrorHandlerArgs } from './ErrorHandler';
+import { ErrorHandler } from './ErrorHandler';
+
+/**
+ * An {@link ErrorHandler} that returns an error response without adding a body.
+ * For certain status codes, such as 304, it is important to not change anything
+ * in the headers, such as content-type.
+ *
+ * The `statusCodes` array contains the status codes of error types for which
+ * a body should never be added.
+ *
+ * The `always` boolean can be set to `true` to indicate that all errors should
+ * be handled here.
+ *
+ * For errors with different status codes, a metadata field can be added
+ * to indicate that this specific error response should not receive a body.
+ * The predicate should be `urn:npm:solid:community-server:error:emptyBody`
+ * and the value `true`.
+ */
+export class EmptyErrorHandler extends ErrorHandler {
+  protected readonly statusCodes: number[];
+  protected readonly always: boolean;
+
+  public constructor(statusCodes = [ 304 ], always = false) {
+    super();
+    this.statusCodes = statusCodes;
+    this.always = always;
+  }
+
+  public async canHandle({ error }: ErrorHandlerArgs): Promise<void> {
+    if (this.always || this.statusCodes.includes(error.statusCode) ||
+      error.metadata.get(SOLID_ERROR.terms.emptyBody)?.value === 'true') {
+      return;
+    }
+    throw new NotImplementedHttpError();
+  }
+
+  public async handle({ error }: ErrorHandlerArgs): Promise<ResponseDescription> {
+    return new ResponseDescription(error.statusCode, error.metadata);
+  }
+}
diff --git a/src/index.ts b/src/index.ts
@@ -97,6 +97,7 @@ export * from './http/ldp/PutOperationHandler';
 
 // HTTP/Output/Error
 export * from './http/output/error/ConvertingErrorHandler';
+export * from './http/output/error/EmptyErrorHandler';
 export * from './http/output/error/ErrorHandler';
 export * from './http/output/error/RedirectingErrorHandler';
 export * from './http/output/error/SafeErrorHandler';

diff --git a/src/util/ResourceUtil.ts b/src/util/ResourceUtil.ts
@@ -92,7 +92,17 @@ export function assertReadConditions(body: Representation, eTagHandler: ETagHand
   const eTag = eTagHandler.getETag(body.metadata);
   if (conditions && !conditions.matchesMetadata(body.metadata, true)) {
     body.data.destroy();
-    throw new NotModifiedHttpError(eTag);
+    const error = new NotModifiedHttpError(eTag);
+
+    // From RFC 9111:
+    // > the cache MUST add each header field in the provided response to the stored response,
+    // > replacing field values that are already present
+    // So we need to make sure to send either no partial headers, or the exact same headers.
+    // By adding the metadata of the original resource here, we ensure we send the same headers.
+    error.metadata.identifier = body.metadata.identifier;
+    error.metadata.addQuads(body.metadata.quads());
+
+    throw error;
   }
   body.metadata.set(HH.terms.etag, eTag);
 }
diff --git a/src/util/Vocabularies.ts b/src/util/Vocabularies.ts
@@ -282,6 +282,8 @@ export const SOLID_AS = createVocabulary(
 export const SOLID_ERROR = createVocabulary(
   'urn:npm:solid:community-server:error:',
   'disallowedMethod',
+  // Boolean value used to indicate that no response body should be returned for this error
+  'emptyBody',
   'errorCode',
   'errorResponse',
   'stack',

diff --git a/test/integration/Conditions.test.ts b/test/integration/Conditions.test.ts
@@ -9,7 +9,6 @@ import {
   getTestConfigPath,
   getTestFolder,
   instantiateFromConfig,
-  removeFolder,
 } from './Config';
 
 const { namedNode, quad } = DataFactory;
@@ -24,12 +23,21 @@ const stores: [string, any][] = [
     storeConfig: 'storage/backend/memory.json',
     teardown: jest.fn(),
   }],
-  [ 'on-disk storage', {
-    storeConfig: 'storage/backend/file.json',
-    teardown: async(): Promise<void> => removeFolder(rootFilePath),
-  }],
+  // [ 'on-disk storage', {
+  //   storeConfig: 'storage/backend/file.json',
+  //   teardown: async(): Promise<void> => removeFolder(rootFilePath),
+  // }],
 ];
 
+function extractHeadersObject(response: Response): Record<string, string> {
+  const result: Record<string, string> = {};
+  // eslint-disable-next-line unicorn/no-array-for-each
+  response.headers.forEach((value, key): void => {
+    result[key] = value;
+  });
+  return result;
+}
+
 describe.each(stores)('A server supporting conditions with %s', (name, { storeConfig, teardown }): void => {
   let app: App;
 
@@ -175,6 +183,7 @@ describe.each(stores)('A server supporting conditions with %s', (name, { storeCo
     // GET root ETag
     let response = await getResource(baseUrl);
     const eTag = response.headers.get('ETag');
+    const originalHeaders = extractHeadersObject(response);
     expect(typeof eTag).toBe('string');
 
     // GET fails because of header
@@ -184,6 +193,10 @@ describe.each(stores)('A server supporting conditions with %s', (name, { storeCo
     });
     expect(response.status).toBe(304);
     expect(response.headers.get('etag')).toBe(eTag);
+    const newGetHeaders = extractHeadersObject(response);
+    // Date field shouldn't be the same
+    delete newGetHeaders.date;
+    expect(expect.objectContaining(newGetHeaders)).toEqual(originalHeaders);
 
     // HEAD fails because of header
     response = await fetch(baseUrl, {
@@ -192,6 +205,10 @@ describe.each(stores)('A server supporting conditions with %s', (name, { storeCo
     });
     expect(response.status).toBe(304);
     expect(response.headers.get('etag')).toBe(eTag);
+    const newHeadHeaders = extractHeadersObject(response);
+    // Date field shouldn't be the same
+    delete newHeadHeaders.date;
+    expect(expect.objectContaining(newHeadHeaders)).toEqual(originalHeaders);
 
     // GET succeeds if the ETag header doesn't match
     response = await fetch(baseUrl, {
@@ -232,17 +249,28 @@ describe.each(stores)('A server supporting conditions with %s', (name, { storeCo
   it('returns different ETags for different content-types.', async(): Promise<void> => {
     let response = await getResource(baseUrl, { accept: 'text/turtle' }, { contentType: 'text/turtle' });
     const eTagTurtle = response.headers.get('ETag');
+    const turtleHeaders = extractHeadersObject(response);
     response = await getResource(baseUrl, { accept: 'application/ld+json' }, { contentType: 'application/ld+json' });
     const eTagJson = response.headers.get('ETag');
+    const jsonHeaders = extractHeadersObject(response);
     expect(eTagTurtle).not.toEqual(eTagJson);
 
     // Both ETags can be used on the same resource
     response = await fetch(baseUrl, { headers: { 'if-none-match': eTagTurtle!, accept: 'text/turtle' }});
     expect(response.status).toBe(304);
     expect(response.headers.get('etag')).toBe(eTagTurtle);
+    const newTurtleHeaders = extractHeadersObject(response);
+    // Date field shouldn't be the same
+    delete newTurtleHeaders.date;
+    expect(expect.objectContaining(newTurtleHeaders)).toEqual(turtleHeaders);
+
     response = await fetch(baseUrl, { headers: { 'if-none-match': eTagJson!, accept: 'application/ld+json' }});
     expect(response.status).toBe(304);
     expect(response.headers.get('etag')).toBe(eTagJson);
+    const newJsonHeaders = extractHeadersObject(response);
+    // Date field shouldn't be the same
+    delete newJsonHeaders.date;
+    expect(expect.objectContaining(newJsonHeaders)).toEqual(jsonHeaders);
 
     // But not for the other representation
     response = await fetch(baseUrl, { headers: { 'if-none-match': eTagTurtle!, accept: 'application/ld+json' }});

diff --git a/test/unit/http/output/error/EmptyErrorHandler.test.ts b/test/unit/http/output/error/EmptyErrorHandler.test.ts
@@ -0,0 +1,47 @@
+import { EmptyErrorHandler } from '../../../../../src/http/output/error/EmptyErrorHandler';
+import type { HttpRequest } from '../../../../../src/server/HttpRequest';
+import { BadRequestHttpError } from '../../../../../src/util/errors/BadRequestHttpError';
+import { NotImplementedHttpError } from '../../../../../src/util/errors/NotImplementedHttpError';
+import { NotModifiedHttpError } from '../../../../../src/util/errors/NotModifiedHttpError';
+import { SOLID_ERROR } from '../../../../../src/util/Vocabularies';
+
+describe('An EmptyErrorHandler', (): void => {
+  const request: HttpRequest = {} as any;
+
+  it('can only handle 304 errors by default.', async(): Promise<void> => {
+    const handler = new EmptyErrorHandler();
+    await expect(handler.canHandle({ error: new NotModifiedHttpError(), request })).resolves.toBeUndefined();
+    await expect(handler.canHandle({ error: new BadRequestHttpError(), request }))
+      .rejects.toThrow(NotImplementedHttpError);
+  });
+
+  it('can support other error types.', async(): Promise<void> => {
+    const handler = new EmptyErrorHandler([ 400 ]);
+    await expect(handler.canHandle({ error: new BadRequestHttpError(), request })).resolves.toBeUndefined();
+    await expect(handler.canHandle({ error: new NotModifiedHttpError(), request }))
+      .rejects.toThrow(NotImplementedHttpError);
+  });
+
+  it('can support all error types.', async(): Promise<void> => {
+    const handler = new EmptyErrorHandler([], true);
+    await expect(handler.canHandle({ error: new BadRequestHttpError(), request })).resolves.toBeUndefined();
+    await expect(handler.canHandle({ error: new NotModifiedHttpError(), request })).resolves.toBeUndefined();
+  });
+
+  it('can support specific error instances.', async(): Promise<void> => {
+    const handler = new EmptyErrorHandler();
+    const error = new BadRequestHttpError();
+    await expect(handler.canHandle({ error, request })).rejects.toThrow(NotImplementedHttpError);
+    error.metadata.add(SOLID_ERROR.terms.emptyBody, 'true');
+    await expect(handler.canHandle({ error: new NotModifiedHttpError(), request })).resolves.toBeUndefined();
+  });
+
+  it('returns a ResponseDescription with an empty body.', async(): Promise<void> => {
+    const handler = new EmptyErrorHandler();
+    const error = new NotModifiedHttpError();
+    const response = await handler.handle({ error, request });
+    expect(response.statusCode).toBe(error.statusCode);
+    expect(response.data).toBeUndefined();
+    expect(response.metadata).toBe(error.metadata);
+  });
+});