refactor: incorporate requested changes

CommunitySolidServer · Mar 30, 2022 · 5305883 · 5305883
1 parent 0d61d06
commit 5305883
Show file tree

Hide file tree

Showing 5 changed files with 75 additions and 29 deletions.
diff --git a/src/http/input/metadata/LinkRelParser.ts b/src/http/input/metadata/LinkRelParser.ts
@@ -29,9 +29,9 @@ export class LinkRelParser extends MetadataParser {
   }
 
   protected parseLink(linkHeaders: string[], metadata: RepresentationMetadata): void {
-    for (const { link, params } of parseLinkHeader(linkHeaders)) {
-      if (params.rel && this.linkRelMap[params.rel]) {
-        metadata.add(this.linkRelMap[params.rel], namedNode(link));
+    for (const { target, parameters } of parseLinkHeader(linkHeaders)) {
+      if (this.linkRelMap[parameters.rel]) {
+        metadata.add(this.linkRelMap[parameters.rel], namedNode(target));
       }
     }
   }

diff --git a/src/http/input/metadata/PlainJsonLdFilter.ts b/src/http/input/metadata/PlainJsonLdFilter.ts
@@ -1,6 +1,6 @@
 import { getLoggerFor } from '../../../logging/LogUtil';
 import type { HttpRequest } from '../../../server/HttpRequest';
-import { BadRequestHttpError } from '../../../util/errors/BadRequestHttpError';
+import { NotImplementedHttpError } from '../../../util/errors/NotImplementedHttpError';
 import { parseLinkHeader } from '../../../util/HeaderUtil';
 import { JSON_LD } from '../../../util/Vocabularies';
 import type { RepresentationMetadata } from '../../representation/RepresentationMetadata';
@@ -23,13 +23,13 @@ export class PlainJsonLdFilter extends MetadataParser {
     const entries: string[] = Array.isArray(link) ? link : [ link ];
     // Throw error on content-type application/json AND a link header that refers to a JSON-LD context.
     if (contentType === 'application/json' && this.linkHasContextRelation(entries)) {
-      throw new NotImplementedError('JSON-LD is only supported with the application/ld+json content type.');
+      throw new NotImplementedHttpError('JSON-LD is only supported with the application/ld+json content type.');
     }
   }
 
   private linkHasContextRelation(linkHeaders: string[]): boolean {
-    for (const { params } of parseLinkHeader(linkHeaders)) {
-      if (params.rel && params.rel === JSON_LD.context) {
+    for (const { parameters } of parseLinkHeader(linkHeaders)) {
+      if (parameters.rel === JSON_LD.context) {
         return true;
       }
     }

diff --git a/src/util/HeaderUtil.ts b/src/util/HeaderUtil.ts
@@ -108,9 +108,14 @@ export interface ContentType {
   parameters: Record<string, string>;
 }
 
+export interface LinkEntryParameters extends Record<string, string> {
+  /** Required rel properties of Link entry */
+  rel: string;
+}
+
 export interface LinkEntry {
-  link: string;
-  params: Record<string, string>;
+  target: string;
+  parameters: LinkEntryParameters;
 }
 
 // REUSED REGEXES
@@ -503,7 +508,7 @@ export function parseForwarded(headers: IncomingHttpHeaders): Forwarded {
 
 /**
  * Parses the link header(s) and returns an array of LinkEntry objects.
- * @param linkHeaders - Either an array of linkHeaders (can be one header wrapped in an array)
+ * @param linkHeaders - An array of linkHeaders (can be one header wrapped in an array)
  * @returns A LinkEntry array, LinkEntry contains a link and a params Record&lt;string,string&gt;
  */
 export function parseLinkHeader(linkHeaders: string[]): LinkEntry[] {
@@ -516,11 +521,21 @@ export function parseLinkHeader(linkHeaders: string[]): LinkEntry[] {
         logger.warn(`Invalid link header ${part}.`);
         continue;
       }
-      const linkEntry: LinkEntry = { link: link.slice(1, -1), params: {}};
+
+      const params: any = {};
       for (const { name, value } of parseParameters(parameters, replacements)) {
-        linkEntry.params[name] = value;
+        if (name === 'rel' && 'rel' in params) {
+          continue;
+        }
+        params[name] = value;
       }
-      links.push(linkEntry);
+
+      if (!('rel' in params)) {
+        logger.warn(`Invalid link header ${part} contains no 'rel' parameter.`);
+        continue;
+      }
+
+      links.push({ target: link.slice(1, -1), parameters: params });
     }
   }
   return links;

diff --git a/test/unit/http/input/metadata/PlainJsonLdFilter.test.ts b/test/unit/http/input/metadata/PlainJsonLdFilter.test.ts
@@ -1,4 +1,4 @@
-import { BadRequestHttpError } from '../../../../../src';
+import { NotImplementedHttpError } from '../../../../../src';
 import { PlainJsonLdFilter } from '../../../../../src/http/input/metadata/PlainJsonLdFilter';
 import { RepresentationMetadata } from '../../../../../src/http/representation/RepresentationMetadata';
 import type { HttpRequest } from '../../../../../src/server/HttpRequest';
@@ -34,7 +34,7 @@ describe('A PlainJsonLdFilter', (): void => {
   it('throws error when content-type and link header are in conflict.', async(): Promise<void> => {
     request.headers['content-type'] = 'application/json';
     request.headers.link = '<https://json-ld.org/contexts/person.jsonld>; rel="http://www.w3.org/ns/json-ld#context"; type="application/ld+json"';
-    await expect(parser.handle({ request, metadata })).rejects.toThrow(BadRequestHttpError);
+    await expect(parser.handle({ request, metadata })).rejects.toThrow(NotImplementedHttpError);
     expect(metadata.quads()).toHaveLength(0);
   });
 
@@ -44,7 +44,7 @@ describe('A PlainJsonLdFilter', (): void => {
       '<http://test.com/type>; rel="type"',
       '<https://json-ld.org/contexts/person.jsonld>; rel="http://www.w3.org/ns/json-ld#context"; type="application/ld+json"',
     ];
-    await expect(parser.handle({ request, metadata })).rejects.toThrow(BadRequestHttpError);
+    await expect(parser.handle({ request, metadata })).rejects.toThrow(NotImplementedHttpError);
     expect(metadata.quads()).toHaveLength(0);
   });
 

diff --git a/test/unit/util/HeaderUtil.test.ts b/test/unit/util/HeaderUtil.test.ts
@@ -302,8 +302,8 @@ describe('HeaderUtil', (): void => {
       const link = [ '<http://test.com>; rel="myRel"; test="value1"' ];
       expect(parseLinkHeader(link)).toEqual([
         {
-          link: 'http://test.com',
-          params: {
+          target: 'http://test.com',
+          parameters: {
             rel: 'myRel',
             test: 'value1',
           },
@@ -316,15 +316,15 @@ describe('HeaderUtil', (): void => {
       <http://test2.com>; rel="myRel2"; test="value2"` ];
       expect(parseLinkHeader(link)).toEqual([
         {
-          link: 'http://test.com',
-          params: {
+          target: 'http://test.com',
+          parameters: {
             rel: 'myRel',
             test: 'value1',
           },
         },
         {
-          link: 'http://test2.com',
-          params: {
+          target: 'http://test2.com',
+          parameters: {
             rel: 'myRel2',
             test: 'value2',
           },
@@ -339,36 +339,67 @@ describe('HeaderUtil', (): void => {
       ];
       expect(parseLinkHeader(link)).toEqual([
         {
-          link: 'http://test.com',
-          params: {
+          target: 'http://test.com',
+          parameters: {
             rel: 'myRel',
             test: 'value1',
           },
         },
         {
-          link: 'http://test2.com',
-          params: {
+          target: 'http://test2.com',
+          parameters: {
             rel: 'myRel2',
             test: 'value2',
           },
         },
       ]);
     });
 
-    it('ignores invalid links.', (): void => {
+    it('ignores invalid syntax links.', (): void => {
       const link = [
         'http://test.com; rel="myRel"; test="value1"',
         '<http://test2.com>; rel="myRel2"; test="value2"',
       ];
       expect(parseLinkHeader(link)).toEqual([
         {
-          link: 'http://test2.com',
-          params: {
+          target: 'http://test2.com',
+          parameters: {
             rel: 'myRel2',
             test: 'value2',
           },
         },
       ]);
     });
+
+    it('ignores invalid links (no rel parameter).', (): void => {
+      const link = [
+        '<http://test.com>; att="myAtt"; test="value1"',
+        '<http://test2.com>; rel="myRel2"; test="value2"',
+      ];
+      expect(parseLinkHeader(link)).toEqual([
+        {
+          target: 'http://test2.com',
+          parameters: {
+            rel: 'myRel2',
+            test: 'value2',
+          },
+        },
+      ]);
+    });
+
+    it('ignores extra rel parameters.', (): void => {
+      const link = [
+        '<http://test.com>; rel="myRel1"; rel="myRel2"; test="value1"',
+      ];
+      expect(parseLinkHeader(link)).toEqual([
+        {
+          target: 'http://test.com',
+          parameters: {
+            rel: 'myRel1',
+            test: 'value1',
+          },
+        },
+      ]);
+    });
   });
 });