Skip to content

Commit

Permalink
fix: support API testing in proxyless mode and attach cookies to the …
Browse files Browse the repository at this point in the history
…request with hammerhead (#7504)

## Purpose
API Testing doesn't work in proxyless mode
Also, cookies are not attached to the request with "withCredential"
option enabled in hammerhead

## Approach
- [x] Update URL parsing part
- [x] Use cookie providers to obtain cookies
- [x] Update testing pages due test logic is incorrect and about:blank
cannot hold cookies
- [x] Add additional params to setCookie method, to allow setting
cookies from the set-cookie string
- [x] Fix issue on hammerhead side 

## References
[HammerHead
fix](DevExpress/testcafe-hammerhead#2854)
[Issue](https://github.com/DevExpress/testcafe-private/issues/78)


## Pre-Merge TODO
- [x] Write tests for your proposed changes
- [x] Make sure that existing tests do not fail
  • Loading branch information
Artem-Babich authored Feb 16, 2023
1 parent e272dfe commit 11401b9
Show file tree
Hide file tree
Showing 11 changed files with 193 additions and 75 deletions.
1 change: 0 additions & 1 deletion gulp/constants/functional-test-globs.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ const PROXYLESS_TESTS_GLOB = [
'!test/functional/fixtures/run-options/request-timeout/test.js',
'!test/functional/fixtures/run-options/disable-page-caching/test.js',
'!test/functional/fixtures/live/test.js',
'!test/functional/fixtures/api/es-next/request/test.js',
'!test/functional/fixtures/regression/gh-1311/test.js',
'!test/functional/fixtures/hammerhead/gh-2622/test.js',
'!test/functional/fixtures/regression/gh-2861/test.js',
Expand Down
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,11 @@
"resolve-from": "^4.0.0",
"sanitize-filename": "^1.6.0",
"semver": "^5.6.0",
"set-cookie-parser": "^2.5.1",
"source-map-support": "^0.5.16",
"strip-bom": "^2.0.0",
"testcafe-browser-tools": "2.0.23",
"testcafe-hammerhead": "28.4.2",
"testcafe-hammerhead": "29.0.0",
"testcafe-legacy-api": "5.1.6",
"testcafe-reporter-dashboard": "^0.2.10",
"testcafe-reporter-json": "^2.1.0",
Expand Down Expand Up @@ -176,6 +177,7 @@
"@types/mustache": "^0.8.32",
"@types/prompts": "^2.0.14",
"@types/semver": "^7.3.4",
"@types/set-cookie-parser": "^2.4.2",
"@types/source-map-support": "^0.5.0",
"@types/useragent": "^2.1.1",
"@typescript-eslint/eslint-plugin": "^5.29.0",
Expand Down
45 changes: 41 additions & 4 deletions src/proxyless/cookie-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import { CookieProvider, CookieProviderBase } from '../test-run/cookies/base';
import CookieParam = Protocol.Network.CookieParam;
import matchCollection from '../utils/match-collection';
import { getActiveClient } from './utils/get-active-client';
import { parse } from 'set-cookie-parser';
import { castArray } from 'lodash';

declare type CookieSameSite = 'Lax' | 'Strict' | 'None';

Expand All @@ -31,12 +33,17 @@ export class CdpCookieProvider extends CookieProviderBase implements CookieProvi
return (matchCollection(cookies, externalCookies) as Cookie[]).map(this._cdpCookieToExternalCookie);
}

async setCookies (cookies: CookieOptions[], url: string): Promise<void> {
const client = await this._getCdpClient();
async setCookies (cookies: string | string[] | CookieOptions[], url: string): Promise<void> {
const client = await this._getCdpClient();
const { hostname = '', pathname = '/' } = url ? new URL(url) : {};
const cookiesArray = castArray<string | CookieOptions>(cookies);

const parsedCookies = this._isCookieOptionsArray(cookiesArray)
? cookiesArray
: this._parseSetCookieStrings(cookiesArray as string[]);

await client.Network.setCookies({
cookies: cookies.map(cookie => this._cookieOptionToCdpCookieParam(cookie, hostname, pathname)),
cookies: parsedCookies.map(cookie => this._cookieOptionToCdpCookieParam(cookie, hostname, pathname)),
});
}

Expand All @@ -58,7 +65,7 @@ export class CdpCookieProvider extends CookieProviderBase implements CookieProvi

for (const cookie of existingCookies) {
await client.Network.deleteCookies({
name: cookie.name,
name: cookie.name || '',
domain: cookie.domain,
path: cookie.path,
});
Expand All @@ -67,6 +74,14 @@ export class CdpCookieProvider extends CookieProviderBase implements CookieProvi
return void 0;
}

async getCookieHeader (url: string): Promise<string | null> {
const [{ domain, path }] = this._parseUrls([url]);
const cookies = await this.getCookies([{ domain }]);
const filteredCookies = cookies.filter(c => this._includesPath(c.path || '/', path));

return filteredCookies.map(c => `${ c.name }=${ c.value }`).join(';');
}

private _cdpCookieToExternalCookie (cookie: Cookie): ExternalCookies {
return {
name: cookie.name,
Expand Down Expand Up @@ -101,4 +116,26 @@ export class CdpCookieProvider extends CookieProviderBase implements CookieProvi
return { domain: hostname, path: pathname };
});
}

private _includesPath (cookiePath: string, urlPath: string): boolean {
if (cookiePath === '/')
return true;

const cookieParts = cookiePath.split('/');
const urlParts = urlPath.split('/');

if (cookieParts.length > urlParts.length)
return false;

while (cookieParts.length) {
if (cookieParts.shift() !== urlParts.shift())
return false;
}

return true;
}

private _parseSetCookieStrings (cookies: string[]): CookieOptions[] {
return parse(cookies) as CookieOptions[];
}
}
2 changes: 1 addition & 1 deletion src/test-run/commands/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ function initSkipJsErrorsOptions (name, val, initOptions, validate = true) {
else if (isSkipJsErrorsOptionsObject(val))
val = new SkipJsErrorsOptions(val, validate);

return prepareSkipJsErrorsOptions(val, initOptions.testRun.opts.experimentalProxyless);
return prepareSkipJsErrorsOptions(val, initOptions.testRun.isProxyless());
}

// Commands
Expand Down
8 changes: 7 additions & 1 deletion src/test-run/cookies/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@ import { CookieOptions } from '../commands/options';

export interface CookieProvider {
initialize: () => Promise<void>,
setCookies: (cookies: CookieOptions[], url: string) => Promise<void>,
setCookies: (cookies: CookieOptions[] | string | string[], url: string) => Promise<void>,
getCookies: (externalCookies: ExternalCookies[], urls: string[]) => Promise<ExternalCookies[]>,
getCookieHeader: (url: string, hostname: string) => Promise<string | null>

deleteCookies (cookies?: CookieOptions[], urls?: string[]): Promise<void>;
}

Expand All @@ -19,4 +21,8 @@ export class CookieProviderBase {
async initialize (): Promise<void> {
return Promise.resolve();
}

protected _isCookieOptionsArray (cookies: Array<string | CookieOptions>): cookies is CookieOptions[] {
return cookies.every((cookie: string | CookieOptions) => typeof cookie === 'object');
}
}
15 changes: 12 additions & 3 deletions src/test-run/cookies/provider.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { ExternalCookies } from 'testcafe-hammerhead';
import { CookieOptions } from '../commands/options';
import { CookieProvider, CookieProviderBase } from './base';
import { castArray } from 'lodash';

export class ProxyCookieProvider extends CookieProviderBase implements CookieProvider {
async getCookies (externalCookies: ExternalCookies[], urls: string[]): Promise<ExternalCookies[]> {
Expand All @@ -9,15 +10,23 @@ export class ProxyCookieProvider extends CookieProviderBase implements CookiePro
return session.cookies.getCookies(externalCookies, urls);
}

async setCookies (cookies: CookieOptions[], url: string): Promise<void> {
const session = this.testRun.session;
async setCookies (cookies: string | string[] | CookieOptions[], url: string): Promise<void> {
const cookiesArray = castArray<string | CookieOptions>(cookies);
const session = this.testRun.session;

if (this._isCookieOptionsArray(cookiesArray))
return session.cookies.setCookies(cookiesArray, url);

return session.cookies.setCookies(cookies, url);
return session.cookies.copySyncCookies(cookiesArray.join(';'), url);
}

async deleteCookies (cookies: CookieOptions[], urls: string[]): Promise<void> {
const session = this.testRun.session;

return session.cookies.deleteCookies(cookies, urls);
}

async getCookieHeader (url: string, hostname: string): Promise<string | null> {
return this.testRun.session.cookies.getHeader({ url, hostname });
}
}
28 changes: 16 additions & 12 deletions src/test-run/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ export default class TestRun extends AsyncEventEmitter {
public readonly browser: Browser;
private readonly _messageBus?: MessageBus;
private _clientEnvironmentPrepared = false;
private _cookieProvider: CookieProvider;
public readonly cookieProvider: CookieProvider;
private _storagesProvider: StoragesProvider;
public readonly startRunExecutionTime?: Date;
private readonly _requestHookEventProvider: RequestHookEventProvider;
Expand Down Expand Up @@ -368,21 +368,25 @@ export default class TestRun extends AsyncEventEmitter {
this._requestHookEventProvider = this._getRequestHookEventProvider();
this._roleProvider = this._getRoleProvider();

this._cookieProvider = CookieProviderFactory.create(this, this.opts.experimentalProxyless as boolean);
this._storagesProvider = StoragesProviderFactory.create(this, this.opts.experimentalProxyless as boolean);
this.cookieProvider = CookieProviderFactory.create(this, this.isProxyless() as boolean);
this._storagesProvider = StoragesProviderFactory.create(this, this.isProxyless() as boolean);

this._addInjectables();
}

public isProxyless (): boolean {
return !!this.opts.experimentalProxyless;
}

private _getRequestHookEventProvider (): RequestHookEventProvider {
if (!this.opts.experimentalProxyless)
if (!this.isProxyless())
return this.session.requestHookEventProvider;

return this._proxylessRequestPipeline.requestHookEventProvider;
}

public saveStoragesSnapshot (storageSnapshot: StoragesSnapshot): void {
if (this.opts.experimentalProxyless)
if (this.isProxyless())
this._proxylessRequestPipeline.restoringStorages = storageSnapshot;
}

Expand All @@ -397,7 +401,7 @@ export default class TestRun extends AsyncEventEmitter {
}

private _getRoleProvider (): RoleProvider {
if (this.opts.experimentalProxyless)
if (this.isProxyless())
return new ProxylessRoleProvider(this);

return new ProxyRoleProvider(this);
Expand Down Expand Up @@ -630,7 +634,7 @@ export default class TestRun extends AsyncEventEmitter {
speed: this.speed,
dialogHandler: JSON.stringify(this.activeDialogHandler),
canUseDefaultWindowActions: JSON.stringify(await this.browserConnection.canUseDefaultWindowActions()),
proxyless: JSON.stringify(this.opts.experimentalProxyless),
proxyless: JSON.stringify(this.isProxyless()),
domain: JSON.stringify(this.browserConnection.browserConnectionGateway.proxy.server1Info.domain),
});
}
Expand All @@ -643,7 +647,7 @@ export default class TestRun extends AsyncEventEmitter {
retryTestPages: !!this.opts.retryTestPages,
speed: this.speed,
dialogHandler: JSON.stringify(this.activeDialogHandler),
proxyless: JSON.stringify(this.opts.experimentalProxyless),
proxyless: JSON.stringify(this.isProxyless()),
});
}

Expand Down Expand Up @@ -865,20 +869,20 @@ export default class TestRun extends AsyncEventEmitter {
public async _enqueueGetCookies (command: GetCookiesCommand): Promise<Partial<CookieOptions>[]> {
const { cookies, urls } = command;

return this._cookieProvider.getCookies(cookies, urls);
return this.cookieProvider.getCookies(cookies, urls);
}

public async _enqueueSetCookies (command: SetCookiesCommand): Promise<void> {
const cookies = command.cookies;
const url = command.url || await this.getCurrentUrl();

return this._cookieProvider.setCookies(cookies, url);
return this.cookieProvider.setCookies(cookies, url);
}

public async _enqueueDeleteCookies (command: DeleteCookiesCommand): Promise<void> {
const { cookies, urls } = command;

return this._cookieProvider.deleteCookies(cookies, urls);
return this.cookieProvider.deleteCookies(cookies, urls);
}

private async _enqueueSetBreakpointCommand (callsite: CallsiteRecord | undefined, error?: string): Promise<void> {
Expand Down Expand Up @@ -1585,7 +1589,7 @@ export default class TestRun extends AsyncEventEmitter {
if (this.disablePageReloads)
return;

await this._cookieProvider.initialize();
await this.cookieProvider.initialize();
await this._storagesProvider.initialize();
}

Expand Down
54 changes: 40 additions & 14 deletions src/test-run/request/create-request-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,8 @@ async function prepareHeaders (headers: OutgoingHttpHeaders, currentPageUrl: URL

const preparedHeaders: OutgoingHttpHeaders = Object.assign({}, DEFAULT_ACCEPT, changeHeaderNamesToLowercase(headers));


preparedHeaders[HTTP_HEADERS.host] = host;
preparedHeaders[HTTP_HEADERS.origin] = origin;
preparedHeaders[HTTP_HEADERS.host] = host;
preparedHeaders[HTTP_HEADERS.origin] = origin;
preparedHeaders[HTTP_HEADERS.contentLength] = body.length;

if (headers.method && METHODS_WITH_CONTENT_TYPE.includes(String(headers.method)))
Expand All @@ -112,10 +111,7 @@ async function prepareHeaders (headers: OutgoingHttpHeaders, currentPageUrl: URL
preparedHeaders[HTTP_HEADERS.proxyAuthorization] = getAuthString(options.proxy.auth);

if (withCredentials) {
const currentPageCookies = testRun.session.cookies.getHeader({
url: currentPageUrl.href,
hostname: currentPageUrl.hostname,
});
const currentPageCookies = await testRun.cookieProvider.getCookieHeader(currentPageUrl.href, currentPageUrl.hostname);

if (currentPageCookies)
preparedHeaders[HTTP_HEADERS.cookie] = currentPageCookies;
Expand Down Expand Up @@ -173,17 +169,47 @@ function getProxyUrl (testRun: TestRun, url: string, withCredentials?: boolean):
}, testRun, true)) as Promise<string>;
}

async function resolvePoxylessUrlParts (url: URL): Promise<{ hostname: string; port: string; href: string; partAfterHost: string }> {
const {
href, hostname,
port, pathname,
search,
} = url;

const partAfterHost = [pathname, search].join('');

return { partAfterHost, href, hostname, port };
}

async function resolvePoxyUrlParts (testRun: TestRun, url: URL, withCredentials: boolean): Promise<{ hostname: string; port: string; href: string; partAfterHost: string }> {
const href = await getProxyUrl(testRun, url.href, withCredentials);
const urlObj = await parseProxyUrl(href);
const { partAfterHost } = urlObj;
const { hostname, port } = urlObj.proxy;

return { partAfterHost, href, hostname, port };
}

function resolveUrlParts (testRun: TestRun, url: URL, withCredentials: boolean): Promise<{ hostname: string; port: string; href: string; partAfterHost: string }> {
return testRun.isProxyless() ? resolvePoxylessUrlParts(url) : resolvePoxyUrlParts(testRun, url, withCredentials);
}

export async function createRequestOptions (currentPageUrl: URL, testRun: TestRun, options: ExternalRequestOptions, callsite: CallsiteRecord | null): Promise<RequestOptions> {
options.headers = options.headers || {};

const url = await prepareUrl(testRun, currentPageUrl, options.url, callsite);
const withCredentials = !currentPageUrl.host || sameOriginCheck(currentPageUrl.href, url.href) || options.withCredentials || false;
const body = transformBody(options.headers, options.body);
const headers = await prepareHeaders(options.headers, currentPageUrl, url, body, testRun, withCredentials, options);
const proxyUrl = await getProxyUrl(testRun, url.href, withCredentials);
const proxyUrlObj = parseProxyUrl(proxyUrl);
let auth = options.auth;

const {
hostname,
port,
href,
partAfterHost,
} = await resolveUrlParts(testRun, url, withCredentials);

if (!auth && url.username && url.password) {
auth = {
username: url.username,
Expand All @@ -193,12 +219,12 @@ export async function createRequestOptions (currentPageUrl: URL, testRun: TestRu

const requestParams: RequestOptionsParams = {
method: options.method || DEFAULT_REQUEST_METHOD,
url: proxyUrl,
url: href,
protocol: DEFAULT_PROTOCOL,
hostname: proxyUrlObj.proxy.hostname,
host: proxyUrlObj.proxy.hostname,
port: proxyUrlObj.proxy.port,
path: prepareSearchParams(proxyUrlObj.partAfterHost, options.params),
hostname: hostname,
host: hostname,
port: port,
path: prepareSearchParams(partAfterHost, options.params),
auth: auth && withCredentials ? `${auth.username}:${auth.password}` : void 0,
headers: headers,
credentials: withCredentials ? testRun.session.getAuthCredentials() : void 0,
Expand Down
5 changes: 2 additions & 3 deletions src/test-run/request/send.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { processResponseData } from './process-response-data';
import HTTP_HEADERS from '../../utils/http-headers';
import { RequestRuntimeError } from '../../errors/runtime';
import { CallsiteRecord } from 'callsite-record';
import { castArray } from 'lodash';
import { RUNTIME_ERRORS } from '../../errors/types';

import {
Expand Down Expand Up @@ -54,8 +53,8 @@ export async function sendRequestThroughAPI (testRun: TestRun, options: External
const setCookie = (data.headers as IncomingHttpHeaders)[HTTP_HEADERS.setCookie];
const sameOrigin = !currentPageUrl.host || sameOriginCheck(currentPageUrl.href, requestOptions.url);

if (setCookie && (sameOrigin || options.withCredentials) )
testRun.session.cookies.copySyncCookies(castArray(setCookie).join(';'), currentPageUrl.href);
if (setCookie && (sameOrigin || options.withCredentials))
await testRun.cookieProvider.setCookies(setCookie, currentPageUrl.href);

return data;
}
8 changes: 8 additions & 0 deletions test/functional/fixtures/api/es-next/request/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,14 @@ describe('Request', () => {
return runTests('testcafe-fixtures/request-test.js', 'Should send request with cookies');
});

it('Should attach cookies to request with another domain if "withCredentials" is true', function () {
return runTests('testcafe-fixtures/request-test.js', 'Should attach cookies to request with another domain if "withCredentials" is true');
});

it('Should not attach cookies to request with another domain if "withCredentials" is false', function () {
return runTests('testcafe-fixtures/request-test.js', 'Should not attach cookies to request with another domain if "withCredentials" is false');
});

it('Should not set cookies to the client from response', function () {
return runTests('testcafe-fixtures/request-test.js', 'Should not set cookies to the client from response');
});
Expand Down
Loading

0 comments on commit 11401b9

Please sign in to comment.