From 52a3b84ee0dd55baed0dd244f75c12d06ed77666 Mon Sep 17 00:00:00 2001
From: Joachim Van Herwegen <joachimvh@gmail.com>
Date: Wed, 12 May 2021 08:50:02 +0200
Subject: [PATCH] fix: Support missing type preferences in ChainedConverter

---
 src/ldp/http/AcceptPreferenceParser.ts        |  6 ++-
 src/storage/conversion/ChainedConverter.ts    |  8 +---
 test/integration/ServerFetch.test.ts          | 43 ++++++++++++++++---
 .../ldp/http/AcceptPreferenceParser.test.ts   |  3 ++
 .../conversion/ChainedConverter.test.ts       | 18 ++++++--
 5 files changed, 62 insertions(+), 16 deletions(-)

diff --git a/src/ldp/http/AcceptPreferenceParser.ts b/src/ldp/http/AcceptPreferenceParser.ts
index bc915d9ff..df0bd16b0 100644
--- a/src/ldp/http/AcceptPreferenceParser.ts
+++ b/src/ldp/http/AcceptPreferenceParser.ts
@@ -32,8 +32,12 @@ export class AcceptPreferenceParser extends PreferenceParser {
     for (const { name, header, parse } of parsers) {
       const value = headers[header];
       if (typeof value === 'string') {
-        preferences[name] = Object.fromEntries(parse(value)
+        const result = Object.fromEntries(parse(value)
           .map(({ range, weight }): [string, number] => [ range, weight ]));
+        // Interpret empty headers (or headers with no valid values) the same as missing headers
+        if (Object.keys(result).length > 0) {
+          preferences[name] = result;
+        }
       }
     }
     return preferences;
diff --git a/src/storage/conversion/ChainedConverter.ts b/src/storage/conversion/ChainedConverter.ts
index fda7125ea..0e29679bd 100644
--- a/src/storage/conversion/ChainedConverter.ts
+++ b/src/storage/conversion/ChainedConverter.ts
@@ -174,15 +174,11 @@ export class ChainedConverter extends RepresentationConverter {
     if (!type) {
       throw new BadRequestHttpError('Missing Content-Type header.');
     }
-    let preferences = input.preferences.type;
-    if (!preferences) {
-      throw new BadRequestHttpError('Missing type preferences.');
-    }
-    preferences = cleanPreferences(preferences);
+    const preferences = cleanPreferences(input.preferences.type);
 
     const weight = getTypeWeight(type, preferences);
     if (weight > 0) {
-      this.logger.debug(`No conversion required: ${type} already matches ${Object.keys(input.preferences.type!)}`);
+      this.logger.debug(`No conversion required: ${type} already matches ${Object.keys(preferences)}`);
       return { value: type, weight };
     }
 
diff --git a/test/integration/ServerFetch.test.ts b/test/integration/ServerFetch.test.ts
index e591e2a22..9c1ec26f4 100644
--- a/test/integration/ServerFetch.test.ts
+++ b/test/integration/ServerFetch.test.ts
@@ -36,17 +36,50 @@ describe('A Solid server', (): void => {
     });
   });
 
-  it('can GET results from a container.', async(): Promise<void> => {
+  it('can do a successful HEAD request to a container.', async(): Promise<void> => {
+    const res = await fetch(baseUrl, { method: 'HEAD' });
+    expect(res.status).toBe(200);
+  });
+
+  it('can do a successful HEAD request to a container without accept headers.', async(): Promise<void> => {
+    const res = await fetch(baseUrl, { method: 'HEAD', headers: { accept: '' }});
+    expect(res.status).toBe(200);
+  });
+
+  it('can do a successful HEAD request to a document.', async(): Promise<void> => {
+    const url = `${baseUrl}.acl`;
+    const res = await fetch(url, { method: 'HEAD' });
+    expect(res.status).toBe(200);
+  });
+
+  it('can do a successful HEAD request to a document without accept headers.', async(): Promise<void> => {
+    const url = `${baseUrl}.acl`;
+    const res = await fetch(url, { method: 'HEAD', headers: { accept: '' }});
+    expect(res.status).toBe(200);
+  });
+
+  it('can do a successful GET request to a container.', async(): Promise<void> => {
     const res = await fetch(baseUrl);
     expect(res.status).toBe(200);
   });
 
-  it('can GET results from a resource.', async(): Promise<void> => {
+  it('can do a successful GET request to a container without accept headers.', async(): Promise<void> => {
+    const res = await fetch(baseUrl, { headers: { accept: '' }});
+    expect(res.status).toBe(200);
+  });
+
+  it('can do a successful GET request to a document.', async(): Promise<void> => {
     const url = `${baseUrl}.acl`;
     const res = await fetch(url);
     expect(res.status).toBe(200);
   });
 
+  it('can do a successful GET request to a document without accept headers.', async(): Promise<void> => {
+    const url = `${baseUrl}.acl`;
+    const res = await fetch(url, { headers: { accept: '' }});
+    expect(res.status).toBe(200);
+  });
+
   it('can PUT to containers.', async(): Promise<void> => {
     const url = `${baseUrl}containerPUT/`;
     const res = await fetch(url, {
@@ -96,7 +129,7 @@ describe('A Solid server', (): void => {
     expect(res.headers.get('location')).toBe(`${baseUrl}containerPOST/`);
   });
 
-  it('can POST to create a resource.', async(): Promise<void> => {
+  it('can POST to create a document.', async(): Promise<void> => {
     const res = await fetch(baseUrl, {
       method: 'POST',
       headers: {
@@ -122,7 +155,7 @@ describe('A Solid server', (): void => {
     expect(res.status).toBe(205);
   });
 
-  it('can DELETE resources.', async(): Promise<void> => {
+  it('can DELETE documents.', async(): Promise<void> => {
     const url = `${baseUrl}resourceDELETE`;
     await fetch(url, {
       method: 'PUT',
@@ -135,7 +168,7 @@ describe('A Solid server', (): void => {
     expect(res.status).toBe(205);
   });
 
-  it('can PATCH resources.', async(): Promise<void> => {
+  it('can PATCH documents.', async(): Promise<void> => {
     const url = `${baseUrl}resourcePATCH`;
     await fetch(url, {
       method: 'PUT',
diff --git a/test/unit/ldp/http/AcceptPreferenceParser.test.ts b/test/unit/ldp/http/AcceptPreferenceParser.test.ts
index d6551b444..b852e1c82 100644
--- a/test/unit/ldp/http/AcceptPreferenceParser.test.ts
+++ b/test/unit/ldp/http/AcceptPreferenceParser.test.ts
@@ -14,6 +14,9 @@ describe('An AcceptPreferenceParser', (): void => {
 
   it('returns an empty result if there is no relevant input.', async(): Promise<void> => {
     await expect(preferenceParser.handle({ request })).resolves.toEqual({});
+
+    request.headers = { accept: '' };
+    await expect(preferenceParser.handle({ request })).resolves.toEqual({});
   });
 
   it('parses accept headers.', async(): Promise<void> => {
diff --git a/test/unit/storage/conversion/ChainedConverter.test.ts b/test/unit/storage/conversion/ChainedConverter.test.ts
index 7902ec54a..f194ce9ed 100644
--- a/test/unit/storage/conversion/ChainedConverter.test.ts
+++ b/test/unit/storage/conversion/ChainedConverter.test.ts
@@ -70,10 +70,6 @@ describe('A ChainedConverter', (): void => {
     const converters = [ new DummyConverter({ 'a/a': 1 }, { 'x/x': 1 }) ];
     const converter = new ChainedConverter(converters);
     await expect(converter.canHandle(args)).rejects.toThrow('Missing Content-Type header.');
-
-    args.representation.metadata.contentType = 'a/a';
-    args.preferences = { };
-    await expect(converter.canHandle(args)).rejects.toThrow('Missing type preferences.');
   });
 
   it('errors if no path can be found.', async(): Promise<void> => {
@@ -95,6 +91,20 @@ describe('A ChainedConverter', (): void => {
     expect(result.metadata.contentType).toBe('b/b');
   });
 
+  it('interprets no preferences as */*.', async(): Promise<void> => {
+    const converters = [ new DummyConverter({ 'a/a': 1 }, { 'x/x': 1 }) ];
+    const converter = new ChainedConverter(converters);
+    args.representation.metadata.contentType = 'b/b';
+    args.preferences.type = undefined;
+
+    let result = await converter.handle(args);
+    expect(result.metadata.contentType).toBe('b/b');
+
+    args.preferences.type = { };
+    result = await converter.handle(args);
+    expect(result.metadata.contentType).toBe('b/b');
+  });
+
   it('can find paths of length 1.', async(): Promise<void> => {
     const converters = [ new DummyConverter({ 'a/a': 1 }, { 'x/x': 1 }) ];
     const converter = new ChainedConverter(converters);