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

✨ Introduce the ability to abort requests from specificed hostnames (disallowedHostnames) #823

Merged
merged 2 commits into from
Mar 14, 2022
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
10 changes: 10 additions & 0 deletions packages/cli-command/src/flags.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,15 @@ export const allowedHostnames = {
short: 'h'
};

export const disallowedHostnames = {
name: 'disallowed-hostname',
description: 'Disallowed hostnames to abort in asset discovery',
percyrc: 'discovery.disallowedHostnames',
type: 'hostname',
multiple: true,
group: 'Percy'
};

export const networkIdleTimeout = {
name: 'network-idle-timeout',
description: 'Asset discovery network idle timeout',
Expand Down Expand Up @@ -95,6 +104,7 @@ export const PERCY = [

export const DISCOVERY = [
allowedHostnames,
disallowedHostnames,
networkIdleTimeout,
disableCache,
debug
Expand Down
1 change: 1 addition & 0 deletions packages/cli-command/test/flags.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ describe('Built-in flags:', () => {
-c, --config <file> Config file path
-d, --dry-run Print snapshot names only
-h, --allowed-hostname <hostname> Allowed hostnames to capture in asset discovery
--disallowed-hostname <hostname> Disallowed hostnames to abort in asset discovery
-t, --network-idle-timeout <ms> Asset discovery network idle timeout
--disable-cache Disable asset discovery caches
--debug Debug asset discovery and do not upload snapshots
Expand Down
2 changes: 2 additions & 0 deletions packages/cli-exec/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ Percy options:
-c, --config <file> Config file path
-d, --dry-run Print snapshot names only
-h, --allowed-hostname <hostname> Allowed hostnames to capture in asset discovery
--disallowed-hostname <hostname> Disallowed hostnames to abort in asset discovery
-t, --network-idle-timeout <ms> Asset discovery network idle timeout
--disable-cache Disable asset discovery caches
--debug Debug asset discovery and do not upload snapshots
Expand Down Expand Up @@ -62,6 +63,7 @@ Percy options:
-c, --config <file> Config file path
-d, --dry-run Print snapshot names only
-h, --allowed-hostname <hostname> Allowed hostnames to capture in asset discovery
--disallowed-hostname <hostname> Disallowed hostnames to abort in asset discovery
-t, --network-idle-timeout <ms> Asset discovery network idle timeout
--disable-cache Disable asset discovery caches
--debug Debug asset discovery and do not upload snapshots
Expand Down
1 change: 1 addition & 0 deletions packages/cli-snapshot/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ Percy options:
-c, --config <file> Config file path
-d, --dry-run Print snapshot names only
-h, --allowed-hostname <hostname> Allowed hostnames to capture in asset discovery
--disallowed-hostname <hostname> Disallowed hostnames to abort in asset discovery
-t, --network-idle-timeout <ms> Asset discovery network idle timeout
--disable-cache Disable asset discovery caches
--debug Debug asset discovery and do not upload snapshots
Expand Down
3 changes: 2 additions & 1 deletion packages/core/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ The following options can also be defined within a Percy config file
- `enableJavaScript` — Enable JavaScript for screenshots (**default** `false`)
- `discovery` — Asset discovery options
- `allowedHostnames` — Array of allowed hostnames to capture assets from
- `disallowedHostnames` — Array of hostnames where requests will be aborted
- `requestHeaders` — Request headers used when discovering snapshot assets
- `authorization` — Basic auth `username` and `password` for protected snapshot assets
- `disableCache` — Disable asset caching (**default** `false`)
Expand Down Expand Up @@ -192,7 +193,7 @@ depending on the provided properties ([see alternate syntaxes below](#alternate-
- `name` — Snapshot name
- `domSnapshot` — Snapshot DOM string
- `discovery` - Limited snapshot specific discovery options
- `allowedHostnames`, `requestHeaders`, `authorization`, `disableCache`, `userAgent`
- `allowedHostnames`, `disallowedHostnames`, `requestHeaders`, `authorization`, `disableCache`, `userAgent`

Common snapshot options are also accepted and will override instance snapshot options. [See instance
options](#options) for common snapshot and discovery options.
Expand Down
15 changes: 15 additions & 0 deletions packages/core/src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,20 @@ export const configSchema = {
}]
}
},
disallowedHostnames: {
Robdel12 marked this conversation as resolved.
Show resolved Hide resolved
type: 'array',
default: [],
items: {
type: 'string',
allOf: [{
not: { pattern: '[^/]/' },
error: 'must not include a pathname'
}, {
not: { pattern: '^([a-zA-Z]+:)?//' },
error: 'must not include a protocol'
}]
}
},
networkIdleTimeout: {
type: 'integer',
default: 100,
Expand Down Expand Up @@ -124,6 +138,7 @@ export const snapshotSchema = {
additionalProperties: false,
properties: {
allowedHostnames: { $ref: '/config/discovery#/properties/allowedHostnames' },
disallowedHostnames: { $ref: '/config/discovery#/properties/disallowedHostnames' },
requestHeaders: { $ref: '/config/discovery#/properties/requestHeaders' },
authorization: { $ref: '/config/discovery#/properties/authorization' },
disableCache: { $ref: '/config/discovery#/properties/disableCache' },
Expand Down
5 changes: 4 additions & 1 deletion packages/core/src/discovery.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const MAX_RESOURCE_SIZE = 15 * (1024 ** 2); // 15MB
const ALLOWED_STATUSES = [200, 201, 301, 302, 304, 307, 308];
const ALLOWED_RESOURCES = ['Document', 'Stylesheet', 'Image', 'Media', 'Font', 'Other'];

export function createRequestHandler(network, { disableCache, getResource }) {
export function createRequestHandler(network, { disableCache, disallowedHostnames, getResource }) {
let log = logger('core:discovery');

return async request => {
Expand All @@ -19,6 +19,9 @@ export function createRequestHandler(network, { disableCache, getResource }) {
if (resource?.root) {
log.debug('- Serving root resource', meta);
await request.respond(resource);
} else if (hostnameMatches(disallowedHostnames, url)) {
log.debug('- Skipping disallowed hostname', meta);
await request.abort(true);
} else if (resource && !disableCache) {
log.debug('- Resource cache hit', meta);
await request.respond(resource);
Expand Down
5 changes: 5 additions & 0 deletions packages/core/src/snapshot.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ export function getSnapshotConfig(percy, options) {
// only specific discovery options are used per-snapshot
discovery: {
allowedHostnames: percy.config.discovery.allowedHostnames,
disallowedHostnames: percy.config.discovery.disallowedHostnames,
networkIdleTimeout: percy.config.discovery.networkIdleTimeout,
requestHeaders: percy.config.discovery.requestHeaders,
authorization: percy.config.discovery.authorization,
Expand All @@ -194,6 +195,8 @@ export function getSnapshotConfig(percy, options) {
case 'execute': // shorthand for execute.beforeSnapshot
return (Array.isArray(next) || typeof next !== 'object')
? [path.concat('beforeSnapshot'), next] : [path];
case 'discovery.disallowedHostnames': // prevent disallowing the root hostname
return [path, (prev ?? []).concat(next).filter(h => !hostnameMatches(h, options.url))];
}

// ensure additional snapshots have complete names
Expand Down Expand Up @@ -238,6 +241,7 @@ function debugSnapshotConfig(snapshot, showInfo) {
debugProp(snapshot, 'execute.afterResize');
debugProp(snapshot, 'execute.beforeSnapshot');
debugProp(snapshot, 'discovery.allowedHostnames');
debugProp(snapshot, 'discovery.disallowedHostnames');
debugProp(snapshot, 'discovery.requestHeaders', JSON.stringify);
debugProp(snapshot, 'discovery.authorization', JSON.stringify);
debugProp(snapshot, 'discovery.disableCache');
Expand Down Expand Up @@ -331,6 +335,7 @@ export async function* discoverSnapshotResources(percy, snapshot, callback) {
enableJavaScript: snapshot.enableJavaScript,
disableCache: snapshot.discovery.disableCache,
allowedHostnames: snapshot.discovery.allowedHostnames,
disallowedHostnames: snapshot.discovery.disallowedHostnames,
getResource: u => resources.get(u) || cache.get(u),
saveResource: r => resources.set(r.url, r) && cache.set(r.url, r)
}
Expand Down
91 changes: 91 additions & 0 deletions packages/core/test/discovery.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1035,6 +1035,97 @@ describe('Discovery', () => {
);
});

it('does not capture resources from disallowed hostnames', async () => {
// stop current instance to create a new one
await percy.stop();

percy = await Percy.start({
token: 'PERCY_TOKEN',
discovery: {
disallowedHostnames: ['ex.localhost']
}
});

logger.loglevel('debug');

await percy.snapshot({
name: 'test snapshot',
url: 'http://localhost:8000',
domSnapshot: testExternalDOM,
widths: [1000]
});

await percy.idle();

expect(logger.stderr).toEqual(jasmine.arrayContaining([
'[percy:core:snapshot] - discovery.disallowedHostnames: ex.localhost'
]));

expect(logger.stderr).toEqual(jasmine.arrayContaining([
'[percy:core:discovery] Handling request: http://ex.localhost:8001/img.gif',
'[percy:core:discovery] - Skipping disallowed hostname'
]));

expect(captured[0]).toEqual([
jasmine.objectContaining({
attributes: jasmine.objectContaining({
'resource-url': jasmine.stringMatching(/^\/percy\.\d+\.log$/)
})
}),
jasmine.objectContaining({
attributes: jasmine.objectContaining({
'resource-url': 'http://localhost:8000/'
})
}),
jasmine.objectContaining({
attributes: jasmine.objectContaining({
'resource-url': 'http://localhost:8000/style.css'
})
})
]);
});

it('ignores root resource URL when passed to disallowedHostnames', async () => {
// stop current instance to create a new one
await percy.stop();

percy = await Percy.start({
token: 'PERCY_TOKEN',
discovery: {
disallowedHostnames: ['localhost']
}
});

logger.loglevel('debug');

await percy.snapshot({
name: 'test snapshot',
url: 'http://localhost:8000',
domSnapshot: testExternalDOM,
widths: [1000]
});

await percy.idle();

expect(captured[0]).toEqual([
jasmine.objectContaining({
attributes: jasmine.objectContaining({
'resource-url': jasmine.stringMatching(/^\/percy\.\d+\.log$/)
})
}),
jasmine.objectContaining({
attributes: jasmine.objectContaining({
'resource-url': 'http://localhost:8000/'
})
}),
jasmine.objectContaining({
attributes: jasmine.objectContaining({
'resource-url': 'http://localhost:8000/style.css'
})
})
]);
});

it('does not hang waiting for embedded isolated pages', async () => {
server.reply('/', () => [200, {
'Content-Type': 'text/html',
Expand Down