From 9988ab71ca6f86990c58c5a3ff52cafc4eddbaec Mon Sep 17 00:00:00 2001 From: Chinmay Maheshwari Date: Thu, 11 Jul 2024 03:12:33 +0530 Subject: [PATCH 1/8] Auto capture and insert cookie in asset discovery browser --- packages/core/src/discovery.js | 10 +++++++++- packages/core/src/page.js | 7 ++++--- packages/dom/src/serialize-dom.js | 9 +++++++++ 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index 814a836c9..4ec73bd32 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -157,6 +157,14 @@ async function* captureSnapshotResources(page, snapshot, options) { const log = logger('core:discovery'); let { discovery, additionalSnapshots = [], ...baseSnapshot } = snapshot; let { capture, captureWidths, deviceScaleFactor, mobile, captureForDevices } = options; + let cookies = snapshot?.domSnapshot?.cookies; + if (!process.env.PERCY_DO_NOT_USE_CAPTURED_COOKIES || process.env.PERCY_DO_NOT_USE_CAPTURED_COOKIES !== 'true') { + cookies = null; + } + if (typeof cookies === 'string') { + cookies = cookies.split('; ').map(c => c.split('=')); + cookies = cookies.map(([key, value]) => { return { name: key, value: value }; }); + } // used to take snapshots and remove any discovered root resource let takeSnapshot = async (options, width) => { @@ -177,7 +185,7 @@ async function* captureSnapshotResources(page, snapshot, options) { // navigate to the url yield resizePage(snapshot.widths[0]); - yield page.goto(snapshot.url); + yield page.goto(snapshot.url, cookies); if (snapshot.execute) { // when any execute options are provided, inject snapshot options diff --git a/packages/core/src/page.js b/packages/core/src/page.js index ca0db063a..6cd792b0d 100644 --- a/packages/core/src/page.js +++ b/packages/core/src/page.js @@ -49,17 +49,18 @@ export class Page { } // Go to a URL and wait for navigation to occur - async goto(url, { waitUntil = 'load' } = {}) { + async goto(url, cookies, { waitUntil = 'load' } = {}) { this.log.debug(`Navigate to: ${url}`, this.meta); let navigate = async () => { // set cookies before navigation so we can default the domain to this hostname - if (this.session.browser.cookies.length) { + if (this.session.browser.cookies.length || cookies) { let defaultDomain = hostname(url); + cookies = this.session.browser.cookies.length > 0 ? this.session.browser.cookies : cookies; await this.session.send('Network.setCookies', { // spread is used to make a shallow copy of the cookie - cookies: this.session.browser.cookies.map(({ ...cookie }) => { + cookies: cookies.map(({ ...cookie }) => { if (!cookie.url) cookie.domain ||= defaultDomain; return cookie; }) diff --git a/packages/dom/src/serialize-dom.js b/packages/dom/src/serialize-dom.js index fe1f49c74..6c58b4e6d 100644 --- a/packages/dom/src/serialize-dom.js +++ b/packages/dom/src/serialize-dom.js @@ -104,8 +104,17 @@ export function serializeDOM(options) { ctx.hints.add('DOM elements found outside '); } + let cookies = null; + try { + cookies = dom.cookie; + } catch (err) { + let errorMessage = `Could not capture cookie: ${err.message}`; + ctx.warnings.add(errorMessage); + console.error(errorMessage); + } let result = { html: serializeHTML(ctx), + cookies: cookies, warnings: Array.from(ctx.warnings), resources: Array.from(ctx.resources), hints: Array.from(ctx.hints) From 19e9b87f1a550a0c9c000f6e9fbedcbf6994476e Mon Sep 17 00:00:00 2001 From: Chinmay Maheshwari Date: Thu, 11 Jul 2024 11:07:46 +0530 Subject: [PATCH 2/8] Added test for cookies serialize-dom --- packages/dom/test/helpers.js | 1 + packages/dom/test/serialize-dom.test.js | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/dom/test/helpers.js b/packages/dom/test/helpers.js index d31dac5e7..56c1a0e03 100644 --- a/packages/dom/test/helpers.js +++ b/packages/dom/test/helpers.js @@ -47,6 +47,7 @@ export function withExample(html, options = { withShadow: true, withRestrictedSh p.innerText = 'P tag outside body'; document.documentElement.append(p); } + document.cookie = 'test-cokkie=test-value'; return document; } diff --git a/packages/dom/test/serialize-dom.test.js b/packages/dom/test/serialize-dom.test.js index abd988186..5780d3d3f 100644 --- a/packages/dom/test/serialize-dom.test.js +++ b/packages/dom/test/serialize-dom.test.js @@ -5,6 +5,7 @@ describe('serializeDOM', () => { it('returns serialied html, warnings, and resources', () => { expect(serializeDOM()).toEqual({ html: jasmine.any(String), + cookies: jasmine.any(String), warnings: jasmine.any(Array), resources: jasmine.any(Array), hints: jasmine.any(Array) @@ -28,7 +29,7 @@ describe('serializeDOM', () => { it('optionally returns a stringified response', () => { expect(serializeDOM({ stringifyResponse: true })) - .toMatch('{"html":".*","warnings":\\[\\],"resources":\\[\\],"hints":\\[\\]}'); + .toMatch('{"html":".*","cookies":".*","warnings":\\[\\],"resources":\\[\\],"hints":\\[\\]}'); }); it('always has a doctype', () => { @@ -76,6 +77,11 @@ describe('serializeDOM', () => { expect(result.html).not.toContain('loading="lazy"'); }); + it('collects cookies', () => { + const result = serializeDOM(); + expect(result.cookies).toContain('test-cokkie=test-value'); + }); + it('clone node is always shallow', () => { class AttributeCallbackTestElement extends window.HTMLElement { static get observedAttributes() { From c6e2ee78e49d0700e2df66a30bb5040dfc0a06f4 Mon Sep 17 00:00:00 2001 From: Chinmay Maheshwari Date: Thu, 11 Jul 2024 12:17:17 +0530 Subject: [PATCH 3/8] Add tests + fix coverage --- packages/core/src/discovery.js | 2 +- packages/core/test/discovery.test.js | 80 ++++++++++++++++++++++++++++ packages/dom/src/serialize-dom.js | 10 +--- 3 files changed, 82 insertions(+), 10 deletions(-) diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index 4ec73bd32..dc8f6f3ce 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -158,7 +158,7 @@ async function* captureSnapshotResources(page, snapshot, options) { let { discovery, additionalSnapshots = [], ...baseSnapshot } = snapshot; let { capture, captureWidths, deviceScaleFactor, mobile, captureForDevices } = options; let cookies = snapshot?.domSnapshot?.cookies; - if (!process.env.PERCY_DO_NOT_USE_CAPTURED_COOKIES || process.env.PERCY_DO_NOT_USE_CAPTURED_COOKIES !== 'true') { + if (process.env.PERCY_DO_NOT_USE_CAPTURED_COOKIES || process.env.PERCY_DO_NOT_USE_CAPTURED_COOKIES === 'true') { cookies = null; } if (typeof cookies === 'string') { diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index e0acf05c6..8df7daadc 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -1265,6 +1265,86 @@ describe('Discovery', () => { expect(cookie).toEqual('chocolate=654321'); }); + + it('should priotize cookie passed by user', async () => { + // test cookie array + await startWithCookies([{ + name: 'chocolate', + value: '654321' + }, { + name: 'shortbread', + value: '987654', + // not the snapshot url + url: 'http://example.com/' + }]); + + await percy.snapshot({ + name: 'mmm cookies', + url: 'http://localhost:8000', + domSnapshot: { + html: testDOM, + cookies: 'test-cookie=value' + } + }); + + expect(logger.stdout).toEqual(jasmine.arrayContaining([ + '[percy] Snapshot taken: mmm cookies' + ])); + + expect(cookie).toEqual('chocolate=654321'); + }); + + it('can send default collected cookies from serialization', async () => { + await percy.stop(); + + percy = await Percy.start({ + token: 'PERCY_TOKEN', + snapshot: { widths: [1000] }, + discovery: { concurrency: 1 } + }); + + await percy.snapshot({ + name: 'mmm cookies', + url: 'http://localhost:8000', + domSnapshot: { + html: testDOM, + cookies: 'test-cookie=value' + } + }); + + expect(logger.stdout).toEqual(jasmine.arrayContaining([ + '[percy] Snapshot taken: mmm cookies' + ])); + + expect(cookie).toEqual('test-cookie=value'); + }); + + it('should not use captured cookie when PERCY_DO_NOT_USE_CAPTURED_COOKIES is set', async () => { + process.env.PERCY_DO_NOT_USE_CAPTURED_COOKIES = true; + await percy.stop(); + + percy = await Percy.start({ + token: 'PERCY_TOKEN', + snapshot: { widths: [1000] }, + discovery: { concurrency: 1 } + }); + + await percy.snapshot({ + name: 'mmm cookies', + url: 'http://localhost:8000', + domSnapshot: { + html: testDOM, + cookies: 'test-cookie=value' + } + }); + + expect(logger.stdout).toEqual(jasmine.arrayContaining([ + '[percy] Snapshot taken: mmm cookies' + ])); + + expect(cookie).toEqual(undefined); + delete process.env.PERCY_DO_NOT_USE_CAPTURED_COOKIES; + }); }); describe('protected resources', () => { diff --git a/packages/dom/src/serialize-dom.js b/packages/dom/src/serialize-dom.js index 6c58b4e6d..38d19668e 100644 --- a/packages/dom/src/serialize-dom.js +++ b/packages/dom/src/serialize-dom.js @@ -104,17 +104,9 @@ export function serializeDOM(options) { ctx.hints.add('DOM elements found outside '); } - let cookies = null; - try { - cookies = dom.cookie; - } catch (err) { - let errorMessage = `Could not capture cookie: ${err.message}`; - ctx.warnings.add(errorMessage); - console.error(errorMessage); - } let result = { html: serializeHTML(ctx), - cookies: cookies, + cookies: dom.cookie, warnings: Array.from(ctx.warnings), resources: Array.from(ctx.resources), hints: Array.from(ctx.hints) From eb8cbdcc3dedbc0c1c9276f084c65c468426cc70 Mon Sep 17 00:00:00 2001 From: chinmay-browserstack <92926953+chinmay-browserstack@users.noreply.github.com> Date: Thu, 11 Jul 2024 12:34:58 +0530 Subject: [PATCH 4/8] Update packages/core/src/discovery.js Co-authored-by: Pradum Kumar --- packages/core/src/discovery.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index dc8f6f3ce..8f8b8f21e 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -158,7 +158,7 @@ async function* captureSnapshotResources(page, snapshot, options) { let { discovery, additionalSnapshots = [], ...baseSnapshot } = snapshot; let { capture, captureWidths, deviceScaleFactor, mobile, captureForDevices } = options; let cookies = snapshot?.domSnapshot?.cookies; - if (process.env.PERCY_DO_NOT_USE_CAPTURED_COOKIES || process.env.PERCY_DO_NOT_USE_CAPTURED_COOKIES === 'true') { + if (process.env.PERCY_DO_NOT_USE_CAPTURED_COOKIES === 'true') { cookies = null; } if (typeof cookies === 'string') { From c9116876a9ec7a0f2af9387b40581f3b348da474 Mon Sep 17 00:00:00 2001 From: Chinmay Maheshwari Date: Thu, 11 Jul 2024 13:22:23 +0530 Subject: [PATCH 5/8] Addressed comments --- packages/core/src/discovery.js | 8 ++++---- packages/core/src/page.js | 27 ++++++++++++++++++++++++--- packages/core/test/discovery.test.js | 12 +++++------- 3 files changed, 33 insertions(+), 14 deletions(-) diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index 8f8b8f21e..39f19b241 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -157,9 +157,9 @@ async function* captureSnapshotResources(page, snapshot, options) { const log = logger('core:discovery'); let { discovery, additionalSnapshots = [], ...baseSnapshot } = snapshot; let { capture, captureWidths, deviceScaleFactor, mobile, captureForDevices } = options; - let cookies = snapshot?.domSnapshot?.cookies; - if (process.env.PERCY_DO_NOT_USE_CAPTURED_COOKIES === 'true') { - cookies = null; + let cookies; + if (process.env.PERCY_DO_NOT_USE_CAPTURED_COOKIES !== 'true') { + cookies = snapshot?.domSnapshot?.cookies; } if (typeof cookies === 'string') { cookies = cookies.split('; ').map(c => c.split('=')); @@ -185,7 +185,7 @@ async function* captureSnapshotResources(page, snapshot, options) { // navigate to the url yield resizePage(snapshot.widths[0]); - yield page.goto(snapshot.url, cookies); + yield page.goto(snapshot.url, { cookies }); if (snapshot.execute) { // when any execute options are provided, inject snapshot options diff --git a/packages/core/src/page.js b/packages/core/src/page.js index 6cd792b0d..f61891db1 100644 --- a/packages/core/src/page.js +++ b/packages/core/src/page.js @@ -49,15 +49,36 @@ export class Page { } // Go to a URL and wait for navigation to occur - async goto(url, cookies, { waitUntil = 'load' } = {}) { + async goto(url, { waitUntil = 'load', cookies } = {}) { this.log.debug(`Navigate to: ${url}`, this.meta); let navigate = async () => { + const mergeCookies = (userPassedCookie, autoCapturedCookie) => { + const mergedCookies = [...userPassedCookie, ...autoCapturedCookie]; + const uniqueCookies = []; + const names = new Set(); + + for (const cookie of mergedCookies) { + if (!names.has(cookie.name)) { + uniqueCookies.push(cookie); + names.add(cookie.name); + } + } + + return uniqueCookies; + }; + + const userPassedCookie = this.session.browser.cookies; // set cookies before navigation so we can default the domain to this hostname - if (this.session.browser.cookies.length || cookies) { + if (userPassedCookie.length || cookies) { let defaultDomain = hostname(url); - cookies = this.session.browser.cookies.length > 0 ? this.session.browser.cookies : cookies; + if (userPassedCookie.length > 0 && cookies) { + cookies = mergeCookies(userPassedCookie, cookies); + } else if (userPassedCookie.length) { + cookies = userPassedCookie; + } + await this.session.send('Network.setCookies', { // spread is used to make a shallow copy of the cookie cookies: cookies.map(({ ...cookie }) => { diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index 8df7daadc..8a8bb3a7a 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -1266,16 +1266,14 @@ describe('Discovery', () => { expect(cookie).toEqual('chocolate=654321'); }); - it('should priotize cookie passed by user', async () => { + it('should merge cookie passed by user', async () => { // test cookie array await startWithCookies([{ - name: 'chocolate', + name: 'test-cookie', value: '654321' }, { name: 'shortbread', - value: '987654', - // not the snapshot url - url: 'http://example.com/' + value: '987654' }]); await percy.snapshot({ @@ -1283,7 +1281,7 @@ describe('Discovery', () => { url: 'http://localhost:8000', domSnapshot: { html: testDOM, - cookies: 'test-cookie=value' + cookies: 'test-cookie=value; cookie-name=cookie-value' } }); @@ -1291,7 +1289,7 @@ describe('Discovery', () => { '[percy] Snapshot taken: mmm cookies' ])); - expect(cookie).toEqual('chocolate=654321'); + expect(cookie).toEqual('test-cookie=654321; shortbread=987654; cookie-name=cookie-value'); }); it('can send default collected cookies from serialization', async () => { From 8c076245dfcdc20e4904b6aaecfaa974a994a120 Mon Sep 17 00:00:00 2001 From: Chinmay Maheshwari Date: Thu, 11 Jul 2024 15:15:19 +0530 Subject: [PATCH 6/8] Handle edge case of httponly cookies --- packages/core/src/discovery.js | 2 +- packages/core/test/discovery.test.js | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index 39f19b241..1ed9a32cc 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -161,7 +161,7 @@ async function* captureSnapshotResources(page, snapshot, options) { if (process.env.PERCY_DO_NOT_USE_CAPTURED_COOKIES !== 'true') { cookies = snapshot?.domSnapshot?.cookies; } - if (typeof cookies === 'string') { + if (typeof cookies === 'string' && cookies !== '') { cookies = cookies.split('; ').map(c => c.split('=')); cookies = cookies.map(([key, value]) => { return { name: key, value: value }; }); } diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index 8a8bb3a7a..4ea8a5a13 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -1317,6 +1317,31 @@ describe('Discovery', () => { expect(cookie).toEqual('test-cookie=value'); }); + it('does not use cookie if empty cookies is passed (in case of httponly)', async () => { + await percy.stop(); + + percy = await Percy.start({ + token: 'PERCY_TOKEN', + snapshot: { widths: [1000] }, + discovery: { concurrency: 1 } + }); + + await percy.snapshot({ + name: 'mmm cookies', + url: 'http://localhost:8000', + domSnapshot: { + html: testDOM, + cookies: '' + } + }); + + expect(logger.stdout).toEqual(jasmine.arrayContaining([ + '[percy] Snapshot taken: mmm cookies' + ])); + + expect(cookie).toEqual(undefined); + }); + it('should not use captured cookie when PERCY_DO_NOT_USE_CAPTURED_COOKIES is set', async () => { process.env.PERCY_DO_NOT_USE_CAPTURED_COOKIES = true; await percy.stop(); From 7fd1392939cbd1f918350caed1c9601dd14163b9 Mon Sep 17 00:00:00 2001 From: Chinmay Maheshwari Date: Thu, 11 Jul 2024 17:29:25 +0530 Subject: [PATCH 7/8] Refactored according to comments --- packages/core/src/page.js | 40 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/packages/core/src/page.js b/packages/core/src/page.js index f61891db1..097390780 100644 --- a/packages/core/src/page.js +++ b/packages/core/src/page.js @@ -48,36 +48,34 @@ export class Page { }); } + mergeCookies(userPassedCookie, autoCapturedCookie) { + if (!autoCapturedCookie) return userPassedCookie; + if (userPassedCookie.length === 0) return autoCapturedCookie; + + const mergedCookies = [...userPassedCookie, ...autoCapturedCookie]; + const uniqueCookies = []; + const names = new Set(); + + for (const cookie of mergedCookies) { + if (!names.has(cookie.name)) { + uniqueCookies.push(cookie); + names.add(cookie.name); + } + } + + return uniqueCookies; + } + // Go to a URL and wait for navigation to occur async goto(url, { waitUntil = 'load', cookies } = {}) { this.log.debug(`Navigate to: ${url}`, this.meta); let navigate = async () => { - const mergeCookies = (userPassedCookie, autoCapturedCookie) => { - const mergedCookies = [...userPassedCookie, ...autoCapturedCookie]; - const uniqueCookies = []; - const names = new Set(); - - for (const cookie of mergedCookies) { - if (!names.has(cookie.name)) { - uniqueCookies.push(cookie); - names.add(cookie.name); - } - } - - return uniqueCookies; - }; - const userPassedCookie = this.session.browser.cookies; // set cookies before navigation so we can default the domain to this hostname if (userPassedCookie.length || cookies) { let defaultDomain = hostname(url); - - if (userPassedCookie.length > 0 && cookies) { - cookies = mergeCookies(userPassedCookie, cookies); - } else if (userPassedCookie.length) { - cookies = userPassedCookie; - } + cookies = this.mergeCookies(userPassedCookie, cookies); await this.session.send('Network.setCookies', { // spread is used to make a shallow copy of the cookie From 37599cf78e5824a3d8f7e3c41eb8945c9dbe9ec1 Mon Sep 17 00:00:00 2001 From: Chinmay Maheshwari Date: Fri, 12 Jul 2024 11:27:55 +0530 Subject: [PATCH 8/8] Added comment --- packages/core/src/page.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/core/src/page.js b/packages/core/src/page.js index 097390780..80a936e31 100644 --- a/packages/core/src/page.js +++ b/packages/core/src/page.js @@ -52,6 +52,7 @@ export class Page { if (!autoCapturedCookie) return userPassedCookie; if (userPassedCookie.length === 0) return autoCapturedCookie; + // User passed cookie will be prioritized over auto captured cookie const mergedCookies = [...userPassedCookie, ...autoCapturedCookie]; const uniqueCookies = []; const names = new Set();