fixed percent-encoding issue in URLs vs. filepaths, see #290

daisy · Oct 26, 2019 · 009142f · 009142f
1 parent 86c1d18
commit 009142f
Show file tree

Hide file tree

Showing 10 changed files with 245 additions and 49 deletions.
diff --git a/package.json b/package.json
@@ -22,6 +22,7 @@
     "jest-mock": "^24.9.0",
     "jest-environment-node": "^24.9.0",
     "extract-zip": "^1.6.7",
+    "node-stream-zip": "^1.8.2",
     "xmldom-alpha": "^0.1.28",
     "xpath": "0.0.24",
     "escape-html": "^1.0.3",

diff --git a/packages/ace-axe-runner-electron/src/init.js b/packages/ace-axe-runner-electron/src/init.js
@@ -22,6 +22,8 @@ const generateSelfSignedData = require('./selfsigned').generateSelfSignedData;
 const isDev = process && process.env && (process.env.NODE_ENV === 'development' || process.env.DEBUG_PROD === 'true');
 const showWindow = false;
 
+const LOG_DEBUG_URLS = process.env.LOG_DEBUG_URLS === "1";
+
 const LOG_DEBUG = false;
 const ACE_LOG_PREFIX = "[ACE-AXE]";
 
@@ -61,7 +63,12 @@ function loadUrl(browserWindow) {
     browserWindow.ace__timeout = undefined;
 
     const options = {}; // { extraHeaders: 'pragma: no-cache\n' };
-    browserWindow.loadURL(`${rootUrl}${browserWindow.ace__currentUrl}?${HTTP_QUERY_PARAM}=${iHttpReq++}`, options);
+    const uareel = `${rootUrl}${browserWindow.ace__currentUrl}?${HTTP_QUERY_PARAM}=${iHttpReq++}`;
+    if (LOG_DEBUG_URLS) {
+        console.log("======>>>>>> URL TO LOAD");
+        console.log(uareel);
+    }
+    browserWindow.loadURL(uareel, options);
 
     const MILLISECONDS_TIMEOUT_INITIAL = 5000; // 5s max to load the window's web contents
     const MILLISECONDS_TIMEOUT_EXTENSION = 35000; // 40s max to load + execute Axe checkers
@@ -339,12 +346,37 @@ function axeRunnerInit(eventEmmitter, CONCURRENT_INSTANCES) {
         const scripts = payload.scripts;
         const scriptContents = payload.scriptContents;
 
+        if (LOG_DEBUG_URLS) {
+            console.log("######## URL 1");
+            console.log(uarel);
+        }
         // windows! file://C:\aa\bb\chapter.xhtml
         const uarelObj = url.parse(uarel.replace(/\\/g, "/"));
         const windowsDrive = uarelObj.hostname ? `${uarelObj.hostname.toUpperCase()}:` : "";
+        if (LOG_DEBUG_URLS) {
+            console.log("######## URL 2");
+            console.log(windowsDrive);
+        }
         const bd = basedir.replace(/\\/g, "/");
+        if (LOG_DEBUG_URLS) {
+            console.log("######## URL 3");
+            console.log(uarelObj.pathname);
+        }
         const full = (windowsDrive + decodeURI(uarelObj.pathname));
-        const httpUrl = full.replace(bd, "");
+        if (LOG_DEBUG_URLS) {
+            console.log("######## URL 4");
+            console.log(full);
+        }
+        let httpUrl = full.replace(bd, "");
+        if (LOG_DEBUG_URLS) {
+            console.log("######## URL 5");
+            console.log(httpUrl);
+        }
+        httpUrl = encodeURI(httpUrl);
+        if (LOG_DEBUG_URLS) {
+            console.log("######## URL 6");
+            console.log(httpUrl);
+        }
 
         if (LOG_DEBUG) console.log(`${ACE_LOG_PREFIX} axeRunner running ... ${basedir} --- ${uarel} => ${httpUrl}`);
 
@@ -553,7 +585,20 @@ function startAxeServer(basedir, scripts, scriptContents) {
             if (req.query[HTTP_QUERY_PARAM]) {
                 if (LOG_DEBUG) console.log(`${ACE_LOG_PREFIX} HTTP intercept ${req.url}`);
 
-                const pn = url.parse(req.url).pathname;
+                if (LOG_DEBUG_URLS) {
+                    console.log(">>>>>>>>>> URL 1");
+                    console.log(req.url);
+                }
+                const ptn = url.parse(req.url).pathname;
+                if (LOG_DEBUG_URLS) {
+                    console.log(">>>>>>>>>> URL 2");
+                    console.log(ptn);
+                }
+                const pn = decodeURI(ptn);
+                if (LOG_DEBUG_URLS) {
+                    console.log(">>>>>>>>>> URL 3");
+                    console.log(pn);
+                }
                 let fileSystemPath = path.join(expressApp.basedir, pn);
                 if (LOG_DEBUG) console.log(`${ACE_LOG_PREFIX} filepath to read: ${fileSystemPath}`);
                 if (!fs.existsSync(fileSystemPath)) {
@@ -585,7 +630,7 @@ function startAxeServer(basedir, scripts, scriptContents) {
             expressApp.use("/", (req, res, next) => {
                 // const url = new URL(`https://fake.org${req.url}`);
                 // const pathname = url.pathname;
-                const pathname = url.parse(req.url).pathname;
+                const pathname = decodeURI(url.parse(req.url).pathname);
 
                 const filePath = path.join(basedir, pathname);
                 if (filePathsExpressStaticNotExist[filePath]) {

diff --git a/packages/ace-core/src/checker/checker-chromium.js b/packages/ace-core/src/checker/checker-chromium.js
@@ -24,9 +24,17 @@ const scripts = [
   require.resolve('../scripts/ace-extraction.js'),
 ];
 
+const LOG_DEBUG_URLS = process.env.LOG_DEBUG_URLS === "1";
+
 async function checkSingle(spineItem, epub, lang, axeRunner) {
   winston.verbose(`- Processing ${spineItem.relpath}`);
   try {
+    if (LOG_DEBUG_URLS) {
+        console.log("....... URL 1");
+        console.log(spineItem.url);
+        console.log(spineItem.filepath);
+        console.log(spineItem.relpath);
+    }
     let url = spineItem.url;
     let ext = path.extname(spineItem.filepath);
 
@@ -39,8 +47,17 @@ async function checkSingle(spineItem, epub, lang, axeRunner) {
       const tmpdir = tmp.dirSync({ unsafeCleanup: true }).name;
       const tmpFile = path.join(tmpdir, `${path.basename(spineItem.filepath, ext)}.xhtml`)
       fs.copySync(spineItem.filepath, tmpFile);
+
+      // does encodeURI() as per https://tools.ietf.org/html/rfc3986#section-3.3 in a nutshell: encodeURI(`file://${tmpFile}`).replace(/[?#]/g, encodeURIComponent)
       url = fileUrl(tmpFile);
-      winston.debug(`checking copied file at ${url}`)
+      // url = "file://" + encodeURI(tmpFile);
+
+      winston.debug(`checking copied file at ${tmpFile}`)
+    }
+
+    if (LOG_DEBUG_URLS) {
+      console.log("....... URL 2");
+      console.log(url);
     }
 
     const scriptContents = [];
@@ -105,18 +122,50 @@ async function checkSingle(spineItem, epub, lang, axeRunner) {
             if (Array.isArray(item.src)) {
               item.src = item.src.map((srcItem) => {
                 if (srcItem.src !== undefined) {
-                  srcItem.path = path.resolve(path.dirname(spineItem.filepath),
-                                              srcItem.src.toString());
+                  if (LOG_DEBUG_URLS) {
+                    console.log("----- ITEMs SRC 1");
+                    console.log(srcItem.src);
+                  }
+                  srcItem.path = path.resolve(path.dirname(spineItem.filepath), decodeURI(srcItem.src.toString()));
+                  if (LOG_DEBUG_URLS) {
+                    console.log("----- ITEMs SRC 2");
+                    console.log(srcItem.path);
+                  }
                   srcItem.src = path.relative(epub.basedir, srcItem.path).replace(/\\/g, "/");
+                  if (LOG_DEBUG_URLS) {
+                    console.log("----- ITEMs SRC 3");
+                    console.log(srcItem.src);
+                  }
                 }
                 return srcItem;
               });
             } else {
-              item.path = path.resolve(path.dirname(spineItem.filepath), item.src.toString());
+              if (LOG_DEBUG_URLS) {
+                console.log("----- ITEM SRC 1");
+                console.log(item.src);
+              }
+              item.path = path.resolve(path.dirname(spineItem.filepath), decodeURI(item.src.toString()));
+              if (LOG_DEBUG_URLS) {
+                console.log("----- ITEM SRC 2");
+                console.log(item.path);
+              }
               item.src = path.relative(epub.basedir, item.path).replace(/\\/g, "/");
+              if (LOG_DEBUG_URLS) {
+                console.log("----- ITEM SRC 3");
+                console.log(item.src);
+              }
             }
             if (item.cfi !== undefined) {
-              item.location = `${spineItem.relpath}#epubcfi(${item.cfi})`;
+              if (LOG_DEBUG_URLS) {
+                console.log("----- CFI 1");
+                console.log(spineItem.relpath);
+                console.log(item.cfi);
+              }
+              item.location = `${encodeURI(spineItem.relpath)}#epubcfi(${encodeURI(item.cfi)})`;
+              if (LOG_DEBUG_URLS) {
+                console.log("----- CFI 2");
+                console.log(item.location);
+              }
               delete item.cfi;
             }
           }
@@ -125,6 +174,7 @@ async function checkSingle(spineItem, epub, lang, axeRunner) {
     }
     return results;
   } catch (err) {
+    console.log(err);
     winston.debug(`Error when running HTML checks: ${err}`);
     throw new Error(`Failed to check Content Document '${spineItem.relpath}'`);
   }

diff --git a/packages/ace-http/src/index.js b/packages/ace-http/src/index.js
@@ -16,6 +16,8 @@ const axeRunner = require('@daisy/ace-axe-runner-puppeteer');
 
 const pkg = require('@daisy/ace-meta/package');
 
+// tmp.setGracefulCleanup();
+
 const UPLOADS = tmp.dirSync({ unsafeCleanup: true }).name;
 const DEFAULTPORT = 8000;
 const DEFAULTHOST = "localhost";

diff --git a/packages/ace-report/src/generate-html-report.js b/packages/ace-report/src/generate-html-report.js
@@ -8,6 +8,8 @@ const winston = require('winston');
 
 const { localize, getCurrentLanguage } = require('./l10n/localize').localizer;
 
+const LOG_DEBUG_URLS = process.env.LOG_DEBUG_URLS === "1";
+
 // generate the html report and return it as a string
 module.exports = function generateHtmlReport(reportData) {
 
@@ -39,14 +41,14 @@ module.exports = function generateHtmlReport(reportData) {
       var filterOptions = "";
       violationFilters[rule].forEach(function(value) {
         // winston.info("######## " + value);
-        const valueDisplay = localize(value, {ignoreMissingKey: true}); // only handles "serious", "moderate", etc. so can be missingKey, such as "EPUB/package.opf", "color-contrast", "metadata-schema-accessibilitysummary" etc. (in which case => fallback to key string)
+        const valueDisplay = rule == "file" ? value : localize(value, {ignoreMissingKey: true}); // only handles "serious", "moderate", etc. so can be missingKey, such as "EPUB/package.opf", "color-contrast", "metadata-schema-accessibilitysummary" etc. (in which case => fallback to key string)
         // use nicer labels for ruleset options
         if (rule == "ruleset") {
           filterOptions += "<option value='" + value + "'>" + rulesetTagLabels[value] + "</option>";
         }
         else {
           filterOptions += "<option value='" + value + "'>" +
-          valueDisplay
+          (rule == "file" ? escape(valueDisplay) : valueDisplay)
           + "</option>";
         }
       });
@@ -66,7 +68,7 @@ module.exports = function generateHtmlReport(reportData) {
         <td><span class='${violation['impact']}'>${valueDisplay}</span></td>
         <td><span class='ruleset'>${rulesetTagLabels[violation['applicableRulesetTag']]}</span></td>
         <td>${violation['rule']}<br/><br/><span class='engine'>${violation['engine']}</span></td>
-        <td><em>\"${violation['fileTitle']}\"</em><br/><br/><code class='location'>${violation['location']}</code>`;
+        <td><em>\"${violation['fileTitle']}\"</em><br/><br/><code class='location'>${escape(violation['location'])}</code>`;
 
         if (violation.html) {
           htmlStr +=`<br/><br/><div class='snippet'>${localize("snippet")}<code>${violation.html.trim()}</code></div>`;
@@ -131,6 +133,36 @@ module.exports = function generateHtmlReport(reportData) {
       return new handlebars.SafeString(valueDisplay);
     });
 
+    handlebars.registerHelper('encodeURI', function(src, options) {
+      // console.log(JSON.stringify(options));
+
+      if (LOG_DEBUG_URLS) {
+        console.log("///// Mustache encodeURI 1");
+        console.log(src);
+      }
+      const url = escape(encodeURI(src));
+      if (LOG_DEBUG_URLS) {
+        console.log("///// Mustache encodeURI 2");
+        console.log(url);
+      }
+      return new handlebars.SafeString(url);
+    });
+
+    handlebars.registerHelper('decodeURI', function(url, options) {
+      // console.log(JSON.stringify(options));
+
+      if (LOG_DEBUG_URLS) {
+        console.log("///// Mustache decodeURI 1");
+        console.log(url);
+      }
+      const src = escape(decodeURI(url));
+      if (LOG_DEBUG_URLS) {
+        console.log("///// Mustache decodeURI 2");
+        console.log(src);
+      }
+      return new handlebars.SafeString(src);
+    });
+
     const content = fs.readFileSync(path.join(__dirname, "./report-template.handlebars")).toString();
     var template = handlebars.compile(content);
     var result = template(reportData);

diff --git a/packages/ace-report/src/report-template.handlebars b/packages/ace-report/src/report-template.handlebars
@@ -413,8 +413,8 @@
               {{#each data.images}}
                 <tr>
                   <td class="image">
-                    <a href="data/{{src}}">
-                      <img src = "data/{{src}}"/>
+                    <a href="data/{{#encodeURI src}}{{/encodeURI}}">
+                      <img src = "data/{{#encodeURI src}}{{/encodeURI}}"/>
                     </a>
                   </td>
 
@@ -436,7 +436,7 @@
                     <td class="missing">{{#localize "na"}}{{/localize}}</td>
                   {{/if}}
 
-                  <td class="location">{{location}}</td>
+                  <td class="location">{{#decodeURI location}}{{/decodeURI}}</td>
 
                   {{#if role}}
                     <td>{{role}}</td>

diff --git a/packages/epub-utils/package.json b/packages/epub-utils/package.json
@@ -19,7 +19,9 @@
   "main": "lib/index.js",
   "dependencies": {
     "extract-zip": "^1.6.7",
+    "file-url": "^3.0.0",
     "fs-extra": "^8.1.0",
+    "node-stream-zip": "^1.8.2",
     "tmp": "^0.1.0",
     "winston": "^3.2.1",
     "xmldom-alpha": "^0.1.28",

diff --git a/packages/epub-utils/src/epub-parse.js b/packages/epub-utils/src/epub-parse.js
@@ -10,6 +10,7 @@
 
 'use strict';
 
+const fileUrl = require('file-url');
 const DOMParser = require('xmldom-alpha').DOMParser;
 const XMLSerializer = require('xmldom-alpha').XMLSerializer;
 const fs = require('fs');
@@ -104,7 +105,7 @@ function addLink(rel, href, link) {
 function parseLinks(doc, select) {
   const result = {};
   select('//opf:link[not(@refines)]', doc).forEach((link) => {
-    addLink(link.getAttribute('rel'), link.getAttribute('href'), result);
+    addLink(link.getAttribute('rel'), decodeURI(link.getAttribute('href')), result);
   });
   return result;
 }
@@ -149,10 +150,14 @@ EpubParser.prototype.parseData = function(packageDocPath, epubDir) {
       const contentType = (manifestItem[0].getAttribute('media-type')||'').trim();
       if (this.contentDocMediaType === contentType) {
         var spineItem = new SpineItem();
-        spineItem.relpath = manifestItem[0].getAttribute('href');
+        spineItem.relpath = decodeURI(manifestItem[0].getAttribute('href'));
         spineItem.filepath = path.join(path.dirname(packageDocPath), spineItem.relpath);
         spineItem.title = this.parseContentDocTitle(spineItem.filepath);
-        spineItem.url = "file://" + spineItem.filepath;
+
+        // does encodeURI() as per https://tools.ietf.org/html/rfc3986#section-3.3 in a nutshell: encodeURI(`file://${tmpFile}`).replace(/[?#]/g, encodeURIComponent)
+        spineItem.url = fileUrl(spineItem.filepath);
+        // spineItem.url = "file://" + encodeURI(spineItem.filepath);
+
         this.contentDocs.push(spineItem);
       } else if (!this.hasSVGContentDocuments && 'image/svg+xml' === contentType) {
         winston.warn('The SVG Content Documents in this EPUB will be ignored.');
@@ -165,7 +170,7 @@ EpubParser.prototype.parseData = function(packageDocPath, epubDir) {
                             + '[contains(concat(" ", normalize-space(@properties), " ")," nav ")]'
                             + '/@href', doc);
   if (navDocRef.length > 0) {
-    const navDocPath = navDocRef[0].nodeValue;
+    const navDocPath = decodeURI(navDocRef[0].nodeValue);
     const navDocFullPath = path.join(path.dirname(packageDocPath), navDocPath);
     this.navDoc = parseNavDoc(navDocFullPath, epubDir);
   }
@@ -196,7 +201,7 @@ EpubParser.prototype.calculatePackageDocPath = function(epubDir) {
   const rootfiles = select('//ocf:rootfile[@media-type="application/oebps-package+xml"]/@full-path', doc);
   // just grab the first one as we're not handling the case of multiple renditions
   if (rootfiles.length > 0) {
-    return (path.join(epubDir, rootfiles[0].nodeValue));
+    return (path.join(epubDir, decodeURI(rootfiles[0].nodeValue)));
   }
   return '';
 }