From 59600b07f88027d0bd4ac641919e990ca8016642 Mon Sep 17 00:00:00 2001
From: Joachim Van Herwegen <joachimvh@gmail.com>
Date: Fri, 5 Feb 2021 17:15:03 +0100
Subject: [PATCH] fix: Fixed bug with empty Accept headers and internal/quads
 bodies

In case the Accept header was empty and the store returned an internal/quads
resource, our  mechanism to prevent internal data from being returned
was ignored.
---
 src/storage/conversion/ConversionUtil.ts          |  5 ++++-
 src/storage/conversion/IfNeededConverter.ts       | 10 +++-------
 .../storage/conversion/ConversionUtil.test.ts     | 14 +++++++++++++-
 .../storage/conversion/IfNeededConverter.test.ts  | 15 +++++++++++++++
 4 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/src/storage/conversion/ConversionUtil.ts b/src/storage/conversion/ConversionUtil.ts
index 26f9e353b..0c8faa264 100644
--- a/src/storage/conversion/ConversionUtil.ts
+++ b/src/storage/conversion/ConversionUtil.ts
@@ -10,6 +10,8 @@ import { NotImplementedHttpError } from '../../util/errors/NotImplementedHttpErr
  * Since more specific media ranges override less specific ones,
  * this will be ignored if there is a specific internal type preference.
  *
+ * This should be called even if there are no preferredTypes since this also filters out internal types.
+ *
  * @param preferredTypes - Preferences for output type.
  * @param availableTypes - Media types to compare to the preferences.
  *
@@ -24,8 +26,9 @@ string[] {
   const preferred = { ...preferredTypes };
   if (Object.keys(preferredTypes).length === 0) {
     preferred['*/*'] = 1;
+  }
   // Prevent accidental use of internal types
-  } else if (!(INTERNAL_ALL in preferred)) {
+  if (!(INTERNAL_ALL in preferred)) {
     preferred[INTERNAL_ALL] = 0;
   }
 
diff --git a/src/storage/conversion/IfNeededConverter.ts b/src/storage/conversion/IfNeededConverter.ts
index ffcdcfd9f..6586504c6 100644
--- a/src/storage/conversion/IfNeededConverter.ts
+++ b/src/storage/conversion/IfNeededConverter.ts
@@ -36,11 +36,6 @@ export class IfNeededConverter extends RepresentationConverter {
   }
 
   protected needsConversion({ identifier, representation, preferences }: RepresentationConverterArgs): boolean {
-    // No conversion needed if no preferences were specified
-    if (!preferences.type) {
-      return false;
-    }
-
     // No conversion is needed if there are any matches for the provided content type
     const { contentType } = representation.metadata;
     if (!contentType) {
@@ -49,8 +44,9 @@ export class IfNeededConverter extends RepresentationConverter {
     const noMatchingMediaType = !hasMatchingMediaTypes(preferences.type, { [contentType]: 1 });
     if (noMatchingMediaType) {
       this.logger.debug(`Conversion needed for ${identifier
-        .path} from ${representation.metadata.contentType} to satisfy ${Object.entries(preferences.type)
-        .map(([ value, weight ]): string => `${value};q=${weight}`).join(', ')}`);
+        .path} from ${representation.metadata.contentType} to satisfy ${!preferences.type ?
+        '""' :
+        Object.entries(preferences.type).map(([ value, weight ]): string => `${value};q=${weight}`).join(', ')}`);
     }
     return noMatchingMediaType;
   }
diff --git a/test/unit/storage/conversion/ConversionUtil.test.ts b/test/unit/storage/conversion/ConversionUtil.test.ts
index 79b6c5501..f98fe0903 100644
--- a/test/unit/storage/conversion/ConversionUtil.test.ts
+++ b/test/unit/storage/conversion/ConversionUtil.test.ts
@@ -1,5 +1,6 @@
 import type { ValuePreferences } from '../../../../src/ldp/representation/RepresentationPreferences';
 import {
+  hasMatchingMediaTypes,
   matchesMediaType,
   matchingMediaTypes,
   supportsMediaTypeConversion,
@@ -7,7 +8,7 @@ import {
 import { InternalServerError } from '../../../../src/util/errors/InternalServerError';
 
 describe('ConversionUtil', (): void => {
-  describe('supportsMediaTypeConversion', (): void => {
+  describe('#supportsMediaTypeConversion', (): void => {
     it('requires preferences.', async(): Promise<void> => {
       expect((): any => supportsMediaTypeConversion()).toThrow();
     });
@@ -82,6 +83,17 @@ describe('ConversionUtil', (): void => {
     });
   });
 
+  describe('#hasMatchingMediatypes', (): void => {
+    it('returns false if there are no matches.', async(): Promise<void> => {
+      expect(hasMatchingMediaTypes()).toEqual(false);
+    });
+
+    it('returns true if there are matches.', async(): Promise<void> => {
+      const preferences: ValuePreferences = { 'a/x': 1, 'b/x': 0.5, 'c/x': 0 };
+      expect(hasMatchingMediaTypes(preferences, { 'b/x': 1, 'c/x': 1 })).toEqual(true);
+    });
+  });
+
   describe('#matchesMediaType', (): void => {
     it('matches all possible media types.', async(): Promise<void> => {
       expect(matchesMediaType('*/*', 'text/turtle')).toBeTruthy();
diff --git a/test/unit/storage/conversion/IfNeededConverter.test.ts b/test/unit/storage/conversion/IfNeededConverter.test.ts
index 518f8feb0..1004cecce 100644
--- a/test/unit/storage/conversion/IfNeededConverter.test.ts
+++ b/test/unit/storage/conversion/IfNeededConverter.test.ts
@@ -38,6 +38,21 @@ describe('An IfNeededConverter', (): void => {
     expect(innerConverter.handleSafe).toHaveBeenCalledTimes(0);
   });
 
+  it('performs conversion when there are no preferences but the content-type is internal.', async(): Promise<void> => {
+    const preferences = {};
+    const internalRepresentation = {
+      metadata: { contentType: 'internal/quads' },
+    } as any;
+    const args = { identifier, representation: internalRepresentation, preferences };
+
+    await expect(converter.handleSafe(args)).resolves.toBe(converted);
+
+    expect(innerConverter.canHandle).toHaveBeenCalledTimes(0);
+    expect(innerConverter.handle).toHaveBeenCalledTimes(0);
+    expect(innerConverter.handleSafe).toHaveBeenCalledTimes(1);
+    expect(innerConverter.handleSafe).toHaveBeenCalledWith(args);
+  });
+
   it('errors if no content type is specified on the representation.', async(): Promise<void> => {
     const preferences = { type: { 'text/turtle': 1 }};
     const args = { identifier, representation: { metadata: {}} as any, preferences };