From fc5d0efc86cfceeba8784c2d9ed87ba1374fc352 Mon Sep 17 00:00:00 2001
From: this-is-shivamsingh <shivam.si@browserstack.com>
Date: Thu, 1 Aug 2024 01:08:19 +0530
Subject: [PATCH 01/13] fix: decoding manually for reserved characters

---
 packages/core/src/config.js         |  5 +++
 packages/core/src/discovery.js      |  1 +
 packages/core/src/snapshot.js       |  2 +-
 packages/core/src/utils.js          | 26 ++++++++++++-
 packages/core/test/snapshot.test.js | 57 +++++++++++++++++++++++++++++
 snapshot.yml                        |  3 ++
 6 files changed, 92 insertions(+), 2 deletions(-)
 create mode 100644 snapshot.yml

diff --git a/packages/core/src/config.js b/packages/core/src/config.js
index 48d50b11b..a0267205b 100644
--- a/packages/core/src/config.js
+++ b/packages/core/src/config.js
@@ -50,6 +50,10 @@ export const configSchema = {
         type: 'boolean',
         default: true
       },
+      modifySnapshotUrl: {
+        type: 'boolean',
+        default: true
+      },
       disableShadowDOM: {
         type: 'boolean',
         default: false
@@ -279,6 +283,7 @@ export const snapshotSchema = {
         percyCSS: { $ref: '/config/snapshot#/properties/percyCSS' },
         enableJavaScript: { $ref: '/config/snapshot#/properties/enableJavaScript' },
         cliEnableJavaScript: { $ref: '/config/snapshot#/properties/cliEnableJavaScript' },
+        modifySnapshotUrl: { $ref: '/config/snapshot#/properties/modifySnapshotUrl' },
         disableShadowDOM: { $ref: '/config/snapshot#/properties/disableShadowDOM' },
         domTransformation: { $ref: '/config/snapshot#/properties/domTransformation' },
         enableLayout: { $ref: '/config/snapshot#/properties/enableLayout' },
diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js
index fdc0d247c..e3ef80402 100644
--- a/packages/core/src/discovery.js
+++ b/packages/core/src/discovery.js
@@ -43,6 +43,7 @@ function debugSnapshotOptions(snapshot) {
   debugProp(snapshot, 'enableJavaScript');
   debugProp(snapshot, 'cliEnableJavaScript');
   debugProp(snapshot, 'disableShadowDOM');
+  debugProp(snapshot, 'modifySnapshotUrl');
   debugProp(snapshot, 'enableLayout');
   debugProp(snapshot, 'domTransformation');
   debugProp(snapshot, 'reshuffleInvalidTags');
diff --git a/packages/core/src/snapshot.js b/packages/core/src/snapshot.js
index 37a841d0e..e0e99d8b4 100644
--- a/packages/core/src/snapshot.js
+++ b/packages/core/src/snapshot.js
@@ -102,7 +102,7 @@ function mapSnapshotOptions(snapshots, context) {
     // transform snapshot URL shorthand into an object
     if (typeof snapshot === 'string') snapshot = { url: snapshot };
 
-    validateAndFixSnapshotUrl(snapshot);
+    if (snapshot.modifySnapshotUrl !== false && process.env.PERCY_MODIFY_SNAPSHOT_URL !== 'false') validateAndFixSnapshotUrl(snapshot);
 
     let url = validURL(snapshot.url, context?.baseUrl);
     // normalize the snapshot url and use it for the default name
diff --git a/packages/core/src/utils.js b/packages/core/src/utils.js
index ef4f9f638..9a5eafc88 100644
--- a/packages/core/src/utils.js
+++ b/packages/core/src/utils.js
@@ -375,6 +375,29 @@ export function base64encode(content) {
     .toString('base64');
 }
 
+const RESERVED_CHARACTERS = {
+  '%3A': ':',
+  '%23': '#',
+  '%24': '$',
+  '%26': '&',
+  '%2B': '+',
+  '%2C': ',',
+  '%2F': '/',
+  '%3B': ';',
+  '%3D': '=',
+  '%3F': '?',
+  '%40': '@'
+};
+
+function _replaceReservedCharacters(url) {
+  let result = url;
+  for (let [key, value] of Object.entries(RESERVED_CHARACTERS)) {
+    let regex = new RegExp(key, 'g');
+    result = result.replace(regex, value);
+  }
+  return result;
+}
+
 // This function replaces invalid character that are not the
 // part of valid URI syntax with there correct encoded value.
 // Also, if a character is a part of valid URI syntax, those characters
@@ -388,7 +411,8 @@ export function decodeAndEncodeURLWithLogging(url, logger, options = {}) {
   // correctly.
   const { meta, shouldLogWarning, warningMessage } = options;
   try {
-    let decodedURL = decodeURI(url); // This can throw error, so handle it will trycatch
+    let decodedURL = _replaceReservedCharacters(url);
+    decodedURL = decodeURI(decodedURL);
     let encodedURL = encodeURI(decodedURL);
     return encodedURL;
   } catch (error) {
diff --git a/packages/core/test/snapshot.test.js b/packages/core/test/snapshot.test.js
index 913facf46..fc2ea3bbf 100644
--- a/packages/core/test/snapshot.test.js
+++ b/packages/core/test/snapshot.test.js
@@ -180,6 +180,63 @@ describe('Snapshot', () => {
         sharedExpectModifiedSnapshotURL('http://localhost:8000/advanced-search/cf1a5848-f658-4939-be11-dctwo-%7B%20abc%20%5Bdbd%5D%20%7D/2253');
       });
 
+      it('do not double encode reserved URI character(%2F) in snapshot url', async () => {
+        let partiallyEncodedURL = 'http://localhost:8000/https%2F/abc[123].html';
+        await percy.snapshot([
+          { url: partiallyEncodedURL }
+        ]);
+
+        sharedExpectModifiedSnapshotURL('http://localhost:8000/https//abc%5B123%5D.html');
+      });
+
+      describe('does not modifies snapshot url', () => {
+        let partiallyEncodedURL = 'http://localhost:8000/https%2F/abc[123].html';
+        const sharedExpect = () => {
+          expect(logger.stderr).toEqual(jasmine.arrayContaining([
+            '[percy:core:snapshot] Received snapshot: /https%2F/abc[123].html',
+            '[percy:core:snapshot] - url: http://localhost:8000/https%2F/abc[123].html'
+          ]));
+
+          expect(logger.stderr).not.toEqual(jasmine.arrayContaining([
+            '[percy:core:snapshot] Snapshot URL modified to: http://localhost:8000/https//abc%5B123%5D.html'
+          ]));
+        };
+
+        it('when modifySnapshotUrl set to false', async () => {
+          await percy.snapshot([
+            { url: partiallyEncodedURL, modifySnapshotUrl: true }
+          ]);
+
+          sharedExpect();
+        });
+
+        describe('with PERCY_MODIFY_SNAPSHOT_URL=false', () => {
+          beforeEach(() => {
+            process.env.PERCY_MODIFY_SNAPSHOT_URL = 'false';
+          });
+
+          afterEach(() => {
+            process.env.PERCY_MODIFY_SNAPSHOT_URL = 'true';
+          });
+
+          it('uses original snapshot url', async () => {
+            await percy.snapshot([
+              { url: partiallyEncodedURL }
+            ]);
+
+            sharedExpect();
+          });
+
+          fit('uses original snapshot url if modifySnapshotUrl = false ', async () => {
+            await percy.snapshot([
+              { url: partiallyEncodedURL, modifySnapshotUrl: false }
+            ]);
+
+            sharedExpect();
+          });
+        });
+      });
+
       it('warns for invalid characters that can not get encoded properly', async () => {
         let givenURL = 'http://localhost:8000/advanced-search/cf1a5848%-f658-4939-be11-dct';
         await percy.snapshot([
diff --git a/snapshot.yml b/snapshot.yml
new file mode 100644
index 000000000..9e7c65f9a
--- /dev/null
+++ b/snapshot.yml
@@ -0,0 +1,3 @@
+name: 'some screenshot'
+url: http://localhost:9000/https%3A/abc[123].html
+waitForTimeout: 3000
\ No newline at end of file

From f9d8a4cb214249ca3d59628cbf005da2e42b29bc Mon Sep 17 00:00:00 2001
From: this-is-shivamsingh <shivam.si@browserstack.com>
Date: Thu, 1 Aug 2024 01:11:16 +0530
Subject: [PATCH 02/13] chore: typo fix

---
 packages/core/test/snapshot.test.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/core/test/snapshot.test.js b/packages/core/test/snapshot.test.js
index fc2ea3bbf..76cd5fd52 100644
--- a/packages/core/test/snapshot.test.js
+++ b/packages/core/test/snapshot.test.js
@@ -227,7 +227,7 @@ describe('Snapshot', () => {
             sharedExpect();
           });
 
-          fit('uses original snapshot url if modifySnapshotUrl = false ', async () => {
+          it('uses original snapshot url if modifySnapshotUrl = false ', async () => {
             await percy.snapshot([
               { url: partiallyEncodedURL, modifySnapshotUrl: false }
             ]);

From f9f377a32c26b8ce37f6d0f61c9ba599d85e3472 Mon Sep 17 00:00:00 2001
From: this-is-shivamsingh <shivam.si@browserstack.com>
Date: Thu, 1 Aug 2024 01:26:35 +0530
Subject: [PATCH 03/13] test: test fix on percy.test.js and snapshot.test.js

---
 packages/core/test/percy.test.js    | 3 ++-
 packages/core/test/snapshot.test.js | 4 ++--
 packages/core/types/index.d.ts      | 1 +
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/packages/core/test/percy.test.js b/packages/core/test/percy.test.js
index 7632f54e1..8e5af0579 100644
--- a/packages/core/test/percy.test.js
+++ b/packages/core/test/percy.test.js
@@ -74,7 +74,8 @@ describe('Percy', () => {
       percyCSS: '',
       enableJavaScript: false,
       disableShadowDOM: false,
-      cliEnableJavaScript: true
+      cliEnableJavaScript: true,
+      modifySnapshotUrl: true
     });
   });
 
diff --git a/packages/core/test/snapshot.test.js b/packages/core/test/snapshot.test.js
index 76cd5fd52..36659a398 100644
--- a/packages/core/test/snapshot.test.js
+++ b/packages/core/test/snapshot.test.js
@@ -202,9 +202,9 @@ describe('Snapshot', () => {
           ]));
         };
 
-        it('when modifySnapshotUrl set to false', async () => {
+        fit('when modifySnapshotUrl set to false', async () => {
           await percy.snapshot([
-            { url: partiallyEncodedURL, modifySnapshotUrl: true }
+            { url: partiallyEncodedURL, modifySnapshotUrl: false }
           ]);
 
           sharedExpect();
diff --git a/packages/core/types/index.d.ts b/packages/core/types/index.d.ts
index fecedd2e7..e004915f6 100644
--- a/packages/core/types/index.d.ts
+++ b/packages/core/types/index.d.ts
@@ -45,6 +45,7 @@ interface CommonSnapshotOptions {
   percyCSS?: string;
   enableJavaScript?: boolean;
   disableShadowDOM?: boolean;
+  modifySnapshotUrl?: boolean;
   enableLayout?: boolean;
   domTransformation?: string;
   reshuffleInvalidTags?: boolean;

From 609493a467c99fb5cdaf48717bfb8aeee4bd4ffa Mon Sep 17 00:00:00 2001
From: this-is-shivamsingh <shivam.si@browserstack.com>
Date: Thu, 1 Aug 2024 01:27:45 +0530
Subject: [PATCH 04/13] chore: revert file

---
 snapshot.yml | 3 ---
 1 file changed, 3 deletions(-)
 delete mode 100644 snapshot.yml

diff --git a/snapshot.yml b/snapshot.yml
deleted file mode 100644
index 9e7c65f9a..000000000
--- a/snapshot.yml
+++ /dev/null
@@ -1,3 +0,0 @@
-name: 'some screenshot'
-url: http://localhost:9000/https%3A/abc[123].html
-waitForTimeout: 3000
\ No newline at end of file

From 34d40dfd84a505be71aeb2d6e7acfd307eaa2190 Mon Sep 17 00:00:00 2001
From: this-is-shivamsingh <shivam.si@browserstack.com>
Date: Thu, 1 Aug 2024 01:31:58 +0530
Subject: [PATCH 05/13] test: test fix

---
 packages/core/test/snapshot.test.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/core/test/snapshot.test.js b/packages/core/test/snapshot.test.js
index 36659a398..898e7467b 100644
--- a/packages/core/test/snapshot.test.js
+++ b/packages/core/test/snapshot.test.js
@@ -202,7 +202,7 @@ describe('Snapshot', () => {
           ]));
         };
 
-        fit('when modifySnapshotUrl set to false', async () => {
+        it('when modifySnapshotUrl set to false', async () => {
           await percy.snapshot([
             { url: partiallyEncodedURL, modifySnapshotUrl: false }
           ]);

From dc0aa9ae680a0cecb881909bf830f494ce985b47 Mon Sep 17 00:00:00 2001
From: this-is-shivamsingh <shivam.si@browserstack.com>
Date: Thu, 1 Aug 2024 18:21:19 +0530
Subject: [PATCH 06/13] revert: Removing modifySnapshotUrl from snaphshot
 config

---
 packages/core/src/config.js         |  5 -----
 packages/core/src/discovery.js      |  1 -
 packages/core/src/snapshot.js       |  2 +-
 packages/core/test/percy.test.js    |  3 +--
 packages/core/test/snapshot.test.js | 16 ----------------
 packages/core/types/index.d.ts      |  1 -
 6 files changed, 2 insertions(+), 26 deletions(-)

diff --git a/packages/core/src/config.js b/packages/core/src/config.js
index a0267205b..48d50b11b 100644
--- a/packages/core/src/config.js
+++ b/packages/core/src/config.js
@@ -50,10 +50,6 @@ export const configSchema = {
         type: 'boolean',
         default: true
       },
-      modifySnapshotUrl: {
-        type: 'boolean',
-        default: true
-      },
       disableShadowDOM: {
         type: 'boolean',
         default: false
@@ -283,7 +279,6 @@ export const snapshotSchema = {
         percyCSS: { $ref: '/config/snapshot#/properties/percyCSS' },
         enableJavaScript: { $ref: '/config/snapshot#/properties/enableJavaScript' },
         cliEnableJavaScript: { $ref: '/config/snapshot#/properties/cliEnableJavaScript' },
-        modifySnapshotUrl: { $ref: '/config/snapshot#/properties/modifySnapshotUrl' },
         disableShadowDOM: { $ref: '/config/snapshot#/properties/disableShadowDOM' },
         domTransformation: { $ref: '/config/snapshot#/properties/domTransformation' },
         enableLayout: { $ref: '/config/snapshot#/properties/enableLayout' },
diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js
index e3ef80402..fdc0d247c 100644
--- a/packages/core/src/discovery.js
+++ b/packages/core/src/discovery.js
@@ -43,7 +43,6 @@ function debugSnapshotOptions(snapshot) {
   debugProp(snapshot, 'enableJavaScript');
   debugProp(snapshot, 'cliEnableJavaScript');
   debugProp(snapshot, 'disableShadowDOM');
-  debugProp(snapshot, 'modifySnapshotUrl');
   debugProp(snapshot, 'enableLayout');
   debugProp(snapshot, 'domTransformation');
   debugProp(snapshot, 'reshuffleInvalidTags');
diff --git a/packages/core/src/snapshot.js b/packages/core/src/snapshot.js
index e0e99d8b4..1f04077d3 100644
--- a/packages/core/src/snapshot.js
+++ b/packages/core/src/snapshot.js
@@ -102,7 +102,7 @@ function mapSnapshotOptions(snapshots, context) {
     // transform snapshot URL shorthand into an object
     if (typeof snapshot === 'string') snapshot = { url: snapshot };
 
-    if (snapshot.modifySnapshotUrl !== false && process.env.PERCY_MODIFY_SNAPSHOT_URL !== 'false') validateAndFixSnapshotUrl(snapshot);
+    if (process.env.PERCY_MODIFY_SNAPSHOT_URL !== 'false') validateAndFixSnapshotUrl(snapshot);
 
     let url = validURL(snapshot.url, context?.baseUrl);
     // normalize the snapshot url and use it for the default name
diff --git a/packages/core/test/percy.test.js b/packages/core/test/percy.test.js
index 8e5af0579..7632f54e1 100644
--- a/packages/core/test/percy.test.js
+++ b/packages/core/test/percy.test.js
@@ -74,8 +74,7 @@ describe('Percy', () => {
       percyCSS: '',
       enableJavaScript: false,
       disableShadowDOM: false,
-      cliEnableJavaScript: true,
-      modifySnapshotUrl: true
+      cliEnableJavaScript: true
     });
   });
 
diff --git a/packages/core/test/snapshot.test.js b/packages/core/test/snapshot.test.js
index 898e7467b..c6b21c6e0 100644
--- a/packages/core/test/snapshot.test.js
+++ b/packages/core/test/snapshot.test.js
@@ -202,14 +202,6 @@ describe('Snapshot', () => {
           ]));
         };
 
-        it('when modifySnapshotUrl set to false', async () => {
-          await percy.snapshot([
-            { url: partiallyEncodedURL, modifySnapshotUrl: false }
-          ]);
-
-          sharedExpect();
-        });
-
         describe('with PERCY_MODIFY_SNAPSHOT_URL=false', () => {
           beforeEach(() => {
             process.env.PERCY_MODIFY_SNAPSHOT_URL = 'false';
@@ -226,14 +218,6 @@ describe('Snapshot', () => {
 
             sharedExpect();
           });
-
-          it('uses original snapshot url if modifySnapshotUrl = false ', async () => {
-            await percy.snapshot([
-              { url: partiallyEncodedURL, modifySnapshotUrl: false }
-            ]);
-
-            sharedExpect();
-          });
         });
       });
 
diff --git a/packages/core/types/index.d.ts b/packages/core/types/index.d.ts
index e004915f6..fecedd2e7 100644
--- a/packages/core/types/index.d.ts
+++ b/packages/core/types/index.d.ts
@@ -45,7 +45,6 @@ interface CommonSnapshotOptions {
   percyCSS?: string;
   enableJavaScript?: boolean;
   disableShadowDOM?: boolean;
-  modifySnapshotUrl?: boolean;
   enableLayout?: boolean;
   domTransformation?: string;
   reshuffleInvalidTags?: boolean;

From f485e59ca7e8504708e3696f9d968fa5310c08df Mon Sep 17 00:00:00 2001
From: this-is-shivamsingh <shivam.si@browserstack.com>
Date: Thu, 1 Aug 2024 18:27:32 +0530
Subject: [PATCH 07/13] test: added 1 more test for multiple reserved character

---
 packages/core/test/snapshot.test.js | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/packages/core/test/snapshot.test.js b/packages/core/test/snapshot.test.js
index c6b21c6e0..a8e380987 100644
--- a/packages/core/test/snapshot.test.js
+++ b/packages/core/test/snapshot.test.js
@@ -189,6 +189,15 @@ describe('Snapshot', () => {
         sharedExpectModifiedSnapshotURL('http://localhost:8000/https//abc%5B123%5D.html');
       });
 
+      it('do not double encode if multiple reserved URI characters in snapshot url', async () => {
+        let partiallyEncodedURL = 'http://localhost:8000/https%2F/abc[123]%3A%3F_abc.html';
+        await percy.snapshot([
+          { url: partiallyEncodedURL }
+        ]);
+
+        sharedExpectModifiedSnapshotURL('http://localhost:8000/https//abc%5B123%5D:?_abc.html');
+      });
+
       describe('does not modifies snapshot url', () => {
         let partiallyEncodedURL = 'http://localhost:8000/https%2F/abc[123].html';
         const sharedExpect = () => {

From 0a2b5eec9b5118f0b1049d4dbe3b06c90d9bf917 Mon Sep 17 00:00:00 2001
From: this-is-shivamsingh <shivam.si@browserstack.com>
Date: Tue, 20 Aug 2024 18:31:06 +0530
Subject: [PATCH 08/13] feat: preserving reserved character from encoding

---
 packages/core/src/utils.js | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/packages/core/src/utils.js b/packages/core/src/utils.js
index 9a5eafc88..6b2b5a908 100644
--- a/packages/core/src/utils.js
+++ b/packages/core/src/utils.js
@@ -391,7 +391,23 @@ const RESERVED_CHARACTERS = {
 
 function _replaceReservedCharacters(url) {
   let result = url;
+  let matchedPattern = {};
+  let placeHolderCount = 0;
   for (let [key, value] of Object.entries(RESERVED_CHARACTERS)) {
+    let regex = new RegExp(key, 'g');
+    if (regex.test(result)) {
+      let placeholder = `__PERCY_PLACEHOLDER_${placeHolderCount}__`
+      result = result.replace(regex, placeholder);
+      matchedPattern[placeholder] = key;
+      placeHolderCount++;
+    }
+  }
+  return {result, matchedPattern};
+}
+
+function _decodeMatchedPattern(matchedPattern, url) {
+  let result = url;
+  for (let [key, value] of Object.entries(matchedPattern)) {
     let regex = new RegExp(key, 'g');
     result = result.replace(regex, value);
   }
@@ -409,11 +425,17 @@ export function decodeAndEncodeURLWithLogging(url, logger, options = {}) {
   // will encode those characters again. Therefore decodeURI once helps is decoding
   // partially encoded URL and then after encoding it again, full URL get encoded
   // correctly.
-  const { meta, shouldLogWarning, warningMessage } = options;
+  const {
+    meta,
+    shouldLogWarning,
+    warningMessage
+  } = options;
   try {
-    let decodedURL = _replaceReservedCharacters(url);
-    decodedURL = decodeURI(decodedURL);
+    let {result, matchedPattern} = _replaceReservedCharacters(url);
+    console.log("--> gojo matched ", matchedPattern, result);
+    let decodedURL = decodeURI(result);
     let encodedURL = encodeURI(decodedURL);
+    encodedURL = _decodeMatchedPattern(matchedPattern, encodedURL);
     return encodedURL;
   } catch (error) {
     logger.debug(error, meta);

From e2b3ae31b76249a2f382900e5245525897fef164 Mon Sep 17 00:00:00 2001
From: this-is-shivamsingh <shivam.si@browserstack.com>
Date: Thu, 22 Aug 2024 03:50:20 +0530
Subject: [PATCH 09/13] test: test fix + only show warning for network logs if
 snapshot.url != request.url

---
 packages/core/src/discovery.js      |  2 +-
 packages/core/src/network.js        | 10 +++++++++-
 packages/core/src/snapshot.js       | 18 +++++++++++++++++-
 packages/core/src/utils.js          | 17 ++++++++---------
 packages/core/test/snapshot.test.js |  9 ++++-----
 5 files changed, 39 insertions(+), 17 deletions(-)

diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js
index fdc0d247c..dc57ffad0 100644
--- a/packages/core/src/discovery.js
+++ b/packages/core/src/discovery.js
@@ -342,7 +342,7 @@ export function createDiscoveryQueue(percy) {
             authorization: snapshot.discovery.authorization,
             userAgent: snapshot.discovery.userAgent,
             captureMockedServiceWorker: snapshot.discovery.captureMockedServiceWorker,
-            meta: snapshot.meta,
+            meta: { ...snapshot.meta, snapshotURL: snapshot.url },
 
             // enable network inteception
             intercept: {
diff --git a/packages/core/src/network.js b/packages/core/src/network.js
index 4598e024e..6eb38827e 100644
--- a/packages/core/src/network.js
+++ b/packages/core/src/network.js
@@ -1,7 +1,7 @@
 import { request as makeRequest } from '@percy/client/utils';
 import logger from '@percy/logger';
 import mime from 'mime-types';
-import { DefaultMap, createResource, hostnameMatches, normalizeURL, waitFor } from './utils.js';
+import { DefaultMap, createResource, hostnameMatches, normalizeURL, waitFor, decodeAndEncodeURLWithLogging } from './utils.js';
 
 const MAX_RESOURCE_SIZE = 25 * (1024 ** 2); // 25MB
 const ALLOWED_STATUSES = [200, 201, 301, 302, 304, 307, 308];
@@ -207,6 +207,14 @@ export class Network {
     // do not handle data urls
     if (request.url.startsWith('data:')) return;
 
+    // Browsers handle URL encoding leniently, but invalid characters can break tools like Jackproxy.
+    // This code checks for issues such as `%` and leading spaces and warns the user accordingly.
+    decodeAndEncodeURLWithLogging(request.url, this.log, {
+      meta: { ...this.meta, url: request.url },
+      shouldLogWarning: request.url !== this.meta.snapshotURL,
+      warningMessage: `An invalid URL was detected for url: ${request.url} - the snapshot may fail on Percy. Please verify that your asset URL is valid.`
+    });
+
     if (this.intercept) {
       this.#pending.set(requestId, event);
       if (this.captureMockedServiceWorker) {
diff --git a/packages/core/src/snapshot.js b/packages/core/src/snapshot.js
index e00b71a6f..b687fa175 100644
--- a/packages/core/src/snapshot.js
+++ b/packages/core/src/snapshot.js
@@ -7,7 +7,8 @@ import {
   request,
   hostnameMatches,
   yieldTo,
-  snapshotLogName
+  snapshotLogName,
+  decodeAndEncodeURLWithLogging
 } from './utils.js';
 import { JobData } from './wait-for-job.js';
 
@@ -24,6 +25,21 @@ function validURL(url, base) {
   }
 }
 
+function validateAndFixSnapshotUrl(snapshot) {
+  let log = logger('core:snapshot');
+  // encoding snapshot url, if contians invalid URI characters/syntax
+  let modifiedURL = decodeAndEncodeURLWithLogging(snapshot.url, log, {
+    meta: { snapshot: { name: snapshot.name || snapshot.url } },
+    shouldLogWarning: true,
+    warningMessage: `Invalid URL detected for url: ${snapshot.url} - the snapshot may fail on Percy. Please confirm that your website URL is valid.`
+  });
+
+  if (modifiedURL !== snapshot.url) {
+    log.debug(`Snapshot URL modified to: ${modifiedURL}`);
+    snapshot.url = modifiedURL;
+  }
+}
+
 // used to deserialize regular expression strings
 const RE_REGEXP = /^\/(.+)\/(\w+)?$/;
 
diff --git a/packages/core/src/utils.js b/packages/core/src/utils.js
index 6b2b5a908..81fc06e38 100644
--- a/packages/core/src/utils.js
+++ b/packages/core/src/utils.js
@@ -389,23 +389,23 @@ const RESERVED_CHARACTERS = {
   '%40': '@'
 };
 
-function _replaceReservedCharacters(url) {
+function _replaceReservedCharactersWithPlaceholder(url) {
   let result = url;
   let matchedPattern = {};
   let placeHolderCount = 0;
-  for (let [key, value] of Object.entries(RESERVED_CHARACTERS)) {
+  for (let key of Object.keys(RESERVED_CHARACTERS)) {
     let regex = new RegExp(key, 'g');
     if (regex.test(result)) {
-      let placeholder = `__PERCY_PLACEHOLDER_${placeHolderCount}__`
+      let placeholder = `__PERCY_PLACEHOLDER_${placeHolderCount}__`;
       result = result.replace(regex, placeholder);
       matchedPattern[placeholder] = key;
       placeHolderCount++;
     }
   }
-  return {result, matchedPattern};
+  return { url: result, matchedPattern };
 }
 
-function _decodeMatchedPattern(matchedPattern, url) {
+function _replacePlaceholdersWithReservedCharacters(matchedPattern, url) {
   let result = url;
   for (let [key, value] of Object.entries(matchedPattern)) {
     let regex = new RegExp(key, 'g');
@@ -431,11 +431,10 @@ export function decodeAndEncodeURLWithLogging(url, logger, options = {}) {
     warningMessage
   } = options;
   try {
-    let {result, matchedPattern} = _replaceReservedCharacters(url);
-    console.log("--> gojo matched ", matchedPattern, result);
-    let decodedURL = decodeURI(result);
+    let { url: placeholderURL, matchedPattern } = _replaceReservedCharactersWithPlaceholder(url);
+    let decodedURL = decodeURI(placeholderURL);
     let encodedURL = encodeURI(decodedURL);
-    encodedURL = _decodeMatchedPattern(matchedPattern, encodedURL);
+    encodedURL = _replacePlaceholdersWithReservedCharacters(matchedPattern, encodedURL);
     return encodedURL;
   } catch (error) {
     logger.debug(error, meta);
diff --git a/packages/core/test/snapshot.test.js b/packages/core/test/snapshot.test.js
index a8e380987..456b9afa3 100644
--- a/packages/core/test/snapshot.test.js
+++ b/packages/core/test/snapshot.test.js
@@ -180,13 +180,13 @@ describe('Snapshot', () => {
         sharedExpectModifiedSnapshotURL('http://localhost:8000/advanced-search/cf1a5848-f658-4939-be11-dctwo-%7B%20abc%20%5Bdbd%5D%20%7D/2253');
       });
 
-      it('do not double encode reserved URI character(%2F) in snapshot url', async () => {
+      it('do not encode reserved URI character(%2F) in snapshot url', async () => {
         let partiallyEncodedURL = 'http://localhost:8000/https%2F/abc[123].html';
         await percy.snapshot([
           { url: partiallyEncodedURL }
         ]);
 
-        sharedExpectModifiedSnapshotURL('http://localhost:8000/https//abc%5B123%5D.html');
+        sharedExpectModifiedSnapshotURL('http://localhost:8000/https%2F/abc%5B123%5D.html');
       });
 
       it('do not double encode if multiple reserved URI characters in snapshot url', async () => {
@@ -195,7 +195,7 @@ describe('Snapshot', () => {
           { url: partiallyEncodedURL }
         ]);
 
-        sharedExpectModifiedSnapshotURL('http://localhost:8000/https//abc%5B123%5D:?_abc.html');
+        sharedExpectModifiedSnapshotURL('http://localhost:8000/https%2F/abc%5B123%5D%3A%3F_abc.html');
       });
 
       describe('does not modifies snapshot url', () => {
@@ -237,8 +237,7 @@ describe('Snapshot', () => {
         ]);
 
         expect(logger.stderr).toEqual(jasmine.arrayContaining([
-          `[percy:core:snapshot] Invalid URL detected for url: ${givenURL} - the snapshot may fail on Percy. Please confirm that your website URL is valid.`,
-          `[percy:core:discovery] An invalid URL was detected for url: ${givenURL} - the snapshot may fail on Percy. Please verify that your asset URL is valid.`
+          `[percy:core:snapshot] Invalid URL detected for url: ${givenURL} - the snapshot may fail on Percy. Please confirm that your website URL is valid.`
         ]));
       });
 

From 8bcf155d2e5815236d6b29d6b8d19bf8aa2c323b Mon Sep 17 00:00:00 2001
From: this-is-shivamsingh <shivam.si@browserstack.com>
Date: Thu, 22 Aug 2024 12:52:29 +0530
Subject: [PATCH 10/13] chore: null check for passing tests

---
 packages/core/src/network.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/core/src/network.js b/packages/core/src/network.js
index 6eb38827e..b498027c1 100644
--- a/packages/core/src/network.js
+++ b/packages/core/src/network.js
@@ -211,7 +211,7 @@ export class Network {
     // This code checks for issues such as `%` and leading spaces and warns the user accordingly.
     decodeAndEncodeURLWithLogging(request.url, this.log, {
       meta: { ...this.meta, url: request.url },
-      shouldLogWarning: request.url !== this.meta.snapshotURL,
+      shouldLogWarning: request.url !== this.meta?.snapshotURL,
       warningMessage: `An invalid URL was detected for url: ${request.url} - the snapshot may fail on Percy. Please verify that your asset URL is valid.`
     });
 

From ca4273fdf2ce4f5a45c6a59894245b2b7d348a59 Mon Sep 17 00:00:00 2001
From: this-is-shivamsingh <shivam.si@browserstack.com>
Date: Thu, 22 Aug 2024 13:04:35 +0530
Subject: [PATCH 11/13] test: test coverage

---
 packages/core/test/discovery.test.js | 45 +++++++++++++++++++++++++-
 packages/core/test/utils.test.js     | 48 ++++++++++++++++++++++++++++
 2 files changed, 92 insertions(+), 1 deletion(-)
 create mode 100644 packages/core/test/utils.test.js

diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js
index b181add91..3b80dfccf 100644
--- a/packages/core/test/discovery.test.js
+++ b/packages/core/test/discovery.test.js
@@ -647,6 +647,49 @@ describe('Discovery', () => {
     ]));
   });
 
+  it('debug error log only for invalid network url', async () => {
+    let err = new Error('Some Invalid Error');
+    spyOn(global, 'decodeURI').and.callFake((url) => {
+      if (url === 'http://localhost:8000/style.css') {
+        throw err;
+      }
+      return url;
+    });
+
+    percy.loglevel('debug');
+    await percy.snapshot({
+      name: 'test snapshot',
+      url: 'http://localhost:8000',
+      domSnapshot: testDOM
+    });
+
+    expect(logger.stderr).toEqual(jasmine.arrayContaining([
+      '[percy:core:page] Navigate to: http://localhost:8000/',
+      '[percy:core:discovery] Handling request: http://localhost:8000/',
+      '[percy:core:discovery] - Serving root resource',
+      `[percy:core:discovery] ${err.stack}`,
+      '[percy:core:discovery] Handling request: http://localhost:8000/style.css',
+      '[percy:core:discovery] Handling request: http://localhost:8000/img.gif'
+    ]));
+  });
+
+  it('detect invalid network url', async () => {
+    percy.loglevel('debug');
+    await percy.snapshot({
+      name: 'test snapshot',
+      url: 'http://localhost:8000',
+      domSnapshot: testDOM.replace('style.css', 'http://localhost:404/%style.css')
+    });
+
+    expect(logger.stderr).toEqual(jasmine.arrayContaining([
+      '[percy:core:discovery] Handling request: http://localhost:8000/',
+      '[percy:core:discovery] - Serving root resource',
+      '[percy:core:discovery] An invalid URL was detected for url: http://localhost:404/%style.css - the snapshot may fail on Percy. Please verify that your asset URL is valid.',
+      '[percy:core:discovery] Handling request: http://localhost:404/%style.css',
+      '[percy:core:discovery] Handling request: http://localhost:8000/img.gif'
+    ]));
+  });
+
   it('allows setting a custom discovery user-agent', async () => {
     let userAgent;
 
@@ -1389,7 +1432,7 @@ describe('Discovery', () => {
   });
 
   describe('Dom serialisation', () => {
-    it('should contain valid dom serialization values', async () => {
+    fit('should contain valid dom serialization values', async () => {
       const page = await percy.browser.page();
       await page.goto('http://localhost:8000');
       await page.insertPercyDom();
diff --git a/packages/core/test/utils.test.js b/packages/core/test/utils.test.js
new file mode 100644
index 000000000..2a43d7c8f
--- /dev/null
+++ b/packages/core/test/utils.test.js
@@ -0,0 +1,48 @@
+import { decodeAndEncodeURLWithLogging } from '../src/utils.js';
+import { logger } from './helpers/index.js';
+import percyLogger from '@percy/logger';
+
+describe('utils', () => {
+  let log;
+  beforeEach(async () => {
+    log = percyLogger();
+    logger.reset(true);
+    await logger.mock({ level: 'debug' });
+  });
+
+  describe('decodeAndEncodeURLWithLogging', () => {
+    it('does not warn invalid url when options params is null', async () => {
+      decodeAndEncodeURLWithLogging('https://abc.com/test%abc', log);
+      expect(logger.stderr).toEqual([]);
+    });
+
+    it('does not warn invalid url when shouldLogWarning = false', async () => {
+      decodeAndEncodeURLWithLogging('https://abc.com/test%abc', log,
+        {
+          shouldLogWarning: false
+        });
+
+      expect(logger.stderr).toEqual([]);
+    });
+
+    it('returns modified url', async () => {
+      const url = decodeAndEncodeURLWithLogging('https://abc.com/test[ab]c', log,
+        {
+          shouldLogWarning: false
+        });
+      expect(logger.stderr).toEqual([]);
+      expect(url).toEqual('https://abc.com/test%5Bab%5Dc');
+    });
+
+    it('warns if invalid url when shouldLogWarning = true', async () => {
+      decodeAndEncodeURLWithLogging(
+        'https://abc.com/test%abc',
+        log,
+        {
+          shouldLogWarning: true,
+          warningMessage: 'some Warning Message'
+        });
+      expect(logger.stderr).toEqual(['[percy] some Warning Message']);
+    });
+  });
+});

From 7d4c7accff1995be1781af8d1bd14b6687989ff4 Mon Sep 17 00:00:00 2001
From: this-is-shivamsingh <shivam.si@browserstack.com>
Date: Thu, 22 Aug 2024 13:24:19 +0530
Subject: [PATCH 12/13] test: test fix

---
 packages/core/test/discovery.test.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js
index 3b80dfccf..318a590f8 100644
--- a/packages/core/test/discovery.test.js
+++ b/packages/core/test/discovery.test.js
@@ -1432,7 +1432,7 @@ describe('Discovery', () => {
   });
 
   describe('Dom serialisation', () => {
-    fit('should contain valid dom serialization values', async () => {
+    it('should contain valid dom serialization values', async () => {
       const page = await percy.browser.page();
       await page.goto('http://localhost:8000');
       await page.insertPercyDom();

From c92176cf8f1c97088dec5c655f780fee70e068d9 Mon Sep 17 00:00:00 2001
From: this-is-shivamsingh <shivam.si@browserstack.com>
Date: Tue, 27 Aug 2024 12:34:13 +0530
Subject: [PATCH 13/13] chore: remvoving some excessive comments

---
 packages/core/src/network.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/core/src/network.js b/packages/core/src/network.js
index b498027c1..66841152c 100644
--- a/packages/core/src/network.js
+++ b/packages/core/src/network.js
@@ -207,7 +207,7 @@ export class Network {
     // do not handle data urls
     if (request.url.startsWith('data:')) return;
 
-    // Browsers handle URL encoding leniently, but invalid characters can break tools like Jackproxy.
+    // Browsers handle URL encoding leniently.
     // This code checks for issues such as `%` and leading spaces and warns the user accordingly.
     decodeAndEncodeURLWithLogging(request.url, this.log, {
       meta: { ...this.meta, url: request.url },