Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proxyless: correctly handle page with invalid certificate #7542

Merged
merged 1 commit into from
Mar 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@
"callsite-record": "^4.0.0",
"chai": "4.3.4",
"chalk": "^2.3.0",
"chrome-remote-interface": "^0.31.3",
"chrome-remote-interface": "^0.32.1",
"coffeescript": "^2.3.1",
"commander": "^8.3.0",
"debug": "^4.3.1",
Expand Down Expand Up @@ -192,7 +192,7 @@
"chrome-launcher": "^0.15.0",
"connect": "^3.4.0",
"cross-env": "^7.0.3",
"devtools-protocol": "0.0.1078443",
"devtools-protocol": "0.0.1109433",
"dom-walk": "^0.1.1",
"escape-string-regexp": "^4.0.0",
"eslint": "^7.13.0",
Expand Down
3 changes: 2 additions & 1 deletion src/proxyless/api-base.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { ProtocolApi } from 'chrome-remote-interface';
import BrowserConnection from '../browser/connection';
import TestRun from '../test-run';
import { notImplementedError } from './errors';

export default class ProxylessApiBase {
protected readonly _client: ProtocolApi;
Expand All @@ -12,7 +13,7 @@ export default class ProxylessApiBase {
}

public async init (): Promise<void> {
throw new Error('Not implemented');
throw notImplementedError();
}

protected get _testRun (): TestRun {
Expand Down
16 changes: 16 additions & 0 deletions src/proxyless/errors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
export function notImplementedError (): Error {
return new Error('Not implemented');
}

export function failedToFindDNSError (url: string): Error {
return new Error(`Failed to find a DNS-record for the resource at "${url}"`);
}

export function sslCertificateError (type: string): Error {
return new Error(`SSL certificate error (${type})`);
}

export function unknownCDPEventType (type: string): Error {
return new Error(`Unknown CDP event type: ${type}`);
}

17 changes: 2 additions & 15 deletions src/proxyless/index.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,10 @@
import { ProtocolApi } from 'chrome-remote-interface';
import Protocol from 'devtools-protocol';
import RequestPattern = Protocol.Network.RequestPattern;
import ProxylessRequestPipeline from './request-pipeline';
import addCustomDebugFormatters from './add-custom-debug-formatters';
import { ProxylessSetupOptions } from '../shared/types';
import { proxylessLogger } from '../utils/debug-loggers';
import SessionStorage from './session-storage';

const ALL_REQUEST_RESPONSES = { requestStage: 'Request' } as RequestPattern;
const ALL_REQUEST_REQUESTS = { requestStage: 'Response' } as RequestPattern;

const ALL_REQUESTS_DATA = [ALL_REQUEST_REQUESTS, ALL_REQUEST_RESPONSES];

export default class Proxyless {
private readonly _client: ProtocolApi;
public readonly requestPipeline;
Expand All @@ -20,7 +13,7 @@ export default class Proxyless {
public constructor (browserId: string, client: ProtocolApi) {
this._client = client;
this.requestPipeline = new ProxylessRequestPipeline(browserId, client);
this.sessionStorage = new SessionStorage(browserId, client);
this.sessionStorage = new SessionStorage(browserId, client);

this.sessionStorage.on('contextStorageSync', ({ sessionStorage, testRunId, frameDriverId }) => {
if (sessionStorage) {
Expand All @@ -39,12 +32,6 @@ export default class Proxyless {
}

public async init (options: ProxylessSetupOptions): Promise<void> {
// NOTE: We are forced to handle all requests and responses at once
// because CDP API does not allow specifying request filtering behavior for different handlers.
await this._client.Fetch.enable({
patterns: ALL_REQUESTS_DATA,
});

const proxylessSystems = [
this.requestPipeline,
this.sessionStorage,
Expand All @@ -59,7 +46,7 @@ export default class Proxyless {
public async dispose (): Promise<void> {
this.requestPipeline.stop();

await this._client.Fetch.disable();
await this.requestPipeline.dispose();

proxylessLogger('proxyless disposed');
}
Expand Down
44 changes: 43 additions & 1 deletion src/proxyless/request-pipeline/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import LoadingFailedEvent = Protocol.Network.LoadingFailedEvent;
import ContinueResponseRequest = Protocol.Fetch.ContinueResponseRequest;
import FrameTree = Protocol.Page.FrameTree;
import FulfillRequestRequest = Protocol.Fetch.FulfillRequestRequest;
import RequestPattern = Protocol.Fetch.RequestPattern;
import ProxylessRequestHookEventProvider from '../request-hooks/event-provider';
import ResourceInjector from '../resource-injector';
import { convertToHeaderEntries } from '../utils/headers';
Expand Down Expand Up @@ -42,8 +43,15 @@ import ProxylessApiBase from '../api-base';
import { resendAuthRequest } from './resendAuthRequest';
import TestRunBridge from './test-run-bridge';
import ProxylessRequestContextInfo from './context-info';
import CertificateErrorEvent = Protocol.Security.CertificateErrorEvent;
import { failedToFindDNSError, sslCertificateError } from '../errors';


const ALL_REQUEST_RESPONSES = { requestStage: 'Request' } as RequestPattern;
const ALL_REQUEST_REQUESTS = { requestStage: 'Response' } as RequestPattern;

const ALL_REQUESTS_DATA = [ALL_REQUEST_REQUESTS, ALL_REQUEST_RESPONSES];

export default class ProxylessRequestPipeline extends ProxylessApiBase {
private readonly _testRunBridge: TestRunBridge;
private readonly _contextInfo: ProxylessRequestContextInfo;
Expand All @@ -56,6 +64,7 @@ export default class ProxylessRequestPipeline extends ProxylessApiBase {
private _stopped: boolean;
private _currentFrameTree: FrameTree | null;
private readonly _failedRequestIds: string[];
private _pendingCertificateError: CertificateErrorEvent | null;

public constructor (browserId: string, client: ProtocolApi) {
super(browserId, client);
Expand All @@ -71,6 +80,7 @@ export default class ProxylessRequestPipeline extends ProxylessApiBase {
this._failedRequestIds = [];
this.restoringStorages = null;
this.contextStorage = null;
this._pendingCertificateError = null;
}

private _getSpecialServiceRoutes (): SpecialServiceRoutes {
Expand Down Expand Up @@ -220,9 +230,25 @@ export default class ProxylessRequestPipeline extends ProxylessApiBase {
}
}

private _createError (event: RequestPausedEvent): Error {
if (this._pendingCertificateError)
return sslCertificateError(this._pendingCertificateError.errorType);

if (event.responseErrorReason === 'NameNotResolved')
return failedToFindDNSError(event.request.url);

return new Error(event.responseErrorReason);
}

private async _tryRespondToOtherRequest (event: RequestPausedEvent): Promise<void> {
try {
await this._respondToOtherRequest(event);
if (event.responseErrorReason && this._shouldRedirectToErrorPage(event)) {
const error = this._createError(event);

await this._resourceInjector.redirectToErrorPage(this._client, error, event.request.url);
}
else
await this._respondToOtherRequest(event);
}
catch (err) {
if (event.networkId && this._failedRequestIds.includes(event.networkId)) {
Expand Down Expand Up @@ -289,6 +315,12 @@ export default class ProxylessRequestPipeline extends ProxylessApiBase {
public async init (options?: ProxylessSetupOptions): Promise<void> {
this._options = options as ProxylessSetupOptions;

// NOTE: We are forced to handle all requests and responses at once
// because CDP API does not allow specifying request filtering behavior for different handlers.
await this._client.Fetch.enable({
patterns: ALL_REQUESTS_DATA,
});

this._client.Fetch.on('requestPaused', async (event: RequestPausedEvent) => {
if (this._stopped)
return;
Expand Down Expand Up @@ -331,9 +363,19 @@ export default class ProxylessRequestPipeline extends ProxylessApiBase {
});

await this._client.Page.setBypassCSP({ enabled: true });

await this._client.Security.enable();

this._client.Security.on('certificateError', async (event: CertificateErrorEvent) => {
this._pendingCertificateError = event;
});
}

public stop (): void {
this._stopped = true;
}

public async dispose (): Promise<void> {
await this._client.Fetch.disable();
}
}
3 changes: 2 additions & 1 deletion src/proxyless/resource-injector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
} from './utils/string';
import { safeFulfillRequest } from './request-pipeline/safe-api';
import TestRunBridge from './request-pipeline/test-run-bridge';
import { failedToFindDNSError } from './errors';

const RESPONSE_REMOVED_HEADERS = [
'cross-origin-embedder-policy',
Expand Down Expand Up @@ -155,7 +156,7 @@ export default class ResourceInjector {

try {
if (responseErrorReason === 'NameNotResolved') {
const err = new Error(`Failed to find a DNS-record for the resource at "${event.request.url}"`);
const err = failedToFindDNSError(request.url);

return {
error: err,
Expand Down
3 changes: 2 additions & 1 deletion src/proxyless/utils/cdp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import FrameNavigatedEvent = Protocol.Page.FrameNavigatedEvent;
import { IncomingMessageLike } from 'testcafe-hammerhead';
import { convertToHeaderEntries } from './headers';
import { EventType } from '../types';
import { unknownCDPEventType } from '../errors';


export async function redirect (client: ProtocolApi, requestId: string, url: string): Promise<void> {
Expand Down Expand Up @@ -34,7 +35,7 @@ export async function dispatchEvent (client: ProtocolApi, type: EventType, optio
await client.Input.dispatchTouchEvent(options);
break;
default:
throw new Error(`Unknown "${options.type}" event type`);
throw unknownCDPEventType(options.type);
}
}

Expand Down
13 changes: 7 additions & 6 deletions test/functional/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -238,12 +238,13 @@ module.exports = {
site: {
viewsPath: './test/functional/',
ports: {
server1: 3000,
server2: 3001,
basicAuthServer: 3002,
ntlmAuthServer: 3003,
trustedProxyServer: 3004,
transparentProxyServer: 3005,
server1: 3000,
server2: 3001,
basicAuthServer: 3002,
ntlmAuthServer: 3003,
trustedProxyServer: 3004,
transparentProxyServer: 3005,
invalidCertificateHttpsServer: 3007,
},
},

Expand Down
12 changes: 12 additions & 0 deletions test/functional/fixtures/request-pipeline/errors/test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
const { expect } = require('chai');
const { onlyInProxyless } = require('../../../utils/skip-in');

describe('Should handle request pipeline errors', function () {
onlyInProxyless('Certificate error', function () {
return runTests('./testcafe-fixtures/certificate-error.js', null, { shouldFail: true })
.catch(errs => {
expect(errs[0]).contains('Failed to load the page at "https://localhost:3007/".');
expect(errs[0]).contains('Error: SSL certificate error (ERR_CERT_AUTHORITY_INVALID)');
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
fixture `Certificate error`
.page('https://localhost:3007/');

test('test', () => {});
68 changes: 25 additions & 43 deletions test/functional/site/basic-auth-server.js
Original file line number Diff line number Diff line change
@@ -1,48 +1,30 @@
const http = require('http');
const express = require('express');
const basicAuth = require('basic-auth');


let server = null;
let sockets = null;

function start (port) {
const app = express();

app.all('*', function (req, res) {
const credentials = basicAuth(req);

if (!credentials || credentials.name !== 'username' || credentials.pass !== 'password') {
res.statusCode = 401;
res.setHeader('WWW-Authenticate', 'Basic realm="example"');
res.end('<html><body><div id="result">not authorized</div></body></html>');
}
else {
res.statusCode = 200;
res.end('<html><body><div id="result">authorized</div></body></html>');
}
});

server = http.createServer(app).listen(port);
sockets = [];

const connectionHandler = function (socket) {
sockets.push(socket);

socket.on('close', function () {
sockets.splice(sockets.indexOf(socket), 1);
const http = require('http');
const express = require('express');
const basicAuth = require('basic-auth');
const BasicHttpServer = require('./basic-http-server');

class BasicAuthServer extends BasicHttpServer {
start (port) {
const app = express();

app.all('*', function (req, res) {
const credentials = basicAuth(req);

if (!credentials || credentials.name !== 'username' || credentials.pass !== 'password') {
res.statusCode = 401;
res.setHeader('WWW-Authenticate', 'Basic realm="example"');
res.end('<html><body><div id="result">not authorized</div></body></html>');
}
else {
res.statusCode = 200;
res.end('<html><body><div id="result">authorized</div></body></html>');
}
});
};

server.on('connection', connectionHandler);
}

function shutdown () {
server.close();
this.server = http.createServer(app).listen(port);

sockets.forEach(socket => {
socket.destroy();
});
super.start();
}
}

module.exports = { start: start, shutdown: shutdown };
module.exports = new BasicAuthServer();
35 changes: 35 additions & 0 deletions test/functional/site/basic-http-server.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
class BasicHttpServer {
constructor () {
this.server = null;
this.sockets = [];
}

start () {
if (!this.server)
return;

const self = this;

this.server.on('connection', (socket) => {
self.sockets.push(socket);

socket.on('close', function () {
self.sockets.splice(self.sockets.indexOf(socket), 1);
});
});
}

shutdown () {
if (!this.server)
return;

this.server.close();

this.sockets.forEach(socket => {
socket.destroy();
});
}
}

module.exports = BasicHttpServer;

Loading