Skip to content

Commit

Permalink
Added cookies support in direct requests (#1764)
Browse files Browse the repository at this point in the history
* Added cookies support in direct requests

* Fixed tests
  • Loading branch information
ninadbstack authored Oct 23, 2024
1 parent 9419761 commit 7cabfba
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 25 deletions.
6 changes: 4 additions & 2 deletions packages/cli-build/test/id.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ describe('percy build:id', () => {

expect(logger.stderr).toEqual([]);
expect(logger.stdout).toEqual(['123']);
expect(percyServer.requests).toEqual([['/percy/healthcheck']]);
expect(percyServer.requests.length).toEqual(1);
expect(percyServer.requests[0][0]).toEqual('/percy/healthcheck');
});

it('can call the /percy/healthcheck endpoint at an alternate port', async () => {
Expand All @@ -40,7 +41,8 @@ describe('percy build:id', () => {

expect(logger.stderr).toEqual([]);
expect(logger.stdout).toEqual(['456']);
expect(percyServer.requests).toEqual([['/percy/healthcheck']]);
expect(percyServer.requests.length).toEqual(1);
expect(percyServer.requests[0][0]).toEqual('/percy/healthcheck');
});

it('logs an error when the endpoint errors', async () => {
Expand Down
6 changes: 4 additions & 2 deletions packages/cli-exec/test/ping.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ describe('percy exec:ping', () => {

expect(logger.stderr).toEqual([]);
expect(logger.stdout).toEqual(['[percy] Percy is running']);
expect(percyServer.requests).toEqual([['/percy/healthcheck']]);
expect(percyServer.requests.length).toEqual(1);
expect(percyServer.requests[0][0]).toEqual('/percy/healthcheck');
});

it('can ping /percy/healthcheck at an alternate port', async () => {
Expand All @@ -46,7 +47,8 @@ describe('percy exec:ping', () => {

expect(logger.stderr).toEqual([]);
expect(logger.stdout).toEqual(['[percy] Percy is running']);
expect(percyServer.requests).toEqual([['/percy/healthcheck']]);
expect(percyServer.requests.length).toEqual(1);
expect(percyServer.requests[0][0]).toEqual('/percy/healthcheck');
});

it('logs an error when the endpoint errors', async () => {
Expand Down
14 changes: 6 additions & 8 deletions packages/cli-exec/test/stop.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,9 @@ describe('percy exec:stop', () => {

await stop();

expect(percyServer.requests).toEqual([
['/percy/stop'],
['/percy/healthcheck']
]);
expect(percyServer.requests.length).toEqual(2);
expect(percyServer.requests[0][0]).toEqual('/percy/stop');
expect(percyServer.requests[1][0]).toEqual('/percy/healthcheck');

expect(logger.stderr).toEqual([]);
expect(logger.stdout).toEqual(['[percy] Percy has stopped']);
Expand All @@ -54,10 +53,9 @@ describe('percy exec:stop', () => {

await stop(['--port=4567']);

expect(percyServer.requests).toEqual([
['/percy/stop'],
['/percy/healthcheck']
]);
expect(percyServer.requests.length).toEqual(2);
expect(percyServer.requests[0][0]).toEqual('/percy/stop');
expect(percyServer.requests[1][0]).toEqual('/percy/healthcheck');

expect(logger.stderr).toEqual([]);
expect(logger.stdout).toEqual(['[percy] Percy has stopped']);
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/discovery.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
19 changes: 12 additions & 7 deletions packages/core/src/network.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down Expand Up @@ -286,15 +286,15 @@ 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;
let request = this.#requests.get(requestId);
/* istanbul ignore if: race condition paranioa */
if (!request) return;

await saveResponseResource(this, request);
await saveResponseResource(this, request, session);
this._forgetRequest(request);
}

Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down
16 changes: 13 additions & 3 deletions packages/core/test/discovery.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/helpers/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
Expand Down

0 comments on commit 7cabfba

Please sign in to comment.