Use binary buffers for hash; safe64 encode results. Addresses compiler-explorer#1056

Author: mattgodbolt

Diff for: lib/storage/storage.js

@@ -24,7 +24,7 @@
 // POSSIBILITY OF SUCH DAMAGE.
 
 const logger = require('../logger').logger,
-    hash = require('../utils').getHash;
+    hash = require('../utils').getBinaryHash;
 
 const FILE_HASH_VERSION = 'Compiler Explorer Config Hasher';
 
@@ -34,12 +34,22 @@ class StorageBase {
         this.compilerProps = compilerProps;
     }
 
+    /**
+     * Encode a buffer as a URL-safe string.
+     * @param {Buffer} buffer
+     * @returns {string}
+     */
+    static safe64Encoded(buffer) {
+        return buffer.toString('base64').replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, '');
+    }
+
     handler(req, res) {
         const config = JSON.stringify(req.body.config);
-        const configHash = Buffer.from(hash(config, FILE_HASH_VERSION)).toString('base64');
+        const configHash = StorageBase.safe64Encoded(hash(config, FILE_HASH_VERSION));
         this.findUniqueSubhash(configHash)
             .then(result => {
-                logger.info(`Unique subhash ${result.uniqueSubHash} (${result.alreadyPresent})`);
+                logger.info(`Unique subhash '${result.uniqueSubHash}' ` +
+                    `(${result.alreadyPresent ? "was already present" : "newly-created"})`);
                 if (!result.alreadyPresent) {
                     const storedObject = {
                         prefix: result.prefix,

Diff for: lib/utils.js

@@ -108,19 +108,38 @@ exports.anonymizeIp = function anonymizeIp(ip) {
     }
 };
 
+function objectToHashableString(object) {
+    // See https://stackoverflow.com/questions/899574/which-is-best-to-use-typeof-or-instanceof/6625960#6625960
+    return (typeof(object) === 'string') ? object : JSON.stringify(object);
+}
+
+const DefaultHash = 'Compiler Explorer Default Version 1';
+
+/***
+ * Gets the hash (as a binary buffer) of the given object
+ *
+ * Limitation: object shall not have circular references
+ * @param object {*} Object to get hash from
+ * @param HashVersion {String} Hash "version"
+ * @returns {Buffer} Hash of object
+ */
+exports.getBinaryHash = function getHash(object, HashVersion = DefaultHash) {
+    return crypto.createHmac('sha256', HashVersion)
+        .update(objectToHashableString(object))
+        .digest('buffer');
+};
+
 /***
- * Gets the hash of the given object
+ * Gets the hash (as a hex string) of the given object
  *
  * Limitation: object shall not have circular references
  * @param object {*} Object to get hash from
  * @param HashVersion {String} Hash "version"
  * @returns {String} Hash of object
  */
-exports.getHash = function getHash(object, HashVersion = 'Compiler Explorer Default Version 1') {
-    // See https://stackoverflow.com/questions/899574/which-is-best-to-use-typeof-or-instanceof/6625960#6625960
-    const asString = (typeof(object) === 'string') ? object : JSON.stringify(object);
+exports.getHash = function getHash(object, HashVersion = DefaultHash) {
     return crypto.createHmac('sha256', HashVersion)
-        .update(asString)
+        .update(objectToHashableString(object))
         .digest('hex');
 };
 

Diff for: test/storage/storage-tests.js

@@ -0,0 +1,39 @@
+// Copyright (c) 2018, Compiler Explorer Authors
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are met:
+//
+//     * Redistributions of source code must retain the above copyright notice,
+//       this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above copyright
+//       notice, this list of conditions and the following disclaimer in the
+//       documentation and/or other materials provided with the distribution.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+// ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+// LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+// CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+// SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+// CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+// ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+// POSSIBILITY OF SUCH DAMAGE.
+
+const chai = require('chai'),
+    {StorageBase} = require('../../lib/storage/storage');
+
+chai.should();
+
+describe('Hash tests', () => {
+    it('should never generate invalid characters', () => {
+        for (let i = 0; i < 256; ++i) {
+            const buf = new Buffer([i]);
+            const as64 = StorageBase.safe64Encoded(buf);
+            as64.should.not.contain("/");
+            as64.should.not.contain("+");
+        }
+    });
+});