Installer cache: fix github release url normalization; Improve test coverage

Author: docd27

src/lib/download.js

@@ -1,13 +1,12 @@
 import * as fsSync from 'fs';
+import {URL} from 'url';
 import {default as createHttpsProxyAgent} from 'https-proxy-agent';
 import {default as TFileCache} from '@derhuerst/http-basic/lib/FileCache.js';
-import * as querystring from 'querystring';
 import {default as request} from '@derhuerst/http-basic';
 import {default as ProgressBar} from 'progress';
 import {default as AdmZip} from 'adm-zip';
-
-import {URL} from 'url';
 import {argQuote, getCachePath, runTool} from './tools.js';
+import { URLSearchParams } from 'node:url';
 
 /**
  * @typedef {import('@derhuerst/http-basic/lib/FileCache.js').default} FileCacheInst
@@ -27,13 +26,18 @@ const requestAsync = (method, url, options = null) => new Promise((resolve, reje
     else resolve(response);
   }));
 
-const normalizeS3Url = (url) => {
-  url = new URL(url);
-  if (url.hostname.slice(-17) !== '.s3.amazonaws.com') return url.href;
-  const query = Array.from(url.searchParams.entries())
-      .filter(([key]) => key.slice(0, 6).toLowerCase() !== 'x-amz-')
-      .reduce((query, [key, val]) => ({...query, [key]: val}), {});
-  url.search = querystring.stringify(query);
+/**
+ * Returns normalized URL with X-Amz params stripped and application/x-www-form-urlencoded format
+ * (spaces %20 are encoded as +)
+ * @param {string} inUrl
+ * @return {string}
+ */
+export const normalizeGithubReleaseURL = (inUrl) => {
+  const url = new URL(inUrl), hostname = url.hostname.toLowerCase();
+  if (hostname === 'github-releases.githubusercontent.com' || hostname.endsWith('.s3.amazonaws.com')) {
+    url.search = new URLSearchParams([...url.searchParams]
+        .filter(([key]) => !key.toLowerCase().startsWith('x-amz-'))).toString();
+  }
   return url.href;
 };
 
@@ -54,7 +58,7 @@ function initHTTP() {
   }
   httpHelpers.cache = new FileCache(getCachePath());
   httpHelpers.cache.getCacheKey = (url) => {
-    return FileCache.prototype.getCacheKey(normalizeS3Url(url));
+    return FileCache.prototype.getCacheKey(normalizeGithubReleaseURL(url));
   };
 }
 

test/tests/glslify.js

@@ -22,7 +22,7 @@ export function glslifyTests(glslOptimize) {
     this.timeout(30000);
     before('uninstall glslify', glslifyUninstall);
     it('should not have glslify available when uninstalled', async function() {
-      assert.isRejected(rollup({
+      await assert.isRejected(rollup({
         input: 'test/fixtures/basic.js',
         plugins: [glslOptimize({optimize: false, glslify: true})],
       }), /glslify could not be found/);

test/tests/index.js

@@ -3,6 +3,7 @@ import chai from 'chai';
 import chaiAsPromised from 'chai-as-promised';
 
 import {installTests} from './install.js';
+import {utilTests} from './util.js';
 import {glslifyTests} from './glslify.js';
 import {shaderTests} from './shader.js';
 import {settings} from '../../settings.js';
@@ -22,6 +23,7 @@ export function runTests(glslOptimize) {
   chai.use(chaiAsPromised);
 
   installTests();
+  utilTests();
   shaderTests(glslOptimize);
   glslifyTests(glslOptimize);
 }

test/tests/util.js

@@ -0,0 +1,47 @@
+import {assert} from 'chai';
+import * as download from '../../src/lib/download.js';
+import * as tools from '../../src/lib/tools.js';
+
+export function utilTests() {
+  describe('Utility functions', function() {
+    it('should correctly normalize a github release URL', function() {
+
+      const baseURL = 'https://github-releases.githubusercontent.com/123456/123456-123456';
+      const baseParams = 'actor_id=0&key_id=0&repo_id=123456&' +
+      'response-content-disposition=attachment%3B+filename%3DglslangValidator&' +
+      'response-content-type=application%2Foctet-stream';
+      const amzParams = 'X-Amz-Algorithm=AWS4-HMAC-SHA256' +
+      '&X-Amz-Credential=A123456%2Fus-east-1%2Fs3%2Faws4_request&' +
+      'X-Amz-Date=123456Z&X-Amz-Expires=123&' +
+      'X-Amz-Signature=123456&' +
+      'X-Amz-SignedHeaders=host';
+      const cleanURL = `${baseURL}?${baseParams}`;
+      const testURL = `${baseURL}?${baseParams}&${amzParams}`;
+
+      assert.strictEqual(download.normalizeGithubReleaseURL(testURL), cleanURL);
+    });
+    it('should warn about missing Khronos binaries', function() {
+      process.env.GLSLANG_VALIDATOR = process.env.GLSLANG_OPTIMIZER = process.env.GLSLANG_CROSS = '/does/not/exist';
+      assert.throws(() => tools.configureTools({}), /Khronos tool binaries could not be found/i);
+      delete process.env.GLSLANG_VALIDATOR; delete process.env.GLSLANG_OPTIMIZER; delete process.env.GLSLANG_CROSS;
+    });
+    it('should output stderr from failing command', async function() {
+      // Mock console output:
+      const consoleOrig = global.console;
+      let outBuf = '';
+      global.console = {
+        log: (...args) => {
+          outBuf += `${args.map(String).join(' ')}\n`;
+        },
+      };
+      global.console.error = global.console.warn = global.console.log;
+
+      await assert.isRejected(tools.runToolBuffered('ping', '.', 'ping', ['-']), /failed/i);
+
+      // Unmock:
+      global.console = consoleOrig;
+
+      assert.isNotEmpty(outBuf, 'stdout/stderr empty');
+    });
+  });
+}