From 5f88939d6b42c9d1640ea073b76ff27ff4c80376 Mon Sep 17 00:00:00 2001
From: Elwin Schmitz <eschmitz@redcross.nl>
Date: Thu, 16 Mar 2023 12:26:08 +0100
Subject: [PATCH] Let relative/internal links in HTML/Markdown-content NOT open
 new window/tab

---
 src/app/app.module.ts                         | 12 ++--
 .../q-a-set/q-a-set.component.spec.ts         | 65 +++++++++++++++++--
 src/environments/environment.ts               |  5 +-
 3 files changed, 71 insertions(+), 11 deletions(-)

diff --git a/src/app/app.module.ts b/src/app/app.module.ts
index c36f405a..e42410a0 100644
--- a/src/app/app.module.ts
+++ b/src/app/app.module.ts
@@ -24,15 +24,17 @@ export function markedOptionsFactory(): MarkedOptions {
   };
 
   renderer.link = (href: string, title: string, text: string): string => {
-    return `<a href="${href}" target="_blank" rel="noopener noreferrer" ${
-      title ? ` title="${title}"` : ''
-    }">${text}</a>`;
+    const isExternal = !href.startsWith('/');
+    return `<a href="${href}"
+     ${isExternal ? `target="_blank" rel="external noopener noreferrer"` : ''}
+     ${title ? ` title="${title}"` : ''}
+     >${text}</a>`;
   };
 
   renderer.html = (html: string): string => {
     return html.replaceAll(
-      /(?<raw_a_link>href=)/gi,
-      ` target="_blank" rel="noopener noreferer" $1`,
+      /(?<raw_a_href>href=[\s"']*(?:http|\/\/))/gi,
+      ` target="_blank" rel="external noopener noreferrer" $<raw_a_href>`,
     );
   };
 
diff --git a/src/app/components/q-a-set/q-a-set.component.spec.ts b/src/app/components/q-a-set/q-a-set.component.spec.ts
index fd82ced4..230ef969 100644
--- a/src/app/components/q-a-set/q-a-set.component.spec.ts
+++ b/src/app/components/q-a-set/q-a-set.component.spec.ts
@@ -112,7 +112,7 @@ describe('QASetComponent', () => {
     expect(timeElements[0].textContent).toContain(testDateFormatted);
   });
 
-  it('should contain clickable links, when answer contains plain-text (absolute) URL', () => {
+  it('should contain clickable links that open a new window, when answer contains plain-text (absolute) URL', () => {
     const testQASet = mockQASet1;
     testQASet.answer =
       'Answer with URL: www.example.org Test-link: https://example.net/';
@@ -120,12 +120,69 @@ describe('QASetComponent', () => {
 
     fixture.detectChanges();
 
-    const linkElements = fixture.nativeElement.querySelectorAll(
-      'a[rel="noopener noreferrer"]',
-    );
+    const linkElements = fixture.nativeElement.querySelectorAll('a[href]');
 
     expect(linkElements.length).toBe(2);
+    linkElements.forEach((link: HTMLAnchorElement) => {
+      expect(link.href).toMatch(/^http/);
+      expect(link.target).toBe('_blank');
+      expect(link.rel).toContain('external');
+      expect(link.rel).toContain('noreferrer');
+      expect(link.rel).toContain('noopener');
+    });
+
     expect(linkElements[0].href).toContain('http://www.example.org/');
     expect(linkElements[1].href).toContain('https://example.net/');
   });
+
+  it('should contain (safe) clickable links that open a new window, when answer contains HTML-links', () => {
+    const testQASet = mockQASet1;
+    testQASet.answer =
+      'Answer with HTML-link: <a href="http://evil.example.net/">https://innocent.example.org</a>' +
+      'Evil link: <a \nhref=//evil.example.net>link</a>' +
+      'Evil link: <a\thref= //evil.example.net>link</a>' +
+      'Evil link: <a href=//evil.example.net target="_self">link</a>' +
+      // 'Evil link: <a target="_self" href="//evil.example.net">link</a>' + // This circumvents the addition of target attribute :(
+      // 'Evil link: <a rel="opener" href="//evil.example.net">link</a>' + // This circumvents the addition of rel attribute :(
+      '';
+    component.qaSet = testQASet;
+
+    fixture.detectChanges();
+
+    const linkElements = fixture.nativeElement.querySelectorAll('a[href]');
+
+    expect(linkElements.length).toBe(4);
+    linkElements.forEach((link: HTMLAnchorElement) => {
+      expect(link.target).toBe('_blank');
+      expect(link.rel).toContain('external');
+      expect(link.rel).toContain('noreferrer');
+      expect(link.rel).toContain('noopener');
+    });
+  });
+
+  it('should contain clickable links that open in same window, when answer contains local/relative links', () => {
+    const testQASet = mockQASet1;
+    testQASet.answer =
+      'Answer with:\n' +
+      'a local HTML-link: <a href= /test>local test</a> \n' +
+      'a local Markdown-link: [local test](/test) \n' +
+      'an external HTML-link: <a href= https://example.org/test>external test</a> \n' +
+      'an external Markdown-link: [external test](https://example.org/test) \n';
+    component.qaSet = testQASet;
+
+    fixture.detectChanges();
+
+    const linkElements = fixture.nativeElement.querySelectorAll('a[href]');
+
+    expect(linkElements.length).toBe(4);
+
+    expect(linkElements[0].rel).toBe('');
+    expect(linkElements[0].target).toBe('');
+    expect(linkElements[1].rel).toBe('');
+    expect(linkElements[1].target).toBe('');
+    expect(linkElements[2].rel).toContain('external');
+    expect(linkElements[2].target).toBe('_blank');
+    expect(linkElements[3].rel).toContain('external');
+    expect(linkElements[3].target).toBe('_blank');
+  });
 });
diff --git a/src/environments/environment.ts b/src/environments/environment.ts
index 140a96a5..a1d9be63 100644
--- a/src/environments/environment.ts
+++ b/src/environments/environment.ts
@@ -20,9 +20,10 @@ export const environment = {
   mainPageIntroduction:
     'Intro test-content: \n\n' +
     '<b>Image:</b> <img src="/favicon.ico" alt=""/> \n\n' +
-    ' <b>Link:</b> <a href="https://github.com/rodekruis/helpful-information" target="_blank" rel="noopener noreferrer">helpful-information on GitHub</a> \n\n ' +
+    ' <b>Link:</b> <a href="https://github.com/rodekruis/helpful-information">helpful-information on GitHub</a> \n\n ' +
+    ' <b>Internal:</b> <a href="/test">Test</a> \n\n ' +
     'Markdown content: \n\n' +
-    '_inline_ **styles** and [links](https://example.org) : \n\n' +
+    '_inline_ **styles** and [external links](https://example.org) or [internal links](/test) \n\n' +
     '### Level 3 headings down only? \n\n' +
     'Section content... \n\n' +
     '--- \n\n' +