Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

tests(smokehouse): adopt URLSearchParams for querystring manipulation #3941

Merged
merged 5 commits into from
Dec 8, 2017
Merged

tests(smokehouse): adopt URLSearchParams for querystring manipulation #3941

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
50147fe
bump whatwg-url to latest to get their URLSearchParams
paulirish Nov 29, 2017
58a563b
use real URLSearchParams for header parsing.
paulirish Nov 29, 2017
bd23a28
fix looping over urlsearchparams
paulirish Nov 30, 2017
cd89a2e
latest whatwg-url adds a trailing slash to chrome://version
paulirish Nov 30, 2017
4bbb532
remove trailing slashes on chrome:// urls, as per spec
paulirish Dec 2, 2017
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
28 changes: 13 additions & 15 deletions lighthouse-cli/test/fixtures/static-server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, '../../../');
Expand Down Expand Up @@ -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'));
for (const [headerName, headerValue] of extraHeaders) {
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'));
}
}

Expand Down
20 changes: 10 additions & 10 deletions lighthouse-cli/test/smokehouse/seo/expectations.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)}`)
Copy link
Collaborator

Choose a reason for hiding this comment

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

oh nice catch we should've been using encodeURIComponent here 👍

.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: '<http://example.com>;rel="alternate";hreflang="xx"',
}]);
const failureHeaders = headersParam([[
'x-robots-tag',
'none',
], [
'link',
'<http://example.com>;rel="alternate";hreflang="xx"',
]]);

/**
* Expected Lighthouse audit values for seo tests
Expand Down
6 changes: 6 additions & 0 deletions lighthouse-core/lib/url-shim.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.';
Expand Down Expand Up @@ -104,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://');
}

Expand Down
9 changes: 9 additions & 0 deletions lighthouse-core/test/lib/url-shim-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
});
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
33 changes: 26 additions & 7 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -4168,26 +4182,31 @@ 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"
dependencies:
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"
Expand Down