Repository: solid/community-server (ts)

Issue:
IndexConverter prefers HTML too strongly
When running the [mashlib config](https://github.com/solid/community-server-recipes/blob/7bc815a5d005ded2e9db4ce7829af79b37ea00ea/mashlib/config-mashlib.json), and executing

```
curl 'http://localhost:3000/' -H 'Accept: text/html;q=0.1, text/turtle;q=1'
```

the server sends HTML instead of Turtle.

IndexConverter should only work on containers
Currently, https://css.verborgh.org/publications/iswc2020-ldflex.pdf is being converted for no reason.

Related: #651 

Pull request description:
fix: Add conditional parameters to ConstantConverter
Closes #651. Closes #672.

Patch:
diff --git a/config/util/index/example.json b/config/util/index/example.json
index ecce03b9b..e60eeaa8f 100644
--- a/config/util/index/example.json
+++ b/config/util/index/example.json
@@ -8,8 +8,11 @@
       ],
       "@id": "urn:solid-server:default:IndexConverter",
       "@type": "ConstantConverter",
-      "ConstantConverter:_contentType": "text/html",
-      "ConstantConverter:_filePath": "./node_modules/mashlib/dist/databrowser.html"
+      "contentType": "text/html",
+      "filePath": "./node_modules/mashlib/dist/databrowser.html",
+      "options_container": true,
+      "options_document": false,
+      "options_minQuality": 1
     }
   ]
 }
diff --git a/src/storage/conversion/ConstantConverter.ts b/src/storage/conversion/ConstantConverter.ts
index 501fbcde8..040d5279a 100644
--- a/src/storage/conversion/ConstantConverter.ts
+++ b/src/storage/conversion/ConstantConverter.ts
@@ -2,10 +2,29 @@ import { createReadStream } from 'fs';
 import { BasicRepresentation } from '../../ldp/representation/BasicRepresentation';
 import type { Representation } from '../../ldp/representation/Representation';
 import { NotImplementedHttpError } from '../../util/errors/NotImplementedHttpError';
-import { matchesMediaType, matchesMediaPreferences } from './ConversionUtil';
+import { isContainerIdentifier } from '../../util/PathUtil';
+import { matchesMediaType, getTypeWeight, cleanPreferences } from './ConversionUtil';
 import { RepresentationConverter } from './RepresentationConverter';
 import type { RepresentationConverterArgs } from './RepresentationConverter';
 
