Skip to content

Commit

Permalink
✨ Introduce the ability to abort requests from specificed hostnames (…
Browse files Browse the repository at this point in the history
…`disallowedHostnames`) (#823)

* ✨ Introduce Disallow hostnames

* 🏗 Fix typos in flag description & include in per-snapshot config
  • Loading branch information
Robdel12 authored Mar 14, 2022
1 parent ec4c98a commit 67a794c
Show file tree
Hide file tree
Showing 9 changed files with 131 additions and 2 deletions.
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: {
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

0 comments on commit 67a794c

Please sign in to comment.