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

fix(firefox): throw error when added script blocked by CSP #1841

Merged
merged 1 commit into from
Apr 17, 2020
Merged

fix(firefox): throw error when added script blocked by CSP #1841

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions src/firefox/ffPage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import { selectors } from '../selectors';
const UTILITY_WORLD_NAME = '__playwright_utility_world__';

export class FFPage implements PageDelegate {
readonly cspErrorsAsynchronousForInlineScipts = true;
readonly rawMouse: RawMouseImpl;
readonly rawKeyboard: RawKeyboardImpl;
readonly _session: FFSession;
Expand Down
11 changes: 9 additions & 2 deletions src/frames.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -566,12 +566,19 @@ export class Frame {
return this._raceWithCSPError(async () => {
if (url !== null)
return (await context.evaluateHandleInternal(addScriptUrl, { url, type })).asElement()!;
let result;
if (path !== null) {
let contents = await util.promisify(fs.readFile)(path, 'utf8');
contents += '//# sourceURL=' + path.replace(/\n/g, '');
return (await context.evaluateHandleInternal(addScriptContent, { content: contents, type })).asElement()!;
result = (await context.evaluateHandleInternal(addScriptContent, { content: contents, type })).asElement()!;
} else {
result = (await context.evaluateHandleInternal(addScriptContent, { content: content!, type })).asElement()!;
}
return (await context.evaluateHandleInternal(addScriptContent, { content: content!, type })).asElement()!;
// Another round trip to the browser to ensure that we receive CSP error messages
// (if any) logged asynchronously in a separate task on the content main thread.
if (this._page._delegate.cspErrorsAsynchronousForInlineScipts)
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably do the same for inline styles and test it.

Copy link
Member Author

Choose a reason for hiding this comment

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

No, styles are different. I thought the same but the styles seem to be handled asynchronously and they support onload/error events. We already have a test for this.

await context.evaluateInternal(() => true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you found the source of this? Are we sure it is a single task?

Copy link
Member Author

Choose a reason for hiding this comment

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

The console logging happens in a separate task on the main thread. My assumption here is that the second evaluate is handled in its own task after the logging, so there is still a room for false positives. I could add some hooks in the native part to report the error synchronously via juggler but I felt like the problem we are fixing doesn't warrant it.

return result;
});

async function addScriptUrl(options: { url: string, type: string }): Promise<HTMLElement> {
Expand Down
2 changes: 2 additions & 0 deletions src/page.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ export interface PageDelegate {

// Work around Chrome's non-associated input and protocol.
inputActionEpilogue(): Promise<void>;
// Work around for asynchronously dispatched CSP errors in Firefox.
readonly cspErrorsAsynchronousForInlineScipts?: boolean;
}

type PageState = {
Expand Down
2 changes: 1 addition & 1 deletion test/page.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,7 @@ describe('Page.addScriptTag', function() {
});

// Firefox fires onload for blocked script before it issues the CSP console error.
it.fail(FFOX)('should throw when added with content to the CSP page', async({page, server}) => {
it('should throw when added with content to the CSP page', async({page, server}) => {
await page.goto(server.PREFIX + '/csp.html');
let error = null;
await page.addScriptTag({ content: 'window.__injected = 35;' }).catch(e => error = e);
Expand Down