Skip to content
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
8 changes: 0 additions & 8 deletions lighthouse-core/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@ const log = require('lighthouse-logger');
const ChromeProtocol = require('./gather/connections/cri.js');
const Config = require('./config/config.js');

const URL = require('./lib/url-shim.js');
const LHError = require('./lib/lh-error.js');

/** @typedef {import('./gather/connections/connection.js')} Connection */

/*
Expand All @@ -39,11 +36,6 @@ const LHError = require('./lib/lh-error.js');
* @return {Promise<LH.RunnerResult|undefined>}
*/
async function lighthouse(url, flags = {}, configJSON, connection) {
// verify the url is valid and that protocol is allowed
if (url && (!URL.isValid(url) || !URL.isProtocolAllowed(url))) {
throw new LHError(LHError.errors.INVALID_URL);
}

// set logging preferences, assume quiet
flags.logLevel = flags.logLevel || 'error';
log.setLevel(flags.logLevel);
Expand Down
11 changes: 4 additions & 7 deletions lighthouse-core/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,15 +70,12 @@ class Runner {
throw new Error('Cannot run audit mode on different URL');
}
} else {
if (typeof runOpts.url !== 'string' || runOpts.url.length === 0) {
throw new Error(`You must provide a url to the runner. '${runOpts.url}' provided.`);
}

try {
// verify the url is valid and that protocol is allowed
if (runOpts.url && URL.isValid(runOpts.url) && URL.isProtocolAllowed(runOpts.url)) {
// Use canonicalized URL (with trailing slashes and such)
requestedUrl = new URL(runOpts.url).href;
} catch (e) {
throw new Error('The url provided should have a proper protocol and hostname.');
} else {
throw new LHError(LHError.errors.INVALID_URL);
}

artifacts = await Runner._gatherArtifactsFromBrowser(requestedUrl, runOpts, connection);
Expand Down
36 changes: 20 additions & 16 deletions lighthouse-core/test/index-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,22 +87,26 @@ describe('Module Tests', function() {
});
});

it('should throw an error when the url is invalid', function() {
return lighthouse('https:/i-am-not-valid', {}, {})
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fun fact: actually a valid URL according to the spec and implementation

.then(() => {
throw new Error('Should not have resolved when url is invalid');
}, err => {
assert.ok(err);
});
});

it('should throw an error when the url is invalid protocol (file:///)', function() {
return lighthouse('file:///a/fake/index.html', {}, {})
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this one was actually throwing on invalid input for the config

.then(() => {
throw new Error('Should not have resolved when url is file:///');
}, err => {
assert.ok(err);
});
it('should throw an error when the url is invalid', async () => {
expect.hasAssertions();
try {
await lighthouse('i-am-not-valid', {}, {});
} catch (err) {
expect(err.friendlyMessage)
.toBeDisplayString('The URL you have provided appears to be invalid.');
expect(err.code).toEqual('INVALID_URL');
}
});

it('should throw an error when the url is invalid protocol (file:///)', async () => {
expect.hasAssertions();
try {
await lighthouse('file:///a/fake/index.html', {}, {});
} catch (err) {
expect(err.friendlyMessage)
.toBeDisplayString('The URL you have provided appears to be invalid.');
expect(err.code).toEqual('INVALID_URL');
}
});

it('should return formatted LHR when given no categories', function() {
Expand Down
20 changes: 12 additions & 8 deletions lighthouse-core/test/runner-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -514,20 +514,24 @@ describe('Runner', () => {
});


it('rejects when not given a URL', () => {
return Runner.run({}, {}).then(_ => assert.ok(false), _ => assert.ok(true));
it('rejects when not given a URL', async () => {
const config = new Config({});
await expect(Runner.run({}, {config})).rejects.toThrow('INVALID_URL');
});

it('rejects when given a URL of zero length', () => {
return Runner.run({}, {url: ''}).then(_ => assert.ok(false), _ => assert.ok(true));
it('rejects when given a URL of zero length', async () => {
const config = new Config({});
await expect(Runner.run({}, {url: '', config})).rejects.toThrow('INVALID_URL');
});

it('rejects when given a URL without protocol', () => {
return Runner.run({}, {url: 'localhost'}).then(_ => assert.ok(false), _ => assert.ok(true));
it('rejects when given a URL without protocol', async () => {
const config = new Config({});
await expect(Runner.run({}, {url: 'localhost', config})).rejects.toThrow('INVALID_URL');
});

it('rejects when given a URL without hostname', () => {
return Runner.run({}, {url: 'https://'}).then(_ => assert.ok(false), _ => assert.ok(true));
it('rejects when given a URL without hostname', async () => {
const config = new Config({});
await expect(Runner.run({}, {url: 'https://', config})).rejects.toThrow('INVALID_URL');
});

it('only supports core audits with names matching their filename', () => {
Expand Down