Skip to content

Commit

Permalink
Merge branch 'main' into score-update-for-new-sort
Browse files Browse the repository at this point in the history
  • Loading branch information
adamraine committed Oct 4, 2023
2 parents dd88edf + e2659f3 commit 294f0a0
Show file tree
Hide file tree
Showing 22 changed files with 250 additions and 178 deletions.
1 change: 0 additions & 1 deletion build/build-bundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,6 @@ async function buildBundle(entryPath, distPath, opts = {minify: true}) {
// resolved eventually.
plugins.partialLoaders.inlineFs({
verbose: Boolean(process.env.DEBUG),
ignorePaths: [require.resolve('puppeteer-core/lib/esm/puppeteer/common/Page.js')],
}),
plugins.partialLoaders.rmGetModuleDirectory,
plugins.partialLoaders.replaceText({
Expand Down
2 changes: 1 addition & 1 deletion build/gh-pages-app.js
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ class GhPagesApp {

if (this.preloadScripts.length) {
const preloads = this.preloadScripts.map(fileName =>
`<link rel="preload" href="${fileName}" as="script" crossorigin="anonymous" />`
`<link rel="preload" href="./src/${fileName}" as="script" crossorigin="anonymous" />`
).join('\n');
const endHeadIndex = htmlSrc.indexOf('</head>');
if (endHeadIndex === -1) {
Expand Down
12 changes: 0 additions & 12 deletions cli/test/smokehouse/test-definitions/dobetterweb.js
Original file line number Diff line number Diff line change
Expand Up @@ -494,18 +494,6 @@ const expectations = {
}],
},
},
{
// Support for this was added in M109
// https://crbug.com/1350944
_maxChromiumVersion: '108',
reason: 'Pages that have requested notifications permissions are not currently eligible for back/forward cache.',
failureType: 'Pending browser support',
subItems: {
items: [{
frameUrl: 'http://localhost:10200/dobetterweb/dbw_tester.html',
}],
},
},
{
// This issue only appears in the DevTools runner for some reason.
// TODO: Investigate why this doesn't happen on the CLI runner.
Expand Down
6 changes: 3 additions & 3 deletions clients/lightrider/lightrider-entry.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
import {Buffer} from 'buffer';

import log from 'lighthouse-logger';
import {CDPBrowser} from 'puppeteer-core/lib/esm/puppeteer/common/Browser.js';
import {Connection as PptrConnection} from 'puppeteer-core/lib/esm/puppeteer/common/Connection.js';
import {CdpBrowser} from 'puppeteer-core/lib/esm/puppeteer/cdp/Browser.js';
import {Connection as PptrConnection} from 'puppeteer-core/lib/esm/puppeteer/cdp/Connection.js';

import lighthouse, * as api from '../../core/index.js';
import {LighthouseError} from '../../core/lib/lh-error.js';
Expand Down Expand Up @@ -46,7 +46,7 @@ async function getPageFromConnection(connection) {

const pptrConnection = new PptrConnection(mainTargetInfo.url, transport);

const browser = await CDPBrowser._create(
const browser = await CdpBrowser._create(
'chrome',
pptrConnection,
[] /* contextIds */,
Expand Down
2 changes: 1 addition & 1 deletion core/audits/csp-xss.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const UIStrings = {
/** Summary text for the results of a Lighthouse audit that evaluates the security of a page's CSP. This is displayed if no CSP is being enforced. "CSP" stands for "Content Security Policy". "CSP" does not need to be translated. */
noCsp: 'No CSP found in enforcement mode',
/** Message shown when one or more CSPs are defined in a <meta> tag. Shown in a table with a list of other CSP bypasses and warnings. "CSP" stands for "Content Security Policy". "CSP" and "HTTP" do not need to be translated. */
metaTagMessage: 'The page contains a CSP defined in a <meta> tag. ' +
metaTagMessage: 'The page contains a CSP defined in a `<meta>` tag. ' +
'Consider moving the CSP to an HTTP header or ' +
'defining another strict CSP in an HTTP header.',
/** Label for a column in a data table; entries will be a directive of a CSP. "CSP" stands for "Content Security Policy". */
Expand Down
4 changes: 2 additions & 2 deletions core/audits/installable-manifest.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ const UIStrings = {
/** Error message explaining that the provided manifest URL is invalid. */
'start-url-not-valid': `Manifest start URL is not valid`,
/** Error message explaining that the provided manifest does not contain a name or short_name field. */
'manifest-missing-name-or-short-name': `Manifest does not contain a 'name' or 'short_name' field`,
'manifest-missing-name-or-short-name': 'Manifest does not contain a `name` or `short_name` field',
/** Error message explaining that the manifest display property must be one of 'standalone', 'fullscreen', or 'minimal-ui'. */
'manifest-display-not-supported': `Manifest 'display' property must be one of 'standalone', 'fullscreen', or 'minimal-ui'`,
'manifest-display-not-supported': 'Manifest `display` property must be one of `standalone`, `fullscreen`, or `minimal-ui`',
/** Error message explaining that the manifest could not be fetched, might be empty, or could not be parsed. */
'manifest-empty': `Manifest could not be fetched, is empty, or could not be parsed`,
/**
Expand Down
9 changes: 8 additions & 1 deletion core/audits/long-tasks.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {MainThreadTasks} from '../computed/main-thread-tasks.js';
import {PageDependencyGraph} from '../computed/page-dependency-graph.js';
import {LoadSimulator} from '../computed/load-simulator.js';
import {getJavaScriptURLs, getAttributableURLForTask} from '../lib/tracehouse/task-summary.js';
import {TotalBlockingTime} from '../computed/metrics/total-blocking-time.js';

/** We don't always have timing data for short tasks, if we're missing timing data. Treat it as though it were 0ms. */
const DEFAULT_TIMING = {startTime: 0, endTime: 0, duration: 0};
Expand Down Expand Up @@ -66,8 +67,8 @@ class LongTasks extends Audit {
scoreDisplayMode: Audit.SCORING_MODES.INFORMATIVE,
title: str_(UIStrings.title),
description: str_(UIStrings.description),
requiredArtifacts: ['traces', 'devtoolsLogs', 'URL', 'GatherContext'],
guidanceLevel: 1,
requiredArtifacts: ['traces', 'devtoolsLogs', 'URL'],
};
}

Expand Down Expand Up @@ -180,6 +181,9 @@ class LongTasks extends Audit {
const devtoolsLog = artifacts.devtoolsLogs[LongTasks.DEFAULT_PASS];
const networkRecords = await NetworkRecords.request(devtoolsLog, context);

const metricComputationData = Audit.makeMetricComputationDataInput(artifacts, context);
const tbtResult = await TotalBlockingTime.request(metricComputationData, context);

/** @type {Map<LH.TraceEvent, LH.Gatherer.Simulation.NodeTiming>|undefined} */
let taskTimingsByEvent;

Expand Down Expand Up @@ -245,6 +249,9 @@ class LongTasks extends Audit {
notApplicable: results.length === 0,
details: tableDetails,
displayValue,
metricSavings: {
TBT: tbtResult.timing,
},
};
}
}
Expand Down
2 changes: 2 additions & 0 deletions core/gather/session.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class ProtocolSession extends CrdpEventEmitter {
this._nextProtocolTimeout = undefined;

this._handleProtocolEvent = this._handleProtocolEvent.bind(this);
// @ts-expect-error Puppeteer expects the handler params to be type `unknown`
this._cdpSession.on('*', this._handleProtocolEvent);
}

Expand Down Expand Up @@ -106,6 +107,7 @@ class ProtocolSession extends CrdpEventEmitter {
* @return {Promise<void>}
*/
async dispose() {
// @ts-expect-error Puppeteer expects the handler params to be type `unknown`
this._cdpSession.off('*', this._handleProtocolEvent);
await this._cdpSession.detach();
}
Expand Down
26 changes: 13 additions & 13 deletions core/lib/csp-evaluator.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,31 +19,31 @@ import {isIcuMessage} from '../../shared/localization/format.js';

const UIStrings = {
/** Message shown when a CSP does not have a base-uri directive. Shown in a table with a list of other CSP vulnerabilities and suggestions. "CSP" stands for "Content Security Policy". "base-uri", "'none'", and "'self'" do not need to be translated. */
missingBaseUri: 'Missing base-uri allows injected <base> tags to set the base URL for all ' +
missingBaseUri: 'Missing `base-uri` allows injected `<base>` tags to set the base URL for all ' +
'relative URLs (e.g. scripts) to an attacker controlled domain. ' +
'Consider setting base-uri to \'none\' or \'self\'.',
'Consider setting `base-uri` to `\'none\'` or `\'self\'`.',
/** Message shown when a CSP does not have a script-src directive. Shown in a table with a list of other CSP vulnerabilities and suggestions. "CSP" stands for "Content Security Policy". "script-src" does not need to be translated. */
missingScriptSrc: 'script-src directive is missing. ' +
missingScriptSrc: '`script-src` directive is missing. ' +
'This can allow the execution of unsafe scripts.',
/** Message shown when a CSP does not have a script-src directive. Shown in a table with a list of other CSP vulnerabilities and suggestions. "CSP" stands for "Content Security Policy". "object-src" and "'none'" do not need to be translated. */
missingObjectSrc: 'Missing object-src allows the injection of plugins ' +
'that execute unsafe scripts. Consider setting object-src to \'none\' if you can.',
missingObjectSrc: 'Missing `object-src` allows the injection of plugins ' +
'that execute unsafe scripts. Consider setting `object-src` to `\'none\'` if you can.',
/** Message shown when a CSP uses a domain allowlist to filter out malicious scripts. Shown in a table with a list of other CSP vulnerabilities and suggestions. "CSP" stands for "Content Security Policy". "CSP", "'strict-dynamic'", "nonces", and "hashes" do not need to be translated. "allowlists" can be interpreted as "whitelist". */
strictDynamic: 'Host allowlists can frequently be bypassed. Consider using ' +
'CSP nonces or hashes instead, along with \'strict-dynamic\' if necessary.',
'CSP nonces or hashes instead, along with `\'strict-dynamic\'` if necessary.',
/** Message shown when a CSP allows inline scripts to be run in the page. Shown in a table with a list of other CSP vulnerabilities and suggestions. "CSP" stands for "Content Security Policy". "CSP", "'unsafe-inline'", "nonces", and "hashes" do not need to be translated. */
unsafeInline: '\'unsafe-inline\' allows the execution of unsafe in-page scripts ' +
unsafeInline: '`\'unsafe-inline\'` allows the execution of unsafe in-page scripts ' +
'and event handlers. Consider using CSP nonces or hashes to allow scripts individually.',
/** Message shown when a CSP is not backwards compatible with browsers that do not support CSP nonces/hashes. Shown in a table with a list of other CSP vulnerabilities and suggestions. "CSP" stands for "Content Security Policy". "'unsafe-inline'", "nonces", and "hashes" do not need to be translated. */
unsafeInlineFallback: 'Consider adding \'unsafe-inline\' (ignored by browsers supporting ' +
unsafeInlineFallback: 'Consider adding `\'unsafe-inline\'` (ignored by browsers supporting ' +
'nonces/hashes) to be backward compatible with older browsers.',
/** Message shown when a CSP is not backwards compatible with browsers that do not support the 'strict-dynamic' keyword. Shown in a table with a list of other CSP vulnerabilities and suggestions. "CSP" stands for "Content Security Policy". "http:", "https:", and "'strict-dynamic'" do not need to be translated. */
allowlistFallback: 'Consider adding https: and http: URL schemes (ignored by browsers ' +
'supporting \'strict-dynamic\') to be backward compatible with older browsers.',
'supporting `\'strict-dynamic\'`) to be backward compatible with older browsers.',
/** Message shown when a CSP only provides a reporting destination through the report-to directive. Shown in a table with a list of other CSP vulnerabilities and suggestions. "CSP" stands for "Content Security Policy". "report-to", "report-uri", and "Chromium" do not need to be translated. */
reportToOnly: 'The reporting destination is only configured via the report-to directive. ' +
'This directive is only supported in Chromium-based browsers so it is ' +
'recommended to also use a report-uri directive.',
'recommended to also use a `report-uri` directive.',
/** Message shown when a CSP does not provide a reporting destination. Shown in a table with a list of other CSP vulnerabilities and suggestions. "CSP" stands for "Content Security Policy". "CSP" does not need to be translated. */
reportingDestinationMissing: 'No CSP configures a reporting destination. ' +
'This makes it difficult to maintain the CSP over time and monitor for any breakages.',
Expand All @@ -65,13 +65,13 @@ const UIStrings = {
*/
unknownKeyword: '{keyword} seems to be an invalid keyword.',
/** Message shown when a CSP uses the deprecated reflected-xss directive. Shown in a table with a list of other CSP vulnerabilities and suggestions. "CSP" stands for "Content Security Policy". "reflected-xss", "CSP2" and "X-XSS-Protection" do not need to be translated. */
deprecatedReflectedXSS: 'reflected-xss is deprecated since CSP2. ' +
deprecatedReflectedXSS: '`reflected-xss` is deprecated since CSP2. ' +
'Please, use the X-XSS-Protection header instead.',
/** Message shown when a CSP uses the deprecated referrer directive. Shown in a table with a list of other CSP vulnerabilities and suggestions. "CSP" stands for "Content Security Policy". "referrer", "CSP2" and "Referrer-Policy" do not need to be translated. */
deprecatedReferrer: 'referrer is deprecated since CSP2. ' +
deprecatedReferrer: '`referrer` is deprecated since CSP2. ' +
'Please, use the Referrer-Policy header instead.',
/** Message shown when a CSP uses the deprecated disown-opener directive. Shown in a table with a list of other CSP vulnerabilities and suggestions. "CSP" stands for "Content Security Policy". "disown-opener", "CSP3" and "Cross-Origin-Opener-Policy" do not need to be translated. */
deprecatedDisownOpener: 'disown-opener is deprecated since CSP3. ' +
deprecatedDisownOpener: '`disown-opener` is deprecated since CSP3. ' +
'Please, use the Cross-Origin-Opener-Policy header instead.',
/**
* @description Message shown when a CSP wildcard allows unsafe scripts to be run in the page. Shown in a table with a list of other CSP vulnerabilities and suggestions. "CSP" stands for "Content Security Policy".
Expand Down
17 changes: 11 additions & 6 deletions core/scripts/pptr-run-devtools.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {fileURLToPath} from 'url';
import * as puppeteer from 'puppeteer-core';
import yargs from 'yargs';
import * as yargsHelpers from 'yargs/helpers';
import {getChromePath} from 'chrome-launcher';
import {launch} from 'chrome-launcher';
import esMain from 'es-main';

import {parseChromeFlags} from '../../cli/run.js';
Expand Down Expand Up @@ -300,12 +300,17 @@ function dismissDialog(dialog) {
* @return {Promise<{lhr: LH.Result, artifacts: LH.Artifacts, logs: string[]}>}
*/
async function testUrlFromDevtools(url, options = {}) {
const {config, chromeFlags} = options;
const {config, chromeFlags = []} = options;

const browser = await puppeteer.launch({
executablePath: getChromePath(),
args: chromeFlags,
devtools: true,
const newChromeFlags = [
...chromeFlags,
'--auto-open-devtools-for-tabs',
];

const chrome = await launch({chromeFlags: newChromeFlags});

const browser = await puppeteer.connect({
browserURL: `http://127.0.0.1:${chrome.port}`,
defaultViewport: null,
});

Expand Down
12 changes: 6 additions & 6 deletions core/test/audits/csp-xss-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,26 +26,26 @@ const STATIC_RESULTS = {
severity: SEVERITY.high,
description: {
formattedDefault:
'Missing object-src allows the injection of plugins that execute unsafe scripts. ' +
'Consider setting object-src to \'none\' if you can.',
'Missing `object-src` allows the injection of plugins that execute unsafe scripts. ' +
'Consider setting `object-src` to `\'none\'` if you can.',
},
directive: 'object-src',
},
noBaseUri: {
severity: SEVERITY.high,
description: {
formattedDefault:
'Missing base-uri allows injected <base> tags to set the base URL for all ' +
'Missing `base-uri` allows injected `<base>` tags to set the base URL for all ' +
'relative URLs (e.g. scripts) to an attacker controlled domain. ' +
'Consider setting base-uri to \'none\' or \'self\'.',
'Consider setting `base-uri` to `\'none\'` or `\'self\'`.',
},
directive: 'base-uri',
},
metaTag: {
severity: SEVERITY.medium,
description: {
formattedDefault:
'The page contains a CSP defined in a <meta> tag. ' +
'The page contains a CSP defined in a `<meta>` tag. ' +
'Consider moving the CSP to an HTTP header or ' +
'defining another strict CSP in an HTTP header.',
},
Expand All @@ -55,7 +55,7 @@ const STATIC_RESULTS = {
severity: SEVERITY.medium,
description: {
formattedDefault:
'Consider adding \'unsafe-inline\' (ignored by browsers supporting ' +
'Consider adding `\'unsafe-inline\'` (ignored by browsers supporting ' +
'nonces/hashes) to be backward compatible with older browsers.',
},
directive: 'script-src',
Expand Down
2 changes: 1 addition & 1 deletion core/test/audits/installable-manifest-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ describe('PWA: webapp install banner audit', () => {
return InstallableManifestAudit.audit(artifacts, context).then(result => {
assert.strictEqual(result.score, 0);
const items = result.details.items;
expect(items[0].reason).toBeDisplayString(/does not contain a 'name'/);
expect(items[0].reason).toBeDisplayString(/does not contain a `name`/);
});
});

Expand Down
Loading

0 comments on commit 294f0a0

Please sign in to comment.