Fix edge cases of link validation and conversion

docs/devGuide/design/projectStructure.md

@@ -32,7 +32,7 @@ The MarkBind project is developed in a <tooltip content="We follow a monorepo ap
 
 **The core library mainly houses:**
 
-* Functions and libraries used to parse and process MarkBind into usable output are stored in `src`. The architecture described in [Architecture](../design/architecture) is contained here. A brief rundown of what it includes:
+* Functions and libraries used to parse and process MarkBind into usable output are stored in `src`. The architecture described in [Architecture](architecture.md) is contained here. A brief rundown of what it includes:
 
   * Various key functionalities in processing MarkBind syntax into valid html output, stored in `html`. The other part of the content processing flow is found in `variables`, which manages site variables and facillitates the Nunjucks calls.
 

packages/cli/test/functional/test_site/expected/testLinks.html

@@ -214,7 +214,7 @@ <h1 id="autolinks"><span id="autolinks" class="anchor"></span>Autolinks<a class=
         <p><strong>These will be converted:</strong></p>
         <p><a href="https://www.google.com">https://www.google.com</a></p>
         <p><a href="https://markbind.org">https://markbind.org</a></p>
-        <p><a href="/test_site/mailto:foobar@gmail.com">foobar@gmail.com</a></p>
+        <p><a href="mailto:foobar@gmail.com">foobar@gmail.com</a></p>
         <p><strong>These will not be converted:</strong></p>
         <p>google.com</p>
         <p>markbind.org</p>

packages/core/src/html/SiteLinkManager.js

@@ -58,6 +58,10 @@ class SiteLinkManager {
     }
 
     const resourcePath = linkProcessor.getDefaultTagsResourcePath(node);
+    if (!linkProcessor.isIntraLink(resourcePath)) {
+      return 'Should not validate';
+    }
+
     this._addToCollection(resourcePath, cwf);
     return 'Intralink collected to be validated later';
   }

packages/core/src/html/linkProcessor.js

@@ -37,12 +37,24 @@ function getResourcePathFromRoot(rootPath, fullResourcePath) {
   return fsUtil.ensurePosix(path.relative(rootPath, fullResourcePath));
 }
 
