From 3815f12b89c9f5ac23b563c07398e7b582e2c9de Mon Sep 17 00:00:00 2001 From: Ward Peeters Date: Sun, 29 Apr 2018 17:18:31 +0200 Subject: [PATCH] Only allow same origin (domain + subdomains) for preload --- lighthouse-core/audits/uses-rel-preload.js | 20 ++++++++- lighthouse-core/lib/url-shim.js | 20 +++++++++ .../test/audits/uses-rel-preload-test.js | 44 ++++++++++++++++--- 3 files changed, 77 insertions(+), 7 deletions(-) diff --git a/lighthouse-core/audits/uses-rel-preload.js b/lighthouse-core/audits/uses-rel-preload.js index 2724d26e808a..28f4e6dd8469 100644 --- a/lighthouse-core/audits/uses-rel-preload.js +++ b/lighthouse-core/audits/uses-rel-preload.js @@ -5,6 +5,7 @@ */ 'use strict'; +const URL = require('../lib/url-shim'); const Audit = require('./audit'); const UnusedBytes = require('./byte-efficiency/byte-efficiency-audit'); const THRESHOLD_IN_MS = 100; @@ -58,6 +59,20 @@ class UsesRelPreloadAudit extends Audit { return requests; } + /** + * + * @param {LH.WebInspector.NetworkRequest} request + * @param {LH.WebInspector.NetworkRequest} mainResource + * @return {boolean} + */ + static shouldPreload(request, mainResource) { + if (request._isLinkPreload || request.protocol === 'data') { + return false; + } + + return URL.rootDomainsMatch(request.url, mainResource.url); + } + /** * Computes the estimated effect of preloading all the resources. * @param {Set} urls The array of byte savings results per resource @@ -117,6 +132,7 @@ class UsesRelPreloadAudit extends Audit { const originalNode = originalNodesByRecord.get(node.record); const timingAfter = simulationAfterChanges.nodeTimings.get(node); const timingBefore = simulationBeforeChanges.nodeTimings.get(originalNode); + // @ts-ignore TODO(phulce): fix timing typedef const wastedMs = Math.round(timingBefore.endTime - timingAfter.endTime); if (wastedMs < THRESHOLD_IN_MS) continue; @@ -163,8 +179,8 @@ class UsesRelPreloadAudit extends Audit { /** @type {Set} */ const urls = new Set(); for (const networkRecord of criticalRequests) { - if (!networkRecord._isLinkPreload && networkRecord.protocol !== 'data') { - urls.add(networkRecord._url); + if (UsesRelPreloadAudit.shouldPreload(networkRecord, mainResource)) { + urls.add(networkRecord.url); } } diff --git a/lighthouse-core/lib/url-shim.js b/lighthouse-core/lib/url-shim.js index b2aeca30c5e8..9fd6512a3cce 100644 --- a/lighthouse-core/lib/url-shim.js +++ b/lighthouse-core/lib/url-shim.js @@ -87,6 +87,26 @@ class URLShim extends URL { } } + /** + * Check if rootDomains matches + * + * @param {string} urlA + * @param {string} urlB + */ + static rootDomainsMatch(urlA, urlB) { + const urlAInfo = new URL(urlA); + const urlBInfo = new URL(urlB); + + if (!urlAInfo.host || !urlBInfo.host) { + return false; + } + + const urlARootDomain = urlAInfo.host.split('.').slice(-2).join('.'); + const urlBRootDomain = urlBInfo.host.split('.').slice(-2).join('.'); + + return urlARootDomain === urlBRootDomain; + } + /** * @param {string} url * @param {{numPathParts: number, preserveQuery: boolean, preserveHost: boolean}=} options diff --git a/lighthouse-core/test/audits/uses-rel-preload-test.js b/lighthouse-core/test/audits/uses-rel-preload-test.js index d96db4e75cb4..8790281fb630 100644 --- a/lighthouse-core/test/audits/uses-rel-preload-test.js +++ b/lighthouse-core/test/audits/uses-rel-preload-test.js @@ -53,10 +53,14 @@ describe('Performance: uses-rel-preload audit', () => { const mainDocumentNode = buildNode(2, 'http://www.example.com'); const scriptNode = buildNode(3, 'http://www.example.com/script.js'); const scriptAddedNode = buildNode(4, 'http://www.example.com/script-added.js'); + const scriptSubNode = buildNode(5, 'http://sub.example.com/script-sub.js'); + const scriptOtherNode = buildNode(6, 'http://otherdomain.com/script-other.js'); mainDocumentNode.addDependency(rootNode); scriptNode.addDependency(mainDocumentNode); scriptAddedNode.addDependency(scriptNode); + scriptSubNode.addDependency(scriptNode); + scriptOtherNode.addDependency(scriptNode); mockGraph = rootNode; mockSimulator = { @@ -68,19 +72,31 @@ describe('Performance: uses-rel-preload audit', () => { const mainDocumentNodeLocal = nodesByUrl.get(mainDocumentNode.record.url); const scriptNodeLocal = nodesByUrl.get(scriptNode.record.url); const scriptAddedNodeLocal = nodesByUrl.get(scriptAddedNode.record.url); + const scriptSubNodeLocal = nodesByUrl.get(scriptSubNode.record.url); + const scriptOtherNodeLocal = nodesByUrl.get(scriptOtherNode.record.url); const nodeTimings = new Map([ [rootNodeLocal, {starTime: 0, endTime: 500}], [mainDocumentNodeLocal, {startTime: 500, endTime: 1000}], [scriptNodeLocal, {startTime: 1000, endTime: 2000}], [scriptAddedNodeLocal, {startTime: 2000, endTime: 3250}], + [scriptSubNodeLocal, {startTime: 2000, endTime: 3000}], + [scriptOtherNodeLocal, {startTime: 2000, endTime: 3500}], ]); if (scriptAddedNodeLocal.getDependencies()[0] === mainDocumentNodeLocal) { nodeTimings.set(scriptAddedNodeLocal, {startTime: 1000, endTime: 2000}); } - return {timeInMs: 3250, nodeTimings}; + if (scriptSubNodeLocal.getDependencies()[0] === mainDocumentNodeLocal) { + nodeTimings.set(scriptSubNodeLocal, {startTime: 1000, endTime: 2000}); + } + + if (scriptOtherNodeLocal.getDependencies()[0] === mainDocumentNodeLocal) { + nodeTimings.set(scriptOtherNodeLocal, {startTime: 1000, endTime: 2500}); + } + + return {timeInMs: 3500, nodeTimings}; }, }; @@ -92,17 +108,27 @@ describe('Performance: uses-rel-preload audit', () => { { requestId: '2', _isLinkPreload: false, - _url: 'http://www.example.com', + url: 'http://www.example.com', }, { requestId: '3', _isLinkPreload: false, - _url: 'http://www.example.com/script.js', + url: 'http://www.example.com/script.js', }, { requestId: '4', _isLinkPreload: false, - _url: 'http://www.example.com/script-added.js', + url: 'http://www.example.com/script-added.js', + }, + { + requestId: '5', + _isLinkPreload: false, + url: 'http://sub.example.com/script-sub.js', + }, + { + requestId: '6', + _isLinkPreload: false, + url: 'http://otherdomain.com/script-other.js', }, ]; @@ -119,6 +145,14 @@ describe('Performance: uses-rel-preload audit', () => { request: networkRecords[2], children: {}, }, + '5': { + request: networkRecords[3], + children: {}, + }, + '6': { + request: networkRecords[4], + children: {}, + }, }, }, }, @@ -130,7 +164,7 @@ describe('Performance: uses-rel-preload audit', () => { return UsesRelPreload.audit(mockArtifacts(networkRecords, chains, mainResource), {}).then( output => { assert.equal(output.rawValue, 1250); - assert.equal(output.details.items.length, 1); + assert.equal(output.details.items.length, 2); } ); });