From dd158bc4a956f7e8272745bb514728b6f7aaee7f Mon Sep 17 00:00:00 2001 From: Ninad Sheth Date: Tue, 22 Oct 2024 22:51:03 +0530 Subject: [PATCH] Added cookies support in direct requests --- packages/core/src/discovery.js | 4 ++-- packages/core/src/network.js | 19 ++++++++++++------- packages/core/test/discovery.test.js | 16 +++++++++++++--- packages/core/test/helpers/server.js | 2 +- 4 files changed, 28 insertions(+), 13 deletions(-) diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index dfbab837d..03c7c51c4 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -89,7 +89,7 @@ function parseCookies(cookies) { // If cookies is collected via SDK if (Array.isArray(cookies) && cookies.every(item => typeof item === 'object' && 'name' in item && 'value' in item)) { // omit other fields reason sometimes expiry comes as actual date where we expect it to be double - return cookies.map(c => ({ name: c.name, value: c.value, secure: c.secure })); + return cookies.map(c => ({ name: c.name, value: c.value, secure: c.secure, domain: c.domain })); } if (!(typeof cookies === 'string' && cookies !== '')) return null; @@ -218,7 +218,7 @@ 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 || snapshot?.domSnapshot?.[0]?.cookies; + let cookies = snapshot.domSnapshot?.cookies || snapshot.domSnapshot?.[0]?.cookies; cookies = parseCookies(cookies); // iterate over device to trigger reqeusts and capture other dpr width diff --git a/packages/core/src/network.js b/packages/core/src/network.js index e0dca638e..9a5bc9dae 100644 --- a/packages/core/src/network.js +++ b/packages/core/src/network.js @@ -51,7 +51,7 @@ export class Network { session.on('Network.requestWillBeSent', this._handleRequestWillBeSent); session.on('Network.responseReceived', this._handleResponseReceived.bind(this, session)); session.on('Network.eventSourceMessageReceived', this._handleEventSourceMessageReceived); - session.on('Network.loadingFinished', this._handleLoadingFinished); + session.on('Network.loadingFinished', this._handleLoadingFinished.bind(this, session)); session.on('Network.loadingFailed', this._handleLoadingFailed); let commands = [ @@ -286,7 +286,7 @@ export class Network { // Called when a request has finished loading which triggers the this.onrequestfinished // callback. The request should have an associated response and be finished with any redirects. - _handleLoadingFinished = async event => { + _handleLoadingFinished = async (session, event) => { let { requestId } = event; // wait for upto 2 seconds or check if response has been sent await this.#requestsLifeCycleHandler.get(requestId).responseReceived; @@ -294,7 +294,7 @@ export class Network { /* istanbul ignore if: race condition paranioa */ if (!request) return; - await saveResponseResource(this, request); + await saveResponseResource(this, request, session); this._forgetRequest(request); } @@ -413,8 +413,13 @@ async function sendResponseResource(network, request, session) { } // Make a new request with Node based on a network request -function makeDirectRequest(network, request) { - let headers = { ...request.headers }; +async function makeDirectRequest(network, request, session) { + const { cookies = [] } = await session.send('Network.getCookies', { urls: [request.url] }); + + let headers = { + ...request.headers, + cookie: cookies.map(cookie => `${cookie.name}=${cookie.value}`).join('; ') + }; if (network.authorization?.username) { // include basic authorization username and password @@ -427,7 +432,7 @@ function makeDirectRequest(network, request) { } // Save a resource from a request, skipping it if specific paramters are not met -async function saveResponseResource(network, request) { +async function saveResponseResource(network, request, session) { let { disableCache, allowedHostnames, enableJavaScript } = network.intercept; let log = network.log; @@ -478,7 +483,7 @@ async function saveResponseResource(network, request) { // so request them directly. if (mimeType?.includes('font') || (detectedMime && detectedMime.includes('font'))) { log.debug('- Requesting asset directly', meta); - body = await makeDirectRequest(network, request); + body = await makeDirectRequest(network, request, session); log.debug('- Got direct response', meta); } diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index 273a1f359..059ad279e 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -452,13 +452,23 @@ describe('Discovery', () => { await percy.snapshot({ name: 'test snapshot', url: 'http://localhost:8000', - domSnapshot: testDOM + domSnapshot: { + html: testDOM, + cookies: [ + { name: 'abc', value: '123', secure: false, domain: 'localhost' }, + { name: 'abc2', value: '1234', secure: false, domain: 'localhost' }, + { name: 'other', value: '1234', secure: false, domain: 'notlocalhost.com' } + ] + } }); await percy.idle(); // confirm that request was made 2 times, once via browser and once due to makeDirectRequest - let paths = server.requests.map(r => r[0]); - expect(paths.filter(x => x === '/font.woff?abc=1').length).toEqual(2); + let fontRequests = server.requests.filter(r => r[0] === '/font.woff?abc=1'); + expect(fontRequests.length).toEqual(2); + // confirm that direct request [ 2nd ] cookies contain correct cookies + // order of cookies doesnt matter + expect(['abc=123; abc2=1234', 'abc2=1234; abc=123']).toContain(fontRequests[1][1].cookie); let requestData = captured[0].map((x) => x.attributes) .filter(x => x['resource-url'] === 'http://localhost:8000/font.woff?abc=1')[0]; diff --git a/packages/core/test/helpers/server.js b/packages/core/test/helpers/server.js index 9064f0a33..7bcca535d 100644 --- a/packages/core/test/helpers/server.js +++ b/packages/core/test/helpers/server.js @@ -21,7 +21,7 @@ export function createTestServer({ default: defaultReply, ...replies }, port = 8 server.route(async (req, res, next) => { let pathname = req.url.pathname; if (req.url.search) pathname += req.url.search; - server.requests.push(req.body ? [pathname, req.body] : [pathname]); + server.requests.push(req.body ? [pathname, req.body, req.headers] : [pathname, req.headers]); let reply = replies[pathname] || defaultReply; return reply ? await reply(req, res) : next(); });