fix(buildURL): ensure operation is idempotent

imgix · Oct 6, 2020 · 8c6c6f9 · 8c6c6f9
1 parent f33e942
commit 8c6c6f9
Show file tree

Hide file tree

Showing 3 changed files with 116 additions and 12 deletions.
diff --git a/src/imgix-core-js.js b/src/imgix-core-js.js
@@ -107,11 +107,11 @@
     };
 
     ImgixClient.prototype._buildParams = function(params) {
+      var queryParams = [];
       if (this.settings.libraryParam) {
-        params.ixlib = this.settings.libraryParam
+        queryParams.push('ixlib=' + this.settings.libraryParam)
       }
 
-      var queryParams = [];
       var key, val, encodedKey, encodedVal;
       for (key in params) {
         val = params[key];
@@ -174,10 +174,16 @@
         targetWidths = this._generateTargetWidths(widthTolerance, minWidth, maxWidth);
       }
 
+      var key;
+      var queryParams = {};
+      for (key in params) {
+        queryParams[key] = params[key];
+      }
+
       for (var i = 0; i < targetWidths.length; i++) {
         currentWidth = targetWidths[i];
-        params.w = currentWidth;
-        srcset += this.buildURL(path, params) + ' ' + currentWidth + 'w,\n';
+        queryParams.w = currentWidth;
+        srcset += this.buildURL(path, queryParams) + ' ' + currentWidth + 'w,\n';
       }
 
       return srcset.slice(0,-2);
@@ -186,23 +192,29 @@
     ImgixClient.prototype._buildDPRSrcSet = function(path, params, options) {
         var srcset = '';
         var targetRatios = [1, 2, 3, 4, 5];
-        var currentRatio;
         var disableVariableQuality = options.disableVariableQuality || false;
-        var quality = params.q;
+
+        var key;
+        var queryParams = {};
+        for (key in params) {
+          queryParams[key] = params[key];
+        }
+
+        var quality = queryParams.q;
 
         if (!disableVariableQuality) {
           validateVariableQuality(disableVariableQuality);
         }
 
         for (var i = 0; i < targetRatios.length; i++) {
-          currentRatio = targetRatios[i];
-          params.dpr = currentRatio;
+          var currentRatio = targetRatios[i];
+          queryParams.dpr = currentRatio;
 
           if (!disableVariableQuality) {
-            params.q = quality || DPR_QUALITIES[currentRatio];
+            queryParams.q = quality || DPR_QUALITIES[currentRatio];
           }
 
-          srcset += this.buildURL(path, params) + ' ' + currentRatio + 'x,\n'
+          srcset += this.buildURL(path, queryParams) + ' ' + currentRatio + 'x,\n'
         }
 
         return srcset.slice(0,-2);

diff --git a/test/test-buildSrcSet.js b/test/test-buildSrcSet.js
@@ -387,6 +387,21 @@ describe('SrcSet Builder:', function describeSuite() {
                         assert(src.includes(`q=${QUALITY_OVERRIDE}`));
                     }
                 });
+
+                it('should not modify input params and should be idempotent', function testSpec() {
+                    var client = new ImgixClient({ domain: 'test.imgix.net' });
+                    var params = {};
+                    var srcsetOptions = { widths: [100] };
+                    var srcset1 = client.buildSrcSet('', params, srcsetOptions);
+                    var srcset2 = client.buildSrcSet('', params, srcsetOptions);
+
+                    // Show idempotent, ie. calling buildSrcSet produces the same result given
+                    // the same input-parameters.
+                    assert(srcset1 == srcset2);
+
+                    // Assert that the object remains unchanged.
+                    assert(Object.keys(params).length === 0 && params.constructor === Object);
+                });
             });
 
             describe('with an aspect ratio parameter provided', function describeSuite() {

diff --git a/test/test-buildURL.js b/test/test-buildURL.js
@@ -223,11 +223,31 @@ describe('URL Builder:', function describeSuite() {
             assert.equal(expectation, result);
         });
 
+        it('does not modify its input-argument', function testSpec() {
+            var params = {
+                    w: 400,
+                    h: 300
+                },
+                expectation = '?w=400&h=300',
+                result = client._buildParams(params);
+
+            assert.equal(params.w, 400);
+            assert.equal(params.h, 300);
+            assert.equal(expectation, result);
+
+            var emptyParams = {};
+            var emptyResult = client._buildParams(emptyParams);
+
+            // Ensure the result is empty.
+            assert.equal(emptyResult, '');
+            assert(Object.keys(emptyParams).length === 0 && emptyParams.constructor === Object);
+        });
+
         it('includes an `ixlib` param if the `libraryParam` setting is truthy', function testSpec() {
             var params = {
                     w: 400
                 },
-                expectation = '?w=400&ixlib=test',
+                expectation = '?ixlib=test&w=400',
                 result;
 
             client.settings.libraryParam = 'test';
@@ -294,4 +314,61 @@ describe('URL Builder:', function describeSuite() {
             assert.equal(expectation, result);
         });
     });
-});
+
+    describe('Calling buildURL()', function describeSuite() {
+        var client,
+            path = 'images/1.png';
+
+        beforeEach(function setupClient() {
+            client = new ImgixClient({
+            domain: 'test.imgix.net',
+            includeLibraryParam: false
+            });
+        });
+
+        it('is an idempotent operation with empty args', function testSpec() {
+            var result1 = client.buildURL('', {});
+            var result2 = client.buildURL('', {});
+            assert.equal(result1, result2);
+        });
+
+
+        it('is an idempotent operation with args', function testSpec() {
+            var path = '/image/stöked.png';
+            var params = {w: 100};
+            var result1 = client.buildURL(path, params);
+            var result2 = client.buildURL(path, params);
+            var expected = 'https://test.imgix.net/image/st%C3%B6ked.png?w=100'
+            assert.equal(result1, expected);
+            assert.equal(result2, expected)
+        });
+
+
+        it('does not modify empty args', function testSpec() {
+            var path = '';
+            var params = {};
+            var result1 = client.buildURL(path, params);
+            var result2 = client.buildURL(path, params);
+            var expected = 'https://test.imgix.net/';
+
+
+            assert.equal(path, '');
+            assert.equal(expected, result1);
+            assert.equal(expected, result2)
+            assert(Object.keys(params).length === 0 && params.constructor === Object);
+        });
+
+        it('does not modify its args', function testSpec() {
+            var path = 'image/1.png';
+            var params = {w: 100};
+            var result1 = client.buildURL(path, params);
+            var result2 = client.buildURL(path, params);
+            var expected = 'https://test.imgix.net/image/1.png?w=100';
+
+            assert.equal(path, 'image/1.png');
+            assert.equal(result1, result2, expected);
+            assert(Object.keys(params).length === 1 && params.constructor === Object);
+            assert.equal(params.w, 100);
+        });
+    });
+});