+/**
+ * Extra options for the ConstantConverter.
+ */
+export interface ConstantConverterOptions {
+  /**
+   * Whether this should trigger on containers.
+   */
+  container?: boolean;
+  /**
+   * Whether this should trigger on documents.
+   */
+  document?: boolean;
+  /**
+   * The minimum requested quality/preference before this should trigger.
+   */
+  minQuality?: number;
+}
+
 /**
  * A {@link RepresentationConverter} that ensures
  * a representation for a certain content type is available.
@@ -15,30 +34,53 @@ import type { RepresentationConverterArgs } from './RepresentationConverter';
  *
  * This can for example be used to serve an index.html file,
  * which could then interactively load another representation.
+ *
+ * Options default to the most permissive values when not defined.
  */
 export class ConstantConverter extends RepresentationConverter {
   private readonly filePath: string;
   private readonly contentType: string;
+  private readonly options: Required<ConstantConverterOptions>;
 
   /**
    * Creates a new constant converter.
    *
    * @param filePath - The path to the constant representation.
    * @param contentType - The content type of the constant representation.
+   * @param options - Extra options for the converter.
    */
-  public constructor(filePath: string, contentType: string) {
+  public constructor(filePath: string, contentType: string, options: ConstantConverterOptions = {}) {
     super();
     this.filePath = filePath;
     this.contentType = contentType;
+    this.options = {
+      container: options.container ?? true,
+      document: options.document ?? true,
+      minQuality: options.minQuality ?? 0,
+    };
   }
 
-  public async canHandle({ preferences, representation }: RepresentationConverterArgs): Promise<void> {
+  public async canHandle({ identifier, preferences, representation }: RepresentationConverterArgs): Promise<void> {
     // Do not replace the representation if there is no preference for our content type
     if (!preferences.type) {
       throw new NotImplementedHttpError('No content type preferences specified');
     }
-    if (!matchesMediaPreferences(this.contentType, { ...preferences.type, '*/*': 0 })) {
+
+    // Do not replace the representation of unsupported resource types
+    const isContainer = isContainerIdentifier(identifier);
+    if (isContainer && !this.options.container) {
+      throw new NotImplementedHttpError('Containers are not supported');
+    }
+    if (!isContainer && !this.options.document) {
+      throw new NotImplementedHttpError('Documents are not supported');
+    }
+
+    // Do not replace the representation if the preference weight is too low
+    const quality = getTypeWeight(this.contentType, cleanPreferences({ ...preferences.type, '*/*': 0 }));
+    if (quality === 0) {
       throw new NotImplementedHttpError(`No preference for ${this.contentType}`);
+    } else if (quality < this.options.minQuality) {
+      throw new NotImplementedHttpError(`Preference is lower than the specified minimum quality`);
     }
 
     // Do not replace the representation if it already has our content type


Test patch:
diff --git a/test/unit/storage/conversion/ConstantConverter.test.ts b/test/unit/storage/conversion/ConstantConverter.test.ts
index f71585358..27e81ff78 100644
--- a/test/unit/storage/conversion/ConstantConverter.test.ts
+++ b/test/unit/storage/conversion/ConstantConverter.test.ts
@@ -1,22 +1,47 @@
 import fs from 'fs';
 import arrayifyStream from 'arrayify-stream';
 import { RepresentationMetadata } from '../../../../src/ldp/representation/RepresentationMetadata';
+import type { ConstantConverterOptions } from '../../../../src/storage/conversion/ConstantConverter';
 import { ConstantConverter } from '../../../../src/storage/conversion/ConstantConverter';
 
 const createReadStream = jest.spyOn(fs, 'createReadStream').mockReturnValue('file contents' as any);
 
 describe('A ConstantConverter', (): void => {
   const identifier = { path: 'identifier' };
+  let options: ConstantConverterOptions;
+  let converter: ConstantConverter;
 
-  const converter = new ConstantConverter('abc/def/index.html', 'text/html');
+  beforeEach(async(): Promise<void> => {
+    options = { container: true, document: true, minQuality: 1 };
+    converter = new ConstantConverter('abc/def/index.html', 'text/html', options);
+  });
 
   it('does not support requests without content type preferences.', async(): Promise<void> => {
     const preferences = {};
     const representation = {} as any;
     const args = { identifier, representation, preferences };
 
-    await expect(converter.canHandle(args)).rejects
-      .toThrow('No content type preferences specified');
+    await expect(converter.canHandle(args)).rejects.toThrow('No content type preferences specified');
+  });
+
+  it('does not support requests targeting documents if disabled in the options.', async(): Promise<void> => {
+    const preferences = { type: { 'text/html': 1 }};
+    const representation = { metadata: new RepresentationMetadata() } as any;
+    const args = { identifier, representation, preferences };
+
+    converter = new ConstantConverter('abc/def/index.html', 'text/html', { document: false });
+
+    await expect(converter.canHandle(args)).rejects.toThrow('Documents are not supported');
+  });
+
+  it('does not support requests targeting containers if disabled in the options.', async(): Promise<void> => {
+    const preferences = { type: { 'text/html': 1 }};
+    const representation = { metadata: new RepresentationMetadata() } as any;
+    const args = { identifier: { path: 'container/' }, representation, preferences };
+
+    converter = new ConstantConverter('abc/def/index.html', 'text/html', { container: false });
+
+    await expect(converter.canHandle(args)).rejects.toThrow('Containers are not supported');
   });
 
   it('does not support requests without matching content type preference.', async(): Promise<void> => {
@@ -24,8 +49,15 @@ describe('A ConstantConverter', (): void => {
     const representation = {} as any;
     const args = { identifier, representation, preferences };
 
-    await expect(converter.canHandle(args)).rejects
-      .toThrow('No preference for text/html');
+    await expect(converter.canHandle(args)).rejects.toThrow('No preference for text/html');
+  });
+
+  it('does not support requests not reaching the minimum preference quality.', async(): Promise<void> => {
+    const preferences = { type: { 'text/html': 0.9 }};
+    const representation = {} as any;
+    const args = { identifier, representation, preferences };
+
+    await expect(converter.canHandle(args)).rejects.toThrow('Preference is lower than the specified minimum quality');
   });
 
   it('does not support representations that are already in the right format.', async(): Promise<void> => {
@@ -34,8 +66,7 @@ describe('A ConstantConverter', (): void => {
     const representation = { metadata } as any;
     const args = { identifier, representation, preferences };
 
-    await expect(converter.canHandle(args)).rejects
-      .toThrow('Representation is already text/html');
+    await expect(converter.canHandle(args)).rejects.toThrow('Representation is already text/html');
   });
 
   it('supports representations with an unknown content type.', async(): Promise<void> => {
@@ -64,4 +95,18 @@ describe('A ConstantConverter', (): void => {
     expect(converted.metadata.contentType).toBe('text/html');
     expect(await arrayifyStream(converted.data)).toEqual([ 'file contents' ]);
   });
+
+  it('defaults to the most permissive options.', async(): Promise<void> => {
+    const preferences = { type: { 'text/html': 0.1 }};
+    const metadata = new RepresentationMetadata();
+    const representation = { metadata } as any;
+    const args = { identifier, representation, preferences };
+
+    converter = new ConstantConverter('abc/def/index.html', 'text/html');
+
+    await expect(converter.canHandle(args)).resolves.toBeUndefined();
+
+    args.identifier = { path: 'container/' };
+    await expect(converter.canHandle(args)).resolves.toBeUndefined();
+  });
 });