diff --git a/packages/client/src/proxy.js b/packages/client/src/proxy.js index 21c4b1b7e..b4db80e64 100644 --- a/packages/client/src/proxy.js +++ b/packages/client/src/proxy.js @@ -86,17 +86,20 @@ export function href(options) { (path || `${pathname || ''}${search || ''}${hash || ''}`); }; -// Returns the proxy URL for a set of request options -export function getProxy(options) { - let proxyUrl = (options.protocol === 'https:' && +// Returns the proxy URL for a set of request options. An explicit `override` proxy URL is +// used as-is, without consulting the proxy env vars or NO_PROXY; the caller owns bypassing. +export function getProxy(options, override) { + let envProxyUrl = (options.protocol === 'https:' && (process.env.https_proxy || process.env.HTTPS_PROXY)) || (process.env.http_proxy || process.env.HTTP_PROXY); - let shouldProxy = !!proxyUrl && !hostnameMatches( + let shouldProxy = !!override || (!!envProxyUrl && !hostnameMatches( stripQuotesAndSpaces(process.env.no_proxy || process.env.NO_PROXY) - , href(options)); + , href(options))); - if (proxyUrl && typeof proxyUrl === 'string') { proxyUrl = stripQuotesAndSpaces(proxyUrl); } + // only env values may carry stray quotes/spaces; an explicit override is used verbatim + if (envProxyUrl && typeof envProxyUrl === 'string') { envProxyUrl = stripQuotesAndSpaces(envProxyUrl); } + let proxyUrl = override || envProxyUrl; if (shouldProxy) { proxyUrl = new URL(proxyUrl); @@ -128,10 +131,16 @@ export class ProxyHttpAgent extends http.Agent { // needed for https proxies httpsAgent = new https.Agent({ keepAlive: true }); + // an optional `proxy` URL takes precedence over the proxy env vars + constructor({ proxy, ...options } = {}) { + super(options); + this.proxy = proxy; + } + addRequest(request, options) { - let proxy = getProxy(options); + let proxy = getProxy(options, this.proxy); if (!proxy) return super.addRequest(request, options); - logger('client:proxy').debug(`Proxying request: ${options.href}`); + logger('client:proxy').debug(`Proxying request: ${href(options)}`); // modify the request for proxying request.path = href(options); @@ -168,13 +177,15 @@ export class ProxyHttpAgent extends http.Agent { // Proxified https agent export class ProxyHttpsAgent extends https.Agent { - constructor(options) { + // an optional `proxy` URL takes precedence over the proxy env vars + constructor({ proxy, ...options } = {}) { // default keep-alive super({ keepAlive: true, ...options }); + this.proxy = proxy; } createConnection(options, callback) { - let proxy = getProxy(options); + let proxy = getProxy(options, this.proxy); if (!proxy) return super.createConnection(options, callback); logger('client:proxy').debug(`Proxying request: ${href(options)}`); @@ -192,7 +203,11 @@ export class ProxyHttpsAgent extends https.Agent { // start the proxy connection and setup listeners let socket = proxy.connect(); + // destroying the socket re-emits 'error' and 'close'; only the first failure counts + let failed = false; let handleError = err => { + if (failed) return; + failed = true; socket.destroy(err); logger('client:proxy').error(`Proxying request ${href(options)} failed: ${err}`); @@ -210,6 +225,12 @@ export class ProxyHttpsAgent extends https.Agent { new Error('Connection closed while sending request to upstream proxy') ); + // a request's `timeout` only starts once it has a socket, which is handed over after the + // CONNECT reply; bound the handshake itself so a silent proxy cannot hang the request + let handleTimeout = () => handleError(Object.assign(new Error( + `Request to ${href(options)} timed out after ${options.timeout}ms waiting for the proxy` + ), { code: 'ETIMEDOUT' })); + let buffer = ''; let handleData = data => { buffer += data.toString(); @@ -225,12 +246,17 @@ export class ProxyHttpsAgent extends https.Agent { )); } + // the tunnel is established; its later close (e.g. keep-alive teardown) is not a failure + socket.off('error', handleError).off('close', handleClose).off('timeout', handleTimeout); + socket.setTimeout(0); options.socket = socket; options.servername = options.hostname; // callback not passed in so not to be added as a listener callback(null, super.createConnection(options)); }; + if (options.timeout) socket.setTimeout(options.timeout, handleTimeout); + // send and handle the connect message socket .on('error', handleError) @@ -244,6 +270,7 @@ export function proxyAgentFor(url, options) { let cache = (proxyAgentFor.cache ||= new Map()); let { protocol, hostname } = new URL(url); let cachekey = `${protocol}//${hostname}`; + if (options?.proxy) cachekey += ` via ${options.proxy}`; // If we already have a cached agent, return it if (cache.has(cachekey)) { @@ -254,8 +281,8 @@ export function proxyAgentFor(url, options) { let agent; const pacUrl = process.env.PERCY_PAC_FILE_URL; - // If PAC URL is provided, use PAC proxy - if (pacUrl) { + // If PAC URL is provided, use PAC proxy (an explicit proxy takes precedence) + if (pacUrl && !options?.proxy) { logger('client:proxy').info(`Using PAC file from: ${pacUrl}`); agent = createPacAgent(pacUrl, options); } else { diff --git a/packages/client/src/utils.js b/packages/client/src/utils.js index f5a3927b1..7fe84ab7f 100644 --- a/packages/client/src/utils.js +++ b/packages/client/src/utils.js @@ -124,7 +124,8 @@ const RETRY_ERROR_CODES = [ // and any received error details. Server 500 errors are retried up to 5 times at 50ms intervals by // default, and 404 errors may also be optionally retried. If a callback is provided, it is called // with the parsed response body and response details. If the callback returns a value, that value -// will be returned in the final resolved promise instead of the response body. +// will be returned in the final resolved promise instead of the response body. A `proxy` URL +// routes the request through that proxy regardless of the proxy env vars. export async function request(url, options = {}, callback) { // accept `request(url, callback)` if (typeof options === 'function') [options, callback] = [{}, options]; @@ -132,7 +133,7 @@ export async function request(url, options = {}, callback) { // gather request options let { body, headers, retries, retryNotFound, - interval, noProxy, buffer, meta = {}, ...requestOptions + interval, noProxy, proxy, buffer, meta = {}, ...requestOptions } = options; let { protocol, hostname, port, pathname, search, hash } = new URL(url); @@ -150,7 +151,8 @@ export async function request(url, options = {}, callback) { // combine request options Object.assign(requestOptions, { - agent: requestOptions.agent || (!noProxy && proxyAgentFor(url)) || null, + agent: requestOptions.agent || (proxy && proxyAgentFor(url, { proxy })) || + (!noProxy && proxyAgentFor(url)) || null, path: pathname + search + hash, protocol, hostname, diff --git a/packages/client/test/unit/proxy.test.js b/packages/client/test/unit/proxy.test.js index 7bc93a695..3ca3e92f8 100644 --- a/packages/client/test/unit/proxy.test.js +++ b/packages/client/test/unit/proxy.test.js @@ -8,12 +8,25 @@ describe('proxy', () => { process.env.http_proxy = 'http://proxy.com:8080'; }); + afterEach(() => { + delete process.env.no_proxy; + }); + it('should return proxy object if proxy is set', () => { const options = { protocol: 'http:', hostname: 'example.com' }; const proxy = getProxy(options); expect(proxy).toBeInstanceOf(Object); }); + it('should prefer an explicit proxy over env and NO_PROXY', () => { + process.env.no_proxy = '*'; + const options = { protocol: 'https:', hostname: 'example.com' }; + expect(getProxy(options)).toBeUndefined(); + expect(getProxy(options, 'http://other.com:3128')).toEqual(jasmine.objectContaining({ + host: 'other.com', port: '3128' + })); + }); + it('should return undefined if no proxy is set', () => { delete process.env.http_proxy; const options = { protocol: 'http:', hostname: 'example.com' }; @@ -82,6 +95,15 @@ describe('proxy', () => { expect(agent).toBeInstanceOf(PacProxyAgent); }); + it('should create a separately cached agent for an explicit proxy, ignoring PAC', () => { + const url = 'https://example.com'; + const agent = proxyAgentFor(url, { proxy: 'http://localhost:8118' }); + expect(agent).toBeInstanceOf(ProxyHttpsAgent); + expect(agent.proxy).toBe('http://localhost:8118'); + expect(proxyAgentFor(url, { proxy: 'http://localhost:8118' })).toBe(agent); + expect(proxyAgentFor(url)).toBeInstanceOf(PacProxyAgent); + }); + it('logs an error and throws when proxy agent creation fails', () => { const url = 'http://example.com'; const options = {}; diff --git a/packages/client/test/unit/request.test.js b/packages/client/test/unit/request.test.js index 5666ce002..97adfddbe 100644 --- a/packages/client/test/unit/request.test.js +++ b/packages/client/test/unit/request.test.js @@ -83,6 +83,9 @@ function createProxyServer({ type, port, ...options }) { return res.writeHead(403).end(); } + // accept the request, then never answer it + if (options.stall) return; + (proto === 'http' ? http : https).request(url.href, { method, headers, rejectUnauthorized: false }).on('response', remote => { @@ -111,6 +114,8 @@ function createProxyServer({ type, port, ...options }) { return client.end(); } + if (options.stall) return; + let socket = net.connect({ rejectUnauthorized: false, host: 'localhost', @@ -420,6 +425,52 @@ describe('Unit / Request', () => { ]); }); + it('proxies requests through an explicit `proxy` option over env and NO_PROXY', async () => { + delete process.env[env]; + process.env.NO_PROXY = 'localhost'; + + await expectAsync(server.request('/test', { proxy: proxy.address })) + .toBeResolvedTo('test proxied'); + await expectAsync(server.request('/test')) + .toBeResolvedTo('test'); + }); + + it('does not report an error when an established proxy tunnel later closes', async () => { + await expectAsync(server.request('/test')).toBeResolvedTo('test proxied'); + // the keep-alive connection to the proxy is torn down after a successful request + await proxy.close(); + await new Promise(r => setTimeout(r, 50)); + + expect(logger.stderr).not.toContain(jasmine.stringContaining( + 'Connection closed while sending request to upstream proxy')); + }); + + it('rejects without crashing when an established proxy tunnel fails mid-response', async () => { + server.reply('/hang', (req, res) => { + res.writeHead(200, { 'Content-Length': '1000' }); + res.write('partial'); // never finishes + }); + + let pending = server.request('/hang', { retries: 0 }); + await new Promise(r => setTimeout(r, 100)); + await proxy.close(); + + await expectAsync(pending).toBeRejected(); + }); + + it('times out a request when the proxy never answers', async () => { + proxy.options.stall = true; + + await expectAsync(server.request('/test', { timeout: 100, retries: 0 })) + .toBeRejectedWithError(/timed out after 100ms/); + // let the destroyed socket emit its trailing 'error' and 'close' + await new Promise(r => setTimeout(r, 50)); + + // https requests go through a CONNECT tunnel, whose failure is handled only once + expect(logger.stderr.filter(line => /Proxying request .* failed/.test(line))) + .toHaveSize(serverType === 'https' ? 1 : 0); + }); + it('does not proxy requests matching NO_PROXY', async () => { process.env.NO_PROXY = 'localhost'; diff --git a/packages/core/src/network.js b/packages/core/src/network.js index d5ef21f5b..415114853 100644 --- a/packages/core/src/network.js +++ b/packages/core/src/network.js @@ -2,7 +2,7 @@ import { request as makeRequest } from '@percy/client/utils'; import logger from '@percy/logger'; import mime from 'mime-types'; import dns from 'dns'; -import { AbortError, DefaultMap, createResource, hostnameMatches, normalizeURL, waitFor, decodeAndEncodeURLWithLogging, handleIncorrectFontMimeType, executeDomainValidation, isMetadataTarget, isMetadataIP } from './utils.js'; +import { AbortError, DefaultMap, createResource, hostnameMatches, normalizeURL, waitFor, decodeAndEncodeURLWithLogging, handleIncorrectFontMimeType, executeDomainValidation, isMetadataTarget, isMetadataIP, browserProxyFor } from './utils.js'; export const MAX_RESOURCE_SIZE = 25 * (1024 ** 2) * 0.63; // 25MB, 0.63 factor for accounting for base64 encoding // CDP returns binary bodies via Network.getResponseBody as base64 in the JSON-RPC @@ -31,6 +31,8 @@ const ABORTED_MESSAGE = 'Request was aborted by browser'; const RESPONSE_RECEIVED_TIMEOUT = 2000; // Cap idle() impact when a host accepts the TCP connection then stalls during a direct fetch. const DIRECT_FETCH_TIMEOUT = 5000; +// Under PERCY_GZIP the size ceiling can reach tens of MB; 5s is too short. +const directFetchTimeout = () => process.env.PERCY_GZIP ? DIRECT_FETCH_TIMEOUT_WITH_GZIP : DIRECT_FETCH_TIMEOUT; // Stable, machine-readable codes for abort errors thrown from this module. // Consumers should prefer `error.code` over string matching on `error.message`. @@ -639,12 +641,38 @@ export class Network { cb(err, address, family); }); - let { body, status, headers: responseHeaders } = await makeRequest( - request.url, { buffer: true, headers, lookup }, (body, res) => ({ + // Take the same route as the browser: when Chrome was launched with `--proxy-server`, hosts + // may only resolve through that proxy (e.g. privoxy in front of a BrowserStack Local tunnel). + // The proxy then resolves the target, so as on the browser path (where remoteIPAddress is the + // proxy's) the connected-IP metadata gate only sees the proxy address. + let proxy = browserProxyFor(this.page.session?.browser?.args, request.url); + let fetch = (proxy, timeout) => makeRequest( + request.url, { buffer: true, headers, lookup, proxy, timeout }, (body, res) => ({ body, status: res.statusCode, headers: res.headers })); - return { body, status, headers: responseHeaders, remoteAddresses }; + let response; + if (proxy) { + this.log.debug('- Requesting directly through the browser proxy', this.meta); + + try { + // an idle timeout, so a proxy that accepts the connection and then stalls still leaves + // the fallback reachable (the font re-fetch otherwise has no timeout at all) + response = await fetch(proxy, directFetchTimeout()); + } catch (error) { + // The browser can answer a proxy's auth challenge (with discovery.authorization) but this + // fetch cannot, and the proxy may refuse a non-browser client. When the proxy itself fails + // (407, or no response from the target at all, including a stall), fall back to the pre-browser-proxy route + // (env proxy or direct) so anything that fetched before still does. + if (error.response && error.response.statusCode !== 407) throw error; + this.log.debug(`- Browser proxy failed (${error.message.split('\n')[0]}), retrying via the default route`, this.meta); + response = await fetch(); + } + } else { + response = await fetch(); + } + + return { ...response, remoteAddresses }; } } @@ -952,8 +980,10 @@ async function captureResourceDirectly(network, request, session) { let url = originURL(request); let meta = { ...network.meta, url }; - // Under PERCY_GZIP the size ceiling can reach tens of MB; 5s is too short. - let timeoutMs = process.env.PERCY_GZIP ? DIRECT_FETCH_TIMEOUT_WITH_GZIP : DIRECT_FETCH_TIMEOUT; + let timeoutMs = directFetchTimeout(); + // through a browser proxy, directFetch may spend one timeout on the proxied attempt before + // falling back to the default route; budget for both so the fallback is not cut off + if (browserProxyFor(network.page.session?.browser?.args, request.url)) timeoutMs *= 2; try { log.debug('- Requesting resource directly (responseReceived timeout fallback)', meta); diff --git a/packages/core/src/utils.js b/packages/core/src/utils.js index 2bea8fed0..e9224ca96 100644 --- a/packages/core/src/utils.js +++ b/packages/core/src/utils.js @@ -2,6 +2,7 @@ import EventEmitter from 'events'; import { sha256hash, request } from '@percy/client/utils'; import { camelcase, merge } from '@percy/config/utils'; import YAML from 'yaml'; +import net from 'net'; import path from 'path'; import url from 'url'; import { readFileSync } from 'fs'; @@ -152,6 +153,113 @@ export function isMetadataIP(remoteIP) { return matchMetadataHost(remoteIP); } +// Returns the proxy URL Chrome would route `url` through, given the browser's launch `args` +// (`--proxy-server` and `--proxy-bypass-list`), so Node-side fetches made on the browser's behalf +// take the same route. Hosts reachable only through that proxy (e.g. a BrowserStack Local tunnel) +// otherwise fail DNS from Node. Only http(s) proxies are supported: SOCKS and `direct://` rules, +// like a bypassed host, return undefined and leave the fetch to the proxy env vars. Only the first +// proxy of a fallback list is used; Chrome's failover to later entries is not mirrored. +export function browserProxyFor(args, url) { + // Chrome honours the last occurrence of a repeated switch + let flag = name => [].concat(args ?? []).reverse() + .find(arg => arg.startsWith(`--${name}=`))?.slice(name.length + 3); + + let server = flag('proxy-server'); + if (!server) return; + + let { protocol, hostname, port } = new URL(url); + let scheme = protocol.slice(0, -1); + + // rules are `[=][,...]` separated by `;`, where the `socks=` + // mapping is the fallback for schemes without a mapping of their own + let rules = server.split(';').map(rule => rule.trim()); + let mapping = name => rules.find(rule => rule.toLowerCase().startsWith(`${name}=`)); + let rule = mapping(scheme) ?? rules.find(rule => !rule.includes('=')) ?? mapping('socks'); + if (!rule) return; + + // a proxy without a scheme is http, or SOCKS4 within the `socks=` mapping + let proxy = rule.replace(/^\w+=/, '').split(',')[0].trim(); + if (!proxy.includes('://')) proxy = `${rule === mapping('socks') ? 'socks4' : 'http'}://${proxy}`; + if (!/^https?:\/\//.test(proxy)) return; + + port ||= scheme === 'https' ? '443' : '80'; + if (bypassesBrowserProxy(flag('proxy-bypass-list'), { scheme, hostname, port })) return; + return proxy; +} + +// Mirrors Chrome's `--proxy-bypass-list` matching: `,`/`;` separated rules that are either an IP +// range in CIDR notation or a host glob with an optional scheme and port (`.host` meaning +// `*.host`), plus `` for dotless hosts. Loopback and link-local hosts are bypassed +// implicitly unless the list contains `<-loopback>`. +function bypassesBrowserProxy(list = '', { scheme, hostname, port }) { + let rules = list.split(/[,;]/).map(rule => rule.trim()).filter(Boolean); + + if (!rules.includes('<-loopback>') && ( + /^(localhost|127(\.\d+){3}|169\.254(\.\d+){2}|\[::1\])$|\.localhost$/.test(hostname) || + /^\[fe[89ab][0-9a-f]:/.test(hostname) + )) return true; + + return rules.some(rule => { + if (rule === '') return !hostname.includes('.'); + if (rule === '<-loopback>') return false; + + let host = rule.toLowerCase(); + let sep = host.indexOf('://'); + if (sep !== -1) { + if (host.slice(0, sep) !== scheme) return false; + host = host.slice(sep + 3); + } + + if (host.includes('/')) return cidrMatches(host, hostname); + + let colon = host.lastIndexOf(':'); + if (colon > host.lastIndexOf(']') && /^\d+$/.test(host.slice(colon + 1))) { + if (host.slice(colon + 1) !== port) return false; + host = host.slice(0, colon); + } + + if (host.startsWith('.')) host = `*${host}`; + return globMatches(host, hostname); + }); +} + +// Returns true when the IP-literal `hostname` falls within the `cidr` range (IPv4 or IPv6) +function cidrMatches(cidr, hostname) { + let [ip, prefix, ...extra] = cidr.split('/'); + let address = hostname.replace(/^\[/, '').replace(/\]$/, ''); + let family = net.isIP(ip); + // only a plain decimal prefix is valid; Number() would read '' as 0 (matching everything) + // and also accept forms like '1e1' or '0x8' + let bits = !extra.length && /^\d{1,3}$/.test(prefix) ? Number(prefix) : NaN; + + // net.BlockList needs Node >= 14.18; without it CIDR rules are left unmatched + /* istanbul ignore next: CI runs a Node version that has net.BlockList */ + if (typeof net.BlockList !== 'function') return false; + if (!family || net.isIP(address) !== family) return false; + if (!(bits <= (family === 4 ? 32 : 128))) return false; + + let range = new net.BlockList(); + range.addSubnet(ip, bits, `ipv${family}`); + return range.check(address, `ipv${family}`); +} + +// Returns true when `subject` matches `glob`, where `*` matches any run of characters and `?` any +// single one (as in Chrome's MatchPattern). A linear greedy matcher with single-star +// backtracking, so user-supplied patterns need no RegExp. +function globMatches(glob, subject) { + let [g, s, star, mark] = [0, 0, -1, 0]; + + while (s < subject.length) { + if (glob[g] === '*') [star, mark] = [g++, s]; + else if (glob[g] === '?' || glob[g] === subject[s]) [g, s] = [g + 1, s + 1]; + else if (star !== -1) [g, s] = [star + 1, ++mark]; + else return false; + } + + while (glob[g] === '*') g++; + return g === glob.length; +} + // Throws when the URL points at a cloud instance-metadata endpoint. Used to // refuse navigating the top-level snapshot URL to such a target. export function assertNotMetadataTarget(rawUrl) { diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index 4d64606ce..55a541288 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -2649,6 +2649,186 @@ describe('Discovery', () => { }); }); + describe('with a browser proxy', () => { + // `tunnel.test` never resolves (RFC 2606), so like a host behind a BrowserStack Local + // tunnel it is reachable only through the proxy. The test server doubles as that proxy: + // it routes absolute-form requests (`GET http://tunnel.test/font.woff`) by path. + const proxiedFontDOM = dedent` + + + + + +

