Skip to content

Commit

Permalink
Manual merge of #1918 https audit fix. 1/2
Browse files Browse the repository at this point in the history
  • Loading branch information
paulirish committed Apr 22, 2017
1 parent 6624328 commit 74bb788
Show file tree
Hide file tree
Showing 11 changed files with 116 additions and 207 deletions.
2 changes: 2 additions & 0 deletions lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html
Original file line number Diff line number Diff line change
Expand Up @@ -353,5 +353,7 @@ <h2>Do better web tester page</h2>
<!-- PASS: not in header, so it does not block rendering. zone.js is loaded
by the static-server and provides a polyfill for Promise. -->
<script src="/zone.js"></script>
<!-- FAIL(is-on-https): requires a non-localhost http file -->
<script src="http://ajax.googleapis.com/ajax/libs/jquery/3.2.1/jquery.min.js"></script>
</body>
</html>
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ module.exports = [
url: 'http://localhost:10200/online-only.html',
audits: {
'is-on-https': {
score: false
score: true
},
'uses-http2': {
score: false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ module.exports = [
url: 'http://localhost:10200/online-only.html',
audits: {
'is-on-https': {
score: false
score: true
},
'redirects-http': {
score: false
Expand Down Expand Up @@ -114,7 +114,7 @@ module.exports = [
url: 'http://localhost:10503/offline-ready.html',
audits: {
'is-on-https': {
score: false
score: true
},
'redirects-http': {
score: false
Expand Down
1 change: 0 additions & 1 deletion lighthouse-cli/test/smokehouse/pwa-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ module.exports = {
recordTrace: true,
gatherers: [
'url',
'https',
'manifest',
// https://github.com/GoogleChrome/lighthouse/issues/566
// 'cache-contents'
Expand Down
35 changes: 32 additions & 3 deletions lighthouse-core/audits/is-on-https.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@
'use strict';

const Audit = require('./audit');
const Formatter = require('../report/formatter');
const URL = require('../lib/url-shim');

const SECURE_SCHEMES = ['data', 'https', 'wss'];
const SECURE_DOMAINS = ['localhost', '127.0.0.1'];

class HTTPS extends Audit {
/**
Expand All @@ -32,18 +37,42 @@ class HTTPS extends Audit {
'in on the communications between your app and your users, and is a prerequisite for ' +
'HTTP/2 and many new web platform APIs. ' +
'[Learn more](https://developers.google.com/web/tools/lighthouse/audits/https).',
requiredArtifacts: ['HTTPS']
requiredArtifacts: ['networkRecords']
};
}

/**
* @param {{scheme: string, domain: string}} record
* @return {boolean}
*/
static isSecureRecord(record) {
return SECURE_SCHEMES.includes(record.scheme) || SECURE_DOMAINS.includes(record.domain);
}

/**
* @param {!Artifacts} artifacts
* @return {!AuditResult}
*/
static audit(artifacts) {
const networkRecords = artifacts.networkRecords[Audit.DEFAULT_PASS];
const insecureRecords = networkRecords
.filter(record => !HTTPS.isSecureRecord(record))
.map(record => ({url: URL.getDisplayName(record.url, {preserveHost: true})}));

let displayValue = '';
if (insecureRecords.length > 1) {
displayValue = `${insecureRecords.length} insecure requests found`;
} else if (insecureRecords.length === 1) {
displayValue = `${insecureRecords.length} insecure request found`;
}

return HTTPS.generateAuditResult({
rawValue: artifacts.HTTPS.value,
debugString: artifacts.HTTPS.debugString
rawValue: insecureRecords.length === 0,
displayValue,
extendedInfo: {
formatter: Formatter.SUPPORTED_FORMATS.URL_LIST,
value: insecureRecords
}
});
}
}
Expand Down
1 change: 0 additions & 1 deletion lighthouse-core/config/default.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
"useThrottling": true,
"gatherers": [
"url",
"https",
"viewport",
"theme-color",
"manifest",
Expand Down
67 changes: 0 additions & 67 deletions lighthouse-core/gather/gatherers/https.js

This file was deleted.

66 changes: 49 additions & 17 deletions lighthouse-core/test/audits/is-on-https-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,56 @@ const assert = require('assert');
/* eslint-env mocha */

describe('Security: HTTPS audit', () => {
it('fails when not on HTTPS', () => {
const debugString = 'Error string';
const result = Audit.audit({
HTTPS: {
value: false,
debugString
}
});
assert.strictEqual(result.score, false);
assert.strictEqual(result.debugString, debugString);
function getArtifacts(networkRecords) {
return {networkRecords: {defaultPass: networkRecords}};
}

it('fails when there is more than one insecure record', () => {
const result = Audit.audit(getArtifacts([
{url: 'https://google.com/', scheme: 'https', domain: 'google.com'},
{url: 'http://insecure.com/image.jpeg', scheme: 'http', domain: 'insecure.com'},
{url: 'http://insecure.com/image2.jpeg', scheme: 'http', domain: 'insecure.com'},
{url: 'https://google.com/', scheme: 'https', domain: 'google.com'},
]));
assert.strictEqual(result.rawValue, false);
assert.ok(result.displayValue.includes('requests found'));
assert.strictEqual(result.extendedInfo.value.length, 2);
});

it('passes when on HTTPS', () => {
const result = Audit.audit({
HTTPS: {
value: true
}
it('fails when there is one insecure record', () => {
const result = Audit.audit(getArtifacts([
{url: 'https://google.com/', scheme: 'https', domain: 'google.com'},
{url: 'http://insecure.com/image.jpeg', scheme: 'http', domain: 'insecure.com'},
{url: 'https://google.com/', scheme: 'https', domain: 'google.com'},
]));
assert.strictEqual(result.rawValue, false);
assert.ok(result.displayValue.includes('request found'));
assert.deepEqual(result.extendedInfo.value[0], {url: 'insecure.com/image.jpeg'});
});

it('passes when all records are secure', () => {
const result = Audit.audit(getArtifacts([
{url: 'https://google.com/', scheme: 'https', domain: 'google.com'},
{url: 'http://localhost/image.jpeg', scheme: 'http', domain: 'localhost'},
{url: 'https://google.com/', scheme: 'https', domain: 'google.com'},
]));

assert.strictEqual(result.rawValue, true);
});

describe('#isSecureRecord', () => {
it('correctly identifies insecure records', () => {
assert.strictEqual(Audit.isSecureRecord({scheme: 'http', domain: 'google.com'}), false);
assert.strictEqual(Audit.isSecureRecord({scheme: 'http', domain: '54.33.21.23'}), false);
assert.strictEqual(Audit.isSecureRecord({scheme: 'ws', domain: 'my-service.com'}), false);
assert.strictEqual(Audit.isSecureRecord({scheme: '', domain: 'google.com'}), false);
});

it('correctly identifies secure records', () => {
assert.strictEqual(Audit.isSecureRecord({scheme: 'http', domain: 'localhost'}), true);
assert.strictEqual(Audit.isSecureRecord({scheme: 'https', domain: 'google.com'}), true);
assert.strictEqual(Audit.isSecureRecord({scheme: 'wss', domain: 'my-service.com'}), true);
assert.strictEqual(Audit.isSecureRecord({scheme: 'data', domain: ''}), true);
});
assert.strictEqual(result.score, true);
});
});
});
3 changes: 1 addition & 2 deletions lighthouse-core/test/config/config-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,15 +138,14 @@ describe('Config', () => {
passes: [{
gatherers: [
'url',
'https',
'viewport'
]
}],
audits: ['is-on-https']
};

const _ = new Config(configJSON);
assert.equal(configJSON.passes[0].gatherers.length, 3);
assert.equal(configJSON.passes[0].gatherers.length, 2);
});

it('contains new copies of auditResults and aggregations', () => {
Expand Down
78 changes: 0 additions & 78 deletions lighthouse-core/test/gather/gatherers/https-test.js

This file was deleted.

Loading

0 comments on commit 74bb788

Please sign in to comment.