From 50147fe58d9bda13fbd4b8278dfb836a38b7e5b6 Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Tue, 28 Nov 2017 18:32:35 -0800 Subject: [PATCH 1/5] bump whatwg-url to latest to get their URLSearchParams --- package.json | 2 +- yarn.lock | 33 ++++++++++++++++++++++++++------- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/package.json b/package.json index 960a20459641..4c4199b54ca5 100644 --- a/package.json +++ b/package.json @@ -96,7 +96,7 @@ "semver": "^5.3.0", "speedline": "1.3.0", "update-notifier": "^2.1.0", - "whatwg-url": "4.0.0", + "whatwg-url": "^6.3.0", "ws": "3.3.2", "yargs": "3.32.0", "yargs-parser": "7.0.0" diff --git a/yarn.lock b/yarn.lock index a6ac6b86c6a1..acd7b7047b38 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2691,6 +2691,10 @@ lodash.restparam@^3.0.0: version "3.6.1" resolved "https://registry.yarnpkg.com/lodash.restparam/-/lodash.restparam-3.6.1.tgz#936a4e309ef330a7645ed4145986c85ae5b20805" +lodash.sortby@^4.7.0: + version "4.7.0" + resolved "https://registry.yarnpkg.com/lodash.sortby/-/lodash.sortby-4.7.0.tgz#edd14c824e2cc9c1e0b0a1b42bb5210516a42438" + lodash.template@^3.0.0: version "3.6.2" resolved "https://registry.yarnpkg.com/lodash.template/-/lodash.template-3.6.2.tgz#f8cdecc6169a255be9098ae8b0c53d378931d14f" @@ -3266,6 +3270,10 @@ punycode@^1.4.1: version "1.4.1" resolved "https://registry.yarnpkg.com/punycode/-/punycode-1.4.1.tgz#c0d5a63b2718800ad8e1eb0fa5269c84dd41845e" +punycode@^2.1.0: + version "2.1.0" + resolved "https://registry.yarnpkg.com/punycode/-/punycode-2.1.0.tgz#5f863edc89b96db09074bad7947bf09056ca4e7d" + q@^1.4.1: version "1.5.1" resolved "https://registry.yarnpkg.com/q/-/q-1.5.1.tgz#7e32f75b41381291d04611f1bf14109ac00651d7" @@ -3979,6 +3987,12 @@ tough-cookie@^2.3.2, tough-cookie@~2.3.0: dependencies: punycode "^1.4.1" +tr46@^1.0.0: + version "1.0.1" + resolved "https://registry.yarnpkg.com/tr46/-/tr46-1.0.1.tgz#a8b13fd6bfd2489519674ccde55ba3693b706d09" + dependencies: + punycode "^2.1.0" + tr46@~0.0.3: version "0.0.3" resolved "https://registry.yarnpkg.com/tr46/-/tr46-0.0.3.tgz#8184fd347dac9cdc185992f3a6622e14b9d9ab6a" @@ -4168,19 +4182,16 @@ webidl-conversions@^4.0.0: version "4.0.1" resolved "https://registry.yarnpkg.com/webidl-conversions/-/webidl-conversions-4.0.1.tgz#8015a17ab83e7e1b311638486ace81da6ce206a0" +webidl-conversions@^4.0.1: + version "4.0.2" + resolved "https://registry.yarnpkg.com/webidl-conversions/-/webidl-conversions-4.0.2.tgz#a855980b1f0b6b359ba1d5d9fb39ae941faa63ad" + whatwg-encoding@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/whatwg-encoding/-/whatwg-encoding-1.0.1.tgz#3c6c451a198ee7aec55b1ec61d0920c67801a5f4" dependencies: iconv-lite "0.4.13" -whatwg-url@4.0.0: - version "4.0.0" - resolved "https://registry.yarnpkg.com/whatwg-url/-/whatwg-url-4.0.0.tgz#5be362f0b6e2f8760f7260df6e0e1df536f5479c" - dependencies: - tr46 "~0.0.3" - webidl-conversions "^3.0.0" - whatwg-url@^4.3.0: version "4.7.0" resolved "https://registry.yarnpkg.com/whatwg-url/-/whatwg-url-4.7.0.tgz#202035ac1955b087cdd20fa8b58ded3ab1cd2af5" @@ -4188,6 +4199,14 @@ whatwg-url@^4.3.0: tr46 "~0.0.3" webidl-conversions "^3.0.0" +whatwg-url@^6.3.0: + version "6.3.0" + resolved "https://registry.yarnpkg.com/whatwg-url/-/whatwg-url-6.3.0.tgz#597ee5488371abe7922c843397ddec1ae94c048d" + dependencies: + lodash.sortby "^4.7.0" + tr46 "^1.0.0" + webidl-conversions "^4.0.1" + which@^1.1.1, which@^1.2.10, which@^1.2.8, which@^1.2.9: version "1.2.11" resolved "https://registry.yarnpkg.com/which/-/which-1.2.11.tgz#c8b2eeea6b8c1659fa7c1dd4fdaabe9533dc5e8b" From 58a563b569da948e8e96b826db20d63a0275d0a5 Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Tue, 28 Nov 2017 18:32:55 -0800 Subject: [PATCH 2/5] use real URLSearchParams for header parsing. --- lighthouse-cli/test/fixtures/static-server.js | 26 +++++++++---------- .../test/smokehouse/seo/expectations.js | 20 +++++++------- lighthouse-core/lib/url-shim.js | 3 +++ 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/lighthouse-cli/test/fixtures/static-server.js b/lighthouse-cli/test/fixtures/static-server.js index 69e1c3c9148f..9a8150810478 100644 --- a/lighthouse-cli/test/fixtures/static-server.js +++ b/lighthouse-cli/test/fixtures/static-server.js @@ -12,6 +12,7 @@ const path = require('path'); const fs = require('fs'); const parseQueryString = require('querystring').parse; const parseURL = require('url').parse; +const URLSearchParams = require('../../../lighthouse-core/lib/url-shim').URLSearchParams; const HEADER_SAFELIST = new Set(['x-robots-tag', 'link']); const lhRootDirPath = path.join(__dirname, '../../../'); @@ -64,32 +65,29 @@ function requestHandler(request, response) { let delay = 0; if (queryString) { + const params = new URLSearchParams(queryString); // set document status-code - if (typeof queryString.status_code !== 'undefined') { - statusCode = parseInt(queryString.status_code, 10); + if (params.has('status_code')) { + statusCode = parseInt(params.get('status_code'), 10); } // set delay of request when present - if (typeof queryString.delay !== 'undefined') { - delay = parseInt(queryString.delay, 10) || 2000; + if (params.has('delay')) { + delay = parseInt(params.get('delay'), 10) || 2000; } - if (typeof queryString.extra_header !== 'undefined') { - let extraHeaders = queryString.extra_header; - extraHeaders = Array.isArray(extraHeaders) ? extraHeaders : [extraHeaders]; - - extraHeaders.forEach(header => { - const [headerName, ...headerValue] = header.split(':'); - + if (params.has('extra_header')) { + const extraHeaders = new URLSearchParams(params.get('extra_header')); + extraHeaders.forEach((headerValue, headerName) => { if (HEADER_SAFELIST.has(headerName.toLowerCase())) { - headers[headerName] = headerValue.join(':'); + headers[headerName] = headerValue; } }); } // redirect url to new url if present - if (typeof queryString.redirect !== 'undefined') { - return setTimeout(sendRedirect, delay, queryString.redirect); + if (params.has('redirect')) { + return setTimeout(sendRedirect, delay, params.get('redirect')); } } diff --git a/lighthouse-cli/test/smokehouse/seo/expectations.js b/lighthouse-cli/test/smokehouse/seo/expectations.js index ae55871da27f..b63dcd02b879 100644 --- a/lighthouse-cli/test/smokehouse/seo/expectations.js +++ b/lighthouse-cli/test/smokehouse/seo/expectations.js @@ -5,20 +5,20 @@ */ 'use strict'; const BASE_URL = 'http://localhost:10200/seo/'; +const URLSearchParams = require('../../../../lighthouse-core/lib/url-shim').URLSearchParams; function headersParam(headers) { - return headers - .map(({name, value}) => `extra_header=${name}:${encodeURI(value)}`) - .join('&'); + const headerString = new URLSearchParams(headers).toString(); + return new URLSearchParams([['extra_header', headerString]]).toString(); } -const failureHeaders = headersParam([{ - name: 'x-robots-tag', - value: 'none', -}, { - name: 'link', - value: ';rel="alternate";hreflang="xx"', -}]); +const failureHeaders = headersParam([[ + 'x-robots-tag', + 'none', +], [ + 'link', + ';rel="alternate";hreflang="xx"', +]]); /** * Expected Lighthouse audit values for seo tests diff --git a/lighthouse-core/lib/url-shim.js b/lighthouse-core/lib/url-shim.js index 82ec37e621cb..02fc5d90309d 100644 --- a/lighthouse-core/lib/url-shim.js +++ b/lighthouse-core/lib/url-shim.js @@ -18,6 +18,9 @@ const Util = require('../report/v2/renderer/util.js'); // https://github.com/GoogleChrome/lighthouse/issues/1186 const URL = (typeof self !== 'undefined' && self.URL) || require('whatwg-url').URL; +URL.URLSearchParams = (typeof self !== 'undefined' && self.URLSearchParams) || + require('whatwg-url').URLSearchParams; + URL.INVALID_URL_DEBUG_STRING = 'Lighthouse was unable to determine the URL of some script executions. ' + 'It\'s possible a Chrome extension or other eval\'d code is the source.'; From bd23a28cab1d4c0edaa05a505189d78b4bc7d5c1 Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Wed, 29 Nov 2017 16:48:03 -0800 Subject: [PATCH 3/5] fix looping over urlsearchparams --- lighthouse-cli/test/fixtures/static-server.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lighthouse-cli/test/fixtures/static-server.js b/lighthouse-cli/test/fixtures/static-server.js index 9a8150810478..86f4e35d58e2 100644 --- a/lighthouse-cli/test/fixtures/static-server.js +++ b/lighthouse-cli/test/fixtures/static-server.js @@ -78,11 +78,11 @@ function requestHandler(request, response) { if (params.has('extra_header')) { const extraHeaders = new URLSearchParams(params.get('extra_header')); - extraHeaders.forEach((headerValue, headerName) => { + for (const [headerName, headerValue] of extraHeaders) { if (HEADER_SAFELIST.has(headerName.toLowerCase())) { headers[headerName] = headerValue; } - }); + } } // redirect url to new url if present From cd89a2eec69ab7de823e33471280c58e307612cb Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Wed, 29 Nov 2017 17:03:51 -0800 Subject: [PATCH 4/5] latest whatwg-url adds a trailing slash to chrome://version --- lighthouse-cli/test/cli/run-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-cli/test/cli/run-test.js b/lighthouse-cli/test/cli/run-test.js index bca650a20d11..e70f187a6a3e 100644 --- a/lighthouse-cli/test/cli/run-test.js +++ b/lighthouse-cli/test/cli/run-test.js @@ -23,7 +23,7 @@ const getFlags = require('../../cli-flags').getFlags; describe('CLI run', function() { it('runLighthouse completes a LH round trip', () => { - const url = 'chrome://version'; + const url = 'chrome://version/'; const filename = path.join(process.cwd(), 'run.ts.results.json'); const flags = getFlags(`--output=json --output-path=${filename} ${url}`); return run.runLighthouse(url, flags, fastConfig).then(passedResults => { From 4bbb5327d0c46a03fc6452e583d544478a7256d7 Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Fri, 1 Dec 2017 16:50:14 -0800 Subject: [PATCH 5/5] remove trailing slashes on chrome:// urls, as per spec --- lighthouse-cli/test/cli/run-test.js | 2 +- lighthouse-core/lib/url-shim.js | 3 +++ lighthouse-core/test/lib/url-shim-test.js | 9 +++++++++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lighthouse-cli/test/cli/run-test.js b/lighthouse-cli/test/cli/run-test.js index e70f187a6a3e..bca650a20d11 100644 --- a/lighthouse-cli/test/cli/run-test.js +++ b/lighthouse-cli/test/cli/run-test.js @@ -23,7 +23,7 @@ const getFlags = require('../../cli-flags').getFlags; describe('CLI run', function() { it('runLighthouse completes a LH round trip', () => { - const url = 'chrome://version/'; + const url = 'chrome://version'; const filename = path.join(process.cwd(), 'run.ts.results.json'); const flags = getFlags(`--output=json --output-path=${filename} ${url}`); return run.runLighthouse(url, flags, fastConfig).then(passedResults => { diff --git a/lighthouse-core/lib/url-shim.js b/lighthouse-core/lib/url-shim.js index 02fc5d90309d..b6d2fa2d3c4c 100644 --- a/lighthouse-core/lib/url-shim.js +++ b/lighthouse-core/lib/url-shim.js @@ -107,6 +107,9 @@ URL.elideDataURI = function elideDataURI(url) { // As a result, the network URL (chrome://chrome/settings/) doesn't match the final document URL (chrome://settings/). function rewriteChromeInternalUrl(url) { if (!url || !url.startsWith('chrome://')) return url; + // Chrome adds a trailing slash to `chrome://` URLs, but the spec does not. + // https://github.com/GoogleChrome/lighthouse/pull/3941#discussion_r154026009 + if (url.endsWith('/')) url = url.replace(/\/$/, ''); return url.replace(/^chrome:\/\/chrome\//, 'chrome://'); } diff --git a/lighthouse-core/test/lib/url-shim-test.js b/lighthouse-core/test/lib/url-shim-test.js index e14bb1c29429..b07942bd85c6 100644 --- a/lighthouse-core/test/lib/url-shim-test.js +++ b/lighthouse-core/test/lib/url-shim-test.js @@ -218,6 +218,15 @@ describe('URL Shim', () => { assert.ok(URL.equalWithExcludedFragments(...pair)); }); + // https://github.com/GoogleChrome/lighthouse/pull/3941#discussion_r154026009 + it('canonicalizes chrome:// urls without a trailing slash', () => { + const pair = [ + 'chrome://version/', + 'chrome://version', + ]; + assert.ok(URL.equalWithExcludedFragments(...pair)); + }); + it('returns false for invalid URLs', () => { assert.ok(!URL.equalWithExcludedFragments('utter nonsense', 'http://example.com')); });