Hello Percy!

+ ${' '.repeat(1000)} + + + `; + + beforeEach(async () => { + await percy.stop(true); + + percy = await Percy.start({ + token: 'PERCY_TOKEN', + snapshot: { widths: [1000] }, + discovery: { + concurrency: 1, + allowedHostnames: ['tunnel.test'], + launchOptions: { args: ['--proxy-server=http://localhost:8000'] } + } + }); + + percy.loglevel('debug'); + }); + + it('re-fetches fonts through the proxy the browser was launched with', async () => { + await percy.snapshot({ + name: 'proxied font snapshot', + url: 'http://localhost:8000', + domSnapshot: proxiedFontDOM + }); + + await percy.idle(); + + expect(logger.stderr).toContain( + '[percy:core:discovery] - Requesting directly through the browser proxy' + ); + expect(logger.stderr).not.toContain(jasmine.stringContaining('ENOTFOUND')); + expect(captured[0]).toEqual(jasmine.arrayContaining([ + jasmine.objectContaining({ + id: sha256hash(''), + attributes: jasmine.objectContaining({ + 'resource-url': 'http://tunnel.test/font.woff' + }) + }) + ])); + }); + + describe('when the proxy fails the direct fetch', () => { + // localhost is proxied too (<-loopback>), so both the browser and the direct font fetch + // reach the test server as a proxy; only the direct fetch sends `sec-fetch-user: ?1` + const loopbackFontDOM = proxiedFontDOM.replace('http://tunnel.test/font.woff', 'http://localhost:8000/lb-font.woff'); + let directFetches; + + beforeEach(async () => { + await percy.stop(true); + directFetches = 0; + + percy = await Percy.start({ + token: 'PERCY_TOKEN', + snapshot: { widths: [1000] }, + discovery: { + concurrency: 1, + launchOptions: { args: ['--proxy-server=http://localhost:8000', '--proxy-bypass-list=<-loopback>'] } + } + }); + + percy.loglevel('debug'); + }); + + it('falls back to the default route when the proxy requires auth', async () => { + // like an authenticated proxy: the browser got through, the direct fetch gets a 407 + server.reply('/lb-font.woff', req => req.headers['sec-fetch-user'] === '?1' && directFetches++ === 0 + ? [407, { 'Proxy-Authenticate': 'Basic' }, 'proxy auth required'] + : [200, 'font/woff', '']); + + await percy.snapshot({ name: 'proxy 407 snapshot', url: 'http://localhost:8000', domSnapshot: loopbackFontDOM }); + await percy.idle(); + + expect(directFetches).toEqual(2); + expect(logger.stderr).toContain(jasmine.stringMatching( + /- Browser proxy failed \(407 Proxy Authentication Required\), retrying via the default route/)); + expect(captured[0]).toEqual(jasmine.arrayContaining([ + jasmine.objectContaining({ + id: sha256hash(''), + attributes: jasmine.objectContaining({ 'resource-url': 'http://localhost:8000/lb-font.woff' }) + }) + ])); + }); + + it('falls back to the default route when the proxy stalls', async () => { + let release; + server.reply('/lb-font.woff', req => { + if (req.headers['sec-fetch-user'] === '?1' && directFetches++ === 0) { + // accept the request through the proxy, then never answer it + return new Promise(resolve => (release = resolve)); + } + return [200, 'font/woff', '']; + }); + + await percy.snapshot({ name: 'proxy stall snapshot', url: 'http://localhost:8000', domSnapshot: loopbackFontDOM }); + await percy.idle(); + release([200, 'font/woff', '']); + + expect(directFetches).toEqual(2); + expect(logger.stderr).toContain(jasmine.stringMatching( + /- Browser proxy failed \(Request to http:\/\/localhost:8000\/lb-font\.woff timed out after 5000ms\), retrying via the default route/)); + expect(captured[0]).toEqual(jasmine.arrayContaining([ + jasmine.objectContaining({ + id: sha256hash(''), + attributes: jasmine.objectContaining({ 'resource-url': 'http://localhost:8000/lb-font.woff' }) + }) + ])); + }); + + it('leaves time for the fallback when a worker-path direct fetch stalls at the proxy', async () => { + // drop the CDP response so the resource goes through captureResourceDirectly, whose + // overall timeout must cover the stalled proxied attempt plus the default-route retry + spyOn(percy.browser, '_handleMessage').and.callFake(function(data) { + let parsed; try { parsed = JSON.parse(data); } catch { /* binary frame */ } + if (parsed?.method === 'Network.responseReceived' && + parsed.params?.response?.url?.endsWith('/lb-style.css')) return; + this._handleMessage.and.originalFn.call(this, data); + }); + + let release; + server.reply('/lb-style.css', req => { + if (req.headers['sec-fetch-user'] === '?1' && directFetches++ === 0) { + return new Promise(resolve => (release = resolve)); + } + return [200, 'text/css', 'p { color: purple; }']; + }); + + let dom = 'x'; + await percy.snapshot({ name: 'proxy stall worker snapshot', url: 'http://localhost:8000', domSnapshot: dom }); + await percy.idle(); + release([200, 'text/css', 'p { color: purple; }']); + + expect(directFetches).toEqual(2); + expect(logger.stderr).toContain(jasmine.stringMatching(/- Browser proxy failed \(.*timed out after 5000ms\), retrying via the default route/)); + expect(logger.stderr).not.toContain(jasmine.stringContaining('Direct fetch timed out')); + expect(captured[0]).toEqual(jasmine.arrayContaining([ + jasmine.objectContaining({ + attributes: jasmine.objectContaining({ 'resource-url': 'http://localhost:8000/lb-style.css' }) + }) + ])); + }); + + it('does not fall back on an error response from the target', async () => { + server.reply('/lb-font.woff', req => { + // the browser gets the font; the direct fetch gets a 404 from the target + if (req.headers['sec-fetch-user'] !== '?1') return [200, 'font/woff', '']; + directFetches++; + return [404, 'text/plain', 'not found']; + }); + + await percy.snapshot({ name: 'proxy 404 snapshot', url: 'http://localhost:8000', domSnapshot: loopbackFontDOM }); + await percy.idle(); + + expect(directFetches).toEqual(1); + expect(logger.stderr).not.toContain(jasmine.stringContaining('retrying via the default route')); + expect(captured[0]).not.toEqual(jasmine.arrayContaining([ + jasmine.objectContaining({ + attributes: jasmine.objectContaining({ 'resource-url': 'http://localhost:8000/lb-font.woff' }) + }) + ])); + }); + }); + }); + describe('resource caching', () => { let snapshot = async n => { await percy.snapshot({ diff --git a/packages/core/test/unit/utils.test.js b/packages/core/test/unit/utils.test.js index cbd708f78..60f8b528c 100644 --- a/packages/core/test/unit/utils.test.js +++ b/packages/core/test/unit/utils.test.js @@ -6,10 +6,118 @@ import { yieldAll, DefaultMap, redactSecrets, - base64encode + base64encode, + browserProxyFor } from '../../src/utils.js'; describe('Unit / Utils', () => { + describe('browserProxyFor', () => { + const proxy = 'http://127.0.0.1:8118'; + const args = (server, bypass) => [ + '--headless', `--proxy-server=${server}`, + ...(bypass != null ? [`--proxy-bypass-list=${bypass}`] : []) + ]; + + it('returns undefined without a --proxy-server arg', () => { + expect(browserProxyFor(undefined, 'https://a.com')).toBeUndefined(); + expect(browserProxyFor(['--headless'], 'https://a.com')).toBeUndefined(); + }); + + it('returns the proxy for http and https urls', () => { + expect(browserProxyFor(args(proxy), 'https://site.example/font.woff')).toBe(proxy); + expect(browserProxyFor(args(proxy), 'http://site.example/font.woff')).toBe(proxy); + }); + + it('uses the last occurrence of a repeated switch', () => { + expect(browserProxyFor([...args('http://first:1'), ...args(proxy)], 'https://a.com')).toBe(proxy); + }); + + it('defaults a bare host:port to an http proxy and takes the first fallback', () => { + expect(browserProxyFor(args('127.0.0.1:8118,direct://'), 'https://a.com')).toBe(proxy); + expect(browserProxyFor(args('https://secure:443'), 'https://a.com')).toBe('https://secure:443'); + }); + + it('picks per-scheme rules, falling back to a scheme-less rule', () => { + let server = 'https=127.0.0.1:8118;ftp=ftp:21'; + expect(browserProxyFor(args(server), 'https://a.com')).toBe(proxy); + expect(browserProxyFor(args(server), 'http://a.com')).toBeUndefined(); + expect(browserProxyFor(args('ftp=ftp:21;127.0.0.1:8118'), 'http://a.com')).toBe(proxy); + }); + + it('falls back to the socks= mapping for schemes without their own', () => { + expect(browserProxyFor(args('http=first:1;socks=http://127.0.0.1:8118'), 'https://a.com')).toBe(proxy); + // a scheme-less proxy in the socks= mapping is SOCKS4, which is unsupported + expect(browserProxyFor(args('http=first:1;SOCKS=127.0.0.1:1080'), 'https://a.com')).toBeUndefined(); + }); + + it('returns undefined for socks and direct proxies', () => { + expect(browserProxyFor(args('socks5://127.0.0.1:1080'), 'https://a.com')).toBeUndefined(); + expect(browserProxyFor(args('direct://'), 'https://a.com')).toBeUndefined(); + }); + + it('bypasses loopback and link-local hosts unless <-loopback> is listed', () => { + for (let url of [ + 'http://localhost:8000', 'http://127.0.0.1', 'http://[::1]', 'http://app.localhost', + 'http://169.254.1.10', 'http://[fe80::1]', 'http://[febf::1]' + ]) { + expect(browserProxyFor(args(proxy), url)).toBeUndefined(); + expect(browserProxyFor(args(proxy, '<-loopback>'), url)).toBe(proxy); + } + }); + + it('honours the bypass list', () => { + // the scanner's real bypass list + let bypass = '<-loopback>,ws.pusherapp.com,.pusher.com,ssl.gstatic.com,.google.com'; + expect(browserProxyFor(args(proxy, bypass), 'https://ws.pusherapp.com/x')).toBeUndefined(); + expect(browserProxyFor(args(proxy, bypass), 'https://js.pusher.com/x')).toBeUndefined(); + expect(browserProxyFor(args(proxy, bypass), 'https://fonts.google.com/x')).toBeUndefined(); + expect(browserProxyFor(args(proxy, bypass), 'https://fonts.gstatic.com/x')).toBe(proxy); + expect(browserProxyFor(args(proxy, bypass), 'https://nestscheme-sit6.uk.tapue.com/f.woff2')).toBe(proxy); + }); + + it('matches bypass globs, schemes, ports and ', () => { + expect(browserProxyFor(args(proxy, '*.cdn.com'), 'https://a.cdn.com')).toBeUndefined(); + expect(browserProxyFor(args(proxy, '*cdn.com'), 'https://mycdn.com')).toBeUndefined(); + expect(browserProxyFor(args(proxy, '*'), 'https://any.where')).toBeUndefined(); + expect(browserProxyFor(args(proxy, 'cdn.*'), 'https://cdn.example.com')).toBeUndefined(); + // trailing stars left over once the host is exhausted still match + expect(browserProxyFor(args(proxy, 'a.com**'), 'https://a.com')).toBeUndefined(); + expect(browserProxyFor(args(proxy, 'CDN.*.com'), 'https://cdn.a.b.com')).toBeUndefined(); + expect(browserProxyFor(args(proxy, '*.cdn.com'), 'https://cdn.com')).toBe(proxy); + expect(browserProxyFor(args(proxy, 'cdn.*.net'), 'https://cdn.a.com')).toBe(proxy); + expect(browserProxyFor(args(proxy, 'cdn?.com'), 'https://cdn1.com')).toBeUndefined(); + expect(browserProxyFor(args(proxy, 'cdn?.com'), 'https://cdn12.com')).toBe(proxy); + expect(browserProxyFor(args(proxy, 'https://a.com'), 'https://a.com')).toBeUndefined(); + expect(browserProxyFor(args(proxy, 'a.com:443'), 'https://a.com')).toBeUndefined(); + expect(browserProxyFor(args(proxy, 'a.com:8443'), 'https://a.com')).toBe(proxy); + expect(browserProxyFor(args(proxy, 'a.com:80'), 'http://a.com')).toBeUndefined(); + expect(browserProxyFor(args(proxy, ''), 'http://intranet')).toBeUndefined(); + expect(browserProxyFor(args(proxy, ''), 'http://a.com')).toBe(proxy); + expect(browserProxyFor(args(proxy, ' ; a.com'), 'https://b.com')).toBe(proxy); + expect(browserProxyFor(args(proxy, '[2001:db8::1]:8080'), 'http://[2001:db8::1]:8080')).toBeUndefined(); + }); + + it('only bypasses scheme-restricted rules for that scheme', () => { + expect(browserProxyFor(args(proxy, 'https://a.com'), 'http://a.com')).toBe(proxy); + expect(browserProxyFor(args(proxy, 'HTTP://a.com'), 'http://a.com')).toBeUndefined(); + }); + + it('matches CIDR rules against IP-literal hosts', () => { + expect(browserProxyFor(args(proxy, '192.168.1.0/24'), 'http://192.168.1.20')).toBeUndefined(); + expect(browserProxyFor(args(proxy, '192.168.1.0/24'), 'http://192.168.2.1')).toBe(proxy); + expect(browserProxyFor(args(proxy, '192.168.1.0/24'), 'http://a.com')).toBe(proxy); + expect(browserProxyFor(args(proxy, '2001:db8::/32'), 'http://[2001:db8::1]')).toBeUndefined(); + expect(browserProxyFor(args(proxy, '192.168.0.0/16'), 'http://[2001:db8::1]')).toBe(proxy); + expect(browserProxyFor(args(proxy, 'not-an-ip/8'), 'http://10.0.0.1')).toBe(proxy); + expect(browserProxyFor(args(proxy, '10.0.0.0/33'), 'http://10.0.0.1')).toBe(proxy); + expect(browserProxyFor(args(proxy, '10.0.0.0/x'), 'http://10.0.0.1')).toBe(proxy); + // malformed prefixes must not degrade to /0 (which would match every address) + for (let rule of ['10.0.0.0/', '10.0.0.0/1e1', '10.0.0.0/0x8', '10.0.0.0/ 8', '10.0.0.0/8/9']) { + expect(browserProxyFor(args(proxy, rule), 'http://10.0.0.1')).withContext(rule).toBe(proxy); + } + }); + }); + describe('generatePromise', () => { it('accepts a generator and returns a promise', async () => { let gen = (function*(done) { while (!done) done = yield; })();