+/**
+ * @param {string} resourcePath parsed from the node's relevant attribute
+ * @returns {boolean} whether the resourcePath is a valid intra-site link
+ */
+function isIntraLink(resourcePath) {
+  const MAILTO_OR_TEL_REGEX = /^(?:mailto:|tel:)/i;
+  return resourcePath
+    && !urlUtil.isUrl(resourcePath)
+    && !resourcePath.startsWith('#')
+    && !MAILTO_OR_TEL_REGEX.test(resourcePath);
+}
+
 function _convertRelativeLink(node, cwf, rootPath, baseUrl, resourcePath, linkAttribName) {
-  if (!resourcePath) {
+  if (!isIntraLink(resourcePath)) {
     return;
   }
 
-  if (path.isAbsolute(resourcePath) || urlUtil.isUrl(resourcePath) || resourcePath.startsWith('#')) {
+  if (path.isAbsolute(resourcePath)) {
     // Do not rewrite.
     return;
   }
@@ -148,7 +160,7 @@ function isValidFileAsset(resourcePath, config) {
  * @returns {string} these string return values are for unit testing purposes only
  */
 function validateIntraLink(resourcePath, cwf, config) {
-  if (!resourcePath || urlUtil.isUrl(resourcePath) || resourcePath.startsWith('#')) {
+  if (!isIntraLink(resourcePath)) {
     return 'Not Intralink';
   }
 
@@ -246,4 +258,5 @@ module.exports = {
   convertMdExtToHtmlExt,
   validateIntraLink,
   collectSource,
+  isIntraLink,
 };

packages/core/src/utils/fsUtil.js

@@ -9,7 +9,10 @@ module.exports = {
 
   fileExists(filePath) {
     try {
-      return fs.statSync(filePath).isFile();
+      // use decodeURIComponent to deal with space (%20) in file path, e.g
+      // from docs\images\dev%20diagrams\architecture.png
+      // to docs\images\dev diagrams\architecture.png
+      return fs.statSync(decodeURIComponent(filePath)).isFile();
     } catch (err) {
       return false;
     }

packages/core/test/unit/html/SiteLinkManager.test.js

@@ -27,6 +27,26 @@ test('Test invalid URL link ', () => {
   expect(siteLinkManager.collectIntraLinkToValidate(mockNode, mockCwf)).toEqual(EXPECTED_RESULT);
 });
 
+test('Test mailto URL link', () => {
+  const siteLinkManager = getNewSiteLinkManager();
+  const mockLink = '<a href="mailto:test@example.com">Test</a>';
+  const mockNode = cheerio.parseHTML(mockLink)[0];
+
+  const EXPECTED_RESULT = 'Should not validate';
+
+  expect(siteLinkManager.collectIntraLinkToValidate(mockNode, mockCwf)).toEqual(EXPECTED_RESULT);
+});
+
+test('Test tel URL link', () => {
+  const siteLinkManager = getNewSiteLinkManager();
+  const mockLink = '<a href="tel:999">Test</a>';
+  const mockNode = cheerio.parseHTML(mockLink)[0];
+
+  const EXPECTED_RESULT = 'Should not validate';
+
+  expect(siteLinkManager.collectIntraLinkToValidate(mockNode, mockCwf)).toEqual(EXPECTED_RESULT);
+});
+
 test('Test link for disabled intralink validation', () => {
   const siteLinkManager = getNewSiteLinkManager();
   const mockLink = '<a href="https://markbind.org" no-validation>Test</a>';

packages/core/test/unit/html/linkProcessor.test.js

@@ -11,6 +11,7 @@ const json = {
   './css/main.css': '3',
   './devGuide/index.html': '4',
   './rawFile': '5',
+  './spaced folder/img.png': '6',
 };
 
 fs.vol.fromJSON(json, './src');
@@ -180,6 +181,24 @@ test('Test valid file asset links (png)', () => {
   expect(linkProcessor.validateIntraLink(mockResourcePath, mockCwf, mockConfig)).toEqual(EXPECTED_RESULT);
 });
 
+test('Test valid file asset links (with %20 in the file path)', () => {
+  const mockLink = '<a href="/spaced%20folder/img.png">Test</a>';
+  const mockNode = cheerio.parseHTML(mockLink)[0];
+  const mockResourcePath = linkProcessor.getDefaultTagsResourcePath(mockNode);
+  const EXPECTED_RESULT = 'Intralink is a valid File Asset';
+
+  expect(linkProcessor.validateIntraLink(mockResourcePath, mockCwf, mockConfig)).toEqual(EXPECTED_RESULT);
+});
+
+test('Test valid file asset links (with space in the file path)', () => {
+  const mockLink = '<a href="/spaced folder/img.png">Test</a>';
+  const mockNode = cheerio.parseHTML(mockLink)[0];
+  const mockResourcePath = linkProcessor.getDefaultTagsResourcePath(mockNode);
+  const EXPECTED_RESULT = 'Intralink is a valid File Asset';
+
+  expect(linkProcessor.validateIntraLink(mockResourcePath, mockCwf, mockConfig)).toEqual(EXPECTED_RESULT);
+});
+
 test('Test valid file asset links (css)', () => {
   // should be checked as file asset
   const mockLink = '<link rel="stylesheet" href="/css/main.css">Test</a>';
@@ -201,3 +220,21 @@ test('Test invalid link for non-existent file asset (css)', () => {
 
   expect(linkProcessor.validateIntraLink(mockResourcePath, mockCwf, mockConfig)).toEqual(EXPECTED_RESULT);
 });
+
+test('Test non intralinks (mailto)', () => {
+  const mockLink = '<a href="mailto:foo@bar.com">Test Email</a>';
+  const mockNode = cheerio.parseHTML(mockLink)[0];
+  const mockResourcePath = linkProcessor.getDefaultTagsResourcePath(mockNode);
+  const EXPECTED_RESULT = 'Not Intralink';
+
+  expect(linkProcessor.validateIntraLink(mockResourcePath, mockCwf, mockConfig)).toEqual(EXPECTED_RESULT);
+});
+
+test('Test non intralinks (tel)', () => {
+  const mockLink = '<a href="tel:999">Test Phone</a>';
+  const mockNode = cheerio.parseHTML(mockLink)[0];
+  const mockResourcePath = linkProcessor.getDefaultTagsResourcePath(mockNode);
+  const EXPECTED_RESULT = 'Not Intralink';
+
+  expect(linkProcessor.validateIntraLink(mockResourcePath, mockCwf, mockConfig)).toEqual(EXPECTED_RESULT);
+});