From 2b0a34f73dc51021ad4f6b2a5a23a133964c00ef Mon Sep 17 00:00:00 2001 From: Brendan Kenny Date: Wed, 28 Nov 2018 18:27:53 -0800 Subject: [PATCH] core(service-worker): check that start_url is within SW's scope --- lighthouse-core/audits/service-worker.js | 103 ++++++++-- .../test/audits/service-worker-test.js | 183 +++++++++++++++--- lighthouse-core/test/results/sample_v2.json | 2 +- proto/sample_v2_round_trip.json | 2 +- 4 files changed, 242 insertions(+), 48 deletions(-) diff --git a/lighthouse-core/audits/service-worker.js b/lighthouse-core/audits/service-worker.js index a89c186f05e8..d06e651f8d6f 100644 --- a/lighthouse-core/audits/service-worker.js +++ b/lighthouse-core/audits/service-worker.js @@ -15,44 +15,109 @@ class ServiceWorker extends Audit { static get meta() { return { id: 'service-worker', - title: 'Registers a service worker', - failureTitle: 'Does not register a service worker', + title: 'Registers a service worker that controls page and start_url', + failureTitle: 'Does not register a service worker that controls page and start_url', description: 'The service worker is the technology that enables your app to use many ' + 'Progressive Web App features, such as offline, add to homescreen, and push ' + 'notifications. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/registered-service-worker).', - requiredArtifacts: ['URL', 'ServiceWorker'], + requiredArtifacts: ['URL', 'ServiceWorker', 'Manifest'], }; } /** - * @param {LH.Artifacts} artifacts - * @return {LH.Audit.Product} + * Find active service workers for this origin. + * @param {Array} versions + * @param {URL} pageUrl + * @return {Array} */ - static audit(artifacts) { - const {versions, registrations} = artifacts.ServiceWorker; - const pageUrl = new URL(artifacts.URL.finalUrl); - - // Find active service workers for this origin. Match against - // artifacts.URL.finalUrl so audit accounts for any redirects. - const matchingSWVersions = versions.filter(v => v.status === 'activated') + static getVersionsForOrigin(versions, pageUrl) { + return versions + .filter(v => v.status === 'activated') .filter(v => new URL(v.scriptURL).origin === pageUrl.origin); + } - if (matchingSWVersions.length === 0) { - return {rawValue: false}; - } - + /** + * From the set of active service workers for this origin, find the controlling SW (if any) + * and return its scope URL. + * @param {Array} matchingSWVersions + * @param {Array} registrations + * @param {URL} pageUrl + * @return {string|undefined} + */ + static getControllingScopeUrl(matchingSWVersions, registrations, pageUrl) { // Find the normalized scope URLs of possibly-controlling SWs. const matchingScopeUrls = matchingSWVersions .map(v => registrations.find(r => r.registrationId === v.registrationId)) .filter(/** @return {r is LH.Crdp.ServiceWorker.ServiceWorkerRegistration} */ r => !!r) .map(r => new URL(r.scopeURL).href); - // Ensure page is included in a SW's scope. + // Find most-specific applicable scope, the one controlling the page. // See https://w3c.github.io/ServiceWorker/v1/#scope-match-algorithm - const inScope = matchingScopeUrls.some(scopeUrl => pageUrl.href.startsWith(scopeUrl)); + const pageControllingScope = matchingScopeUrls + .filter(scopeUrl => pageUrl.href.startsWith(scopeUrl)) + .sort((scopeA, scopeB) => scopeA.length - scopeB.length) + .pop(); + + return pageControllingScope; + } + + /** + * Returns a failure message if there is no start_url or if the start_url isn't + * contolled by the scopeUrl. + * @param {LH.Artifacts['Manifest']} manifest + * @param {string} scopeUrl + * @return {string|undefined} + */ + static checkStartUrl(manifest, scopeUrl) { + if (!manifest) { + return 'no start_url was found because no manifest was fetched'; + } + if (!manifest.value) { + return 'no start_url was found because manifest failed to parse as valid JSON'; + } + + const startUrl = manifest.value.start_url.value; + if (!startUrl.startsWith(scopeUrl)) { + return `the start_url ("${startUrl}") is not in the service worker's scope ("${scopeUrl}")`; + } + } + + /** + * @param {LH.Artifacts} artifacts + * @return {LH.Audit.Product} + */ + static audit(artifacts) { + // Match against artifacts.URL.finalUrl so audit accounts for any redirects. + const pageUrl = new URL(artifacts.URL.finalUrl); + const {versions, registrations} = artifacts.ServiceWorker; + + const versionsForOrigin = ServiceWorker.getVersionsForOrigin(versions, pageUrl); + if (versionsForOrigin.length === 0) { + return { + rawValue: false, + }; + } + + const controllingScopeUrl = ServiceWorker.getControllingScopeUrl(versionsForOrigin, + registrations, pageUrl); + if (!controllingScopeUrl) { + return { + rawValue: false, + explanation: `This origin has one or more service workers, however the page ("${pageUrl.href}") is not in scope.`, // eslint-disable-line max-len + }; + } + + const startUrlFailure = ServiceWorker.checkStartUrl(artifacts.Manifest, controllingScopeUrl); + if (startUrlFailure) { + return { + rawValue: false, + explanation: `This page is controlled by a service worker, however ${startUrlFailure}.`, + }; + } + // SW controls both finalUrl and start_url. return { - rawValue: inScope, + rawValue: true, }; } } diff --git a/lighthouse-core/test/audits/service-worker-test.js b/lighthouse-core/test/audits/service-worker-test.js index da0a8b243e54..95a1aedc656a 100644 --- a/lighthouse-core/test/audits/service-worker-test.js +++ b/lighthouse-core/test/audits/service-worker-test.js @@ -7,6 +7,7 @@ const ServiceWorker = require('../../audits/service-worker.js'); const URL = require('../../lib/url-shim.js'); +const manifestParser = require('../../lib/manifest-parser.js'); const assert = require('assert'); /* eslint-env jest */ @@ -19,11 +20,10 @@ function getBaseDirectory(urlStr) { /** * Create a ServiceWorker artifact from an array of SW config opts. * @param {Array<{scriptURL: string, status: string, scopeURL?: string}>} swOpts - * @param {string} finalUrl + * @return {LH.Artifact['ServiceWorker']} */ -function createArtifacts(swOpts, finalUrl) { +function createSWArtifact(swOpts) { const artifact = {versions: [], registrations: []}; - swOpts.forEach((sw, index) => { artifact.versions.push({ registrationId: `${index}`, @@ -40,33 +40,56 @@ function createArtifacts(swOpts, finalUrl) { }); }); + return artifact; +} + +/** + * Create a set of artifacts for the ServiceWorker audit. + * @param {Array<{scriptURL: string, status: string, scopeURL?: string}>} swOpts + * @param {string} finalUrl + * @param {{}}} manifestJson Manifest object or null if no manifest desired. + */ +function createArtifacts(swOpts, finalUrl, manifestJson) { + const manifestUrl = getBaseDirectory(finalUrl) + 'manifest.json'; + let Manifest; + if (manifestJson === null) { + Manifest = null; + } else if (typeof manifestJson === 'object') { + Manifest = manifestParser(JSON.stringify(manifestJson), manifestUrl, finalUrl); + } else { + throw new Error('unsupported test manifest format'); + } + return { - ServiceWorker: artifact, + ServiceWorker: createSWArtifact(swOpts), URL: {finalUrl}, + Manifest, }; } describe('Offline: service worker audit', () => { - it('passes when given a matching service worker version', () => { + it('passes when given a controlling service worker', () => { const finalUrl = 'https://example.com'; const swOpts = [{ status: 'activated', scriptURL: 'https://example.com/sw.js', }]; + const manifest = {start_url: finalUrl}; - const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl)); - assert.equal(output.rawValue, true); + const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl, manifest)); + assert.deepStrictEqual(output, {rawValue: true}); }); - it('fails when matching service worker is not activated', () => { + it('fails when controlling service worker is not activated', () => { const finalUrl = 'https://example.com'; const swOpts = [{ status: 'redundant', scriptURL: 'https://example.com/sw.js', }]; + const manifest = {start_url: finalUrl}; - const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl)); - assert.equal(output.rawValue, false); + const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl, manifest)); + assert.deepStrictEqual(output, {rawValue: false}); }); it('discards service worker registrations for other origins', () => { @@ -75,35 +98,80 @@ describe('Offline: service worker audit', () => { status: 'activated', scriptURL: 'https://other-example.com', }]; + const manifest = {start_url: finalUrl}; - const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl)); - assert.equal(output.rawValue, false); + const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl, manifest)); + assert.deepStrictEqual(output, {rawValue: false}); }); - it('fails when URL is out of scope', () => { + it('fails when page URL is out of scope', () => { const finalUrl = 'https://example.com/index.html'; const swOpts = [{ status: 'activated', scriptURL: 'https://example.com/serviceworker/sw.js', }]; + const manifest = {start_url: finalUrl}; - const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl)); - assert.equal(output.rawValue, false); + const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl, manifest)); + expect(output).toMatchObject({ + rawValue: false, + explanation: expect.stringMatching(new RegExp(`${finalUrl}.*not in scope`)), + }); }); - it('fails when explicit scopeURL puts the URL out of scope', () => { + it('fails when start_url is out of scope', () => { + const finalUrl = 'https://example.com/serviceworker/index.html'; + const swOpts = [{ + status: 'activated', + scriptURL: 'https://example.com/serviceworker/sw.js', + }]; + const startUrl = 'https://example.com/'; + const manifest = {start_url: startUrl}; + + const scopeURL = 'https://example.com/serviceworker'; + + const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl, manifest)); + expect(output).toMatchObject({ + rawValue: false, + explanation: expect.stringMatching(new RegExp(`start_url.*${startUrl}.*${scopeURL}`)), + }); + }); + + it('fails when explicit scopeURL puts the page URL out of scope', () => { const finalUrl = 'https://example.com/index.html'; const swOpts = [{ status: 'activated', scriptURL: 'https://example.com/sw.js', scopeURL: 'https://example.com/serviceworker/', }]; + const manifest = {start_url: finalUrl}; - const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl)); - assert.equal(output.rawValue, false); + const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl, manifest)); + expect(output).toMatchObject({ + rawValue: false, + explanation: expect.stringMatching(new RegExp(`${finalUrl}.*not in scope`)), + }); }); - it('passes when outside default scope but explicit scopeURL puts it back in', () => { + it('fails when explicit scopeURL puts the start_url out of scope', () => { + const finalUrl = 'https://example.com/serviceworker/index.html'; + const scopeURL = 'https://example.com/serviceworker/'; + const swOpts = [{ + status: 'activated', + scriptURL: 'https://example.com/sw.js', + scopeURL, + }]; + const startUrl = 'https://example.com/'; + const manifest = {start_url: startUrl}; + + const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl, manifest)); + expect(output).toMatchObject({ + rawValue: false, + explanation: expect.stringMatching(new RegExp(`start_url.*${startUrl}.*${scopeURL}`)), + }); + }); + + it('passes when both outside default scope but explicit scopeURL puts it back in', () => { const finalUrl = 'https://example.com/index.html'; const swOpts = [{ status: 'activated', @@ -111,9 +179,10 @@ describe('Offline: service worker audit', () => { // can happen when 'Service-Worker-Allowed' header widens max scope. scopeURL: 'https://example.com/', }]; + const manifest = {start_url: finalUrl}; - const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl)); - assert.equal(output.rawValue, true); + const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl, manifest)); + assert.deepStrictEqual(output, {rawValue: true}); }); it('passes when multiple SWs control the scope', () => { @@ -125,9 +194,10 @@ describe('Offline: service worker audit', () => { status: 'activated', scriptURL: 'https://example.com/project/sw.js', }]; + const manifest = {start_url: finalUrl}; - const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl)); - assert.equal(output.rawValue, true); + const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl, manifest)); + assert.deepStrictEqual(output, {rawValue: true}); }); it('passes when multiple SWs control the origin but only one is in scope', () => { @@ -145,9 +215,10 @@ describe('Offline: service worker audit', () => { scriptURL: 'https://example.com/project/subproject/sw.js', scopeURL: 'https://example.com/', }]; + const manifest = {start_url: finalUrl}; - const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl)); - assert.equal(output.rawValue, true); + const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl, manifest)); + assert.deepStrictEqual(output, {rawValue: true}); }); it('fails when multiple SWs control the origin but are all out of scope', () => { @@ -165,8 +236,66 @@ describe('Offline: service worker audit', () => { scriptURL: 'https://example.com/project/subproject/sw.js', scopeURL: 'https://example.com/project/', }]; + const manifest = {start_url: finalUrl}; + + const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl, manifest)); + expect(output).toMatchObject({ + rawValue: false, + explanation: expect.stringMatching(new RegExp(`${finalUrl}.*not in scope`)), + }); + }); + + it('fails when SW that controls start_url is different than SW that controls page', () => { + // Tests that most specific SW found for page. + const finalUrl = 'https://example.com/project/index.html'; + const swOpts = [{ + status: 'activated', + scriptURL: 'https://example.com/sw.js', + }, { + status: 'activated', + scriptURL: 'https://example.com/project/sw.js', + }]; + const startUrl = 'https://example.com/index.html'; + const manifest = {start_url: startUrl}; + + const scopeURL = 'https://example.com/project'; + + const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl, manifest)); + expect(output).toMatchObject({ + rawValue: false, + explanation: expect.stringMatching(new RegExp(`start_url.*${startUrl}.*${scopeURL}`)), + }); + }); - const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl)); - assert.equal(output.rawValue, false); + it('fails when a manifest was not found', () => { + const finalUrl = 'https://example.com'; + const swOpts = [{ + status: 'activated', + scriptURL: 'https://example.com/sw.js', + }]; + const manifest = null; + + const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl, manifest)); + expect(output).toMatchObject({ + rawValue: false, + explanation: expect.stringMatching(/start_url.*no manifest was fetched/), + }); + }); + + it('fails when a manifest is invalid', () => { + const finalUrl = 'https://example.com'; + const swOpts = [{ + status: 'activated', + scriptURL: 'https://example.com/sw.js', + }]; + + const artifacts = createArtifacts(swOpts, finalUrl, {}); + artifacts.Manifest = manifestParser('{,;}', finalUrl, finalUrl); + + const output = ServiceWorker.audit(artifacts); + expect(output).toMatchObject({ + rawValue: false, + explanation: expect.stringMatching(/start_url.*manifest failed to parse as valid JSON/), + }); }); }); diff --git a/lighthouse-core/test/results/sample_v2.json b/lighthouse-core/test/results/sample_v2.json index 44cf8eeb0766..90bf9de8b74b 100644 --- a/lighthouse-core/test/results/sample_v2.json +++ b/lighthouse-core/test/results/sample_v2.json @@ -49,7 +49,7 @@ }, "service-worker": { "id": "service-worker", - "title": "Does not register a service worker", + "title": "Does not register a service worker that controls page and start_url", "description": "The service worker is the technology that enables your app to use many Progressive Web App features, such as offline, add to homescreen, and push notifications. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/registered-service-worker).", "score": 0, "scoreDisplayMode": "binary", diff --git a/proto/sample_v2_round_trip.json b/proto/sample_v2_round_trip.json index 2febe8a80b6e..212ce4a39a07 100644 --- a/proto/sample_v2_round_trip.json +++ b/proto/sample_v2_round_trip.json @@ -1895,7 +1895,7 @@ "id": "service-worker", "score": 0.0, "scoreDisplayMode": "binary", - "title": "Does not register a service worker" + "title": "Does not register a service worker that controls page and start_url" }, "speed-index": { "description": "Speed Index shows how quickly the contents of a page are visibly populated. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/speed-index).",