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 missing 'domcontentloaded' event listener removal from disableBlockingInPage #1903

Merged
merged 2 commits into from May 4, 2021
Merged

Fix missing 'domcontentloaded' event listener removal from disableBlockingInPage #1903

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1d55175
adblocker-puppeteer: fix incomplete blocking disabling
May 3, 2021
c9d54f6
adblocker-puppeteer: make tests not request external resources
May 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 11 additions & 10 deletions packages/adblocker-puppeteer/adblocker.ts
View file
Open in desktop
Expand Up @@ -55,23 +55,23 @@ export function fromPuppeteerDetails(details: puppeteer.HTTPRequest): Request {
*/
export class BlockingContext {
private readonly onFrameNavigated: (frame: puppeteer.Frame) => Promise<void>;
private readonly onDomContentLoaded: () => Promise<void>;
private readonly onRequest: (details: puppeteer.HTTPRequest) => void;

constructor(private readonly page: puppeteer.Page, private readonly blocker: PuppeteerBlocker) {
this.onFrameNavigated = (frame) => blocker.onFrameNavigated(frame);
this.onDomContentLoaded = () => blocker.onFrameNavigated(this.page.mainFrame());
this.onRequest = (request) => blocker.onRequest(request);
}

public async enable(): Promise<void> {
if (this.blocker.config.loadCosmeticFilters === true) {
// Register callback to cosmetics injection (CSS + scriptlets)
if (this.blocker.config.loadCosmeticFilters) {
// Register callbacks to cosmetics injection (CSS + scriptlets)
this.page.on('frameattached', this.onFrameNavigated);
this.page.on('domcontentloaded', () => {
this.onFrameNavigated(this.page.mainFrame());
});
this.page.on('domcontentloaded', this.onDomContentLoaded);
}

if (this.blocker.config.loadNetworkFilters === true) {
if (this.blocker.config.loadNetworkFilters) {
// Make sure request interception is enabled for `page` before proceeding
await this.page.setRequestInterception(true);
// NOTES:
Expand All @@ -87,13 +87,14 @@ export class BlockingContext {
}

public async disable(): Promise<void> {
if (this.blocker.config.loadNetworkFilters === true) {
this.page.removeListener('request', this.onRequest);
if (this.blocker.config.loadNetworkFilters) {
this.page.off('request', this.onRequest);
await this.page.setRequestInterception(false);
}

if (this.blocker.config.loadCosmeticFilters === true) {
this.page.removeListener('frameattached', this.onFrameNavigated);
if (this.blocker.config.loadCosmeticFilters) {
this.page.off('frameattached', this.onFrameNavigated);
this.page.off('domcontentloaded', this.onDomContentLoaded);
}
}
}
Expand Down
8 changes: 7 additions & 1 deletion packages/adblocker-puppeteer/package.json
View file
Open in desktop
Expand Up @@ -53,7 +53,9 @@
"tslint": "^6.0.0",
"tslint-config-prettier": "^1.18.0",
"tslint-no-unused-expression-chai": "^0.1.4",
"typescript": "^4.1.2"
"typescript": "^4.1.2",
"express": "^4.17.1",
"@types/express": "^4.17.11"
},
"contributors": [
{
Expand Down Expand Up @@ -87,6 +89,10 @@
{
"name": "Anton Lazarev",
"email": "antonok35@gmail.com"
},
{
"name": "Konstantin Mosunov",
"email": "mosunov.konstantin@huawei.com"
}
]
}
55 changes: 54 additions & 1 deletion packages/adblocker-puppeteer/test/adblocker.test.ts
View file
Open in desktop
Expand Up @@ -2,8 +2,11 @@ import { expect } from 'chai';
import 'mocha';

import * as puppeteer from 'puppeteer';
import * as express from 'express';
import { Server } from 'http';

import { fromPuppeteerDetails, getHostnameHashesFromLabelsBackward } from '../adblocker';
import { fromPuppeteerDetails, getHostnameHashesFromLabelsBackward, PuppeteerBlocker } from '../adblocker';
import { AddressInfo } from 'net';

describe('#fromPuppeteerDetails', () => {
const baseFrame: Partial<puppeteer.Frame> = {
Expand Down Expand Up @@ -36,3 +39,53 @@ describe('#fromPuppeteerDetails', () => {
});
});
});

describe('#stylesInjection', () => {
let server: Server;
let port: number;
let browser: puppeteer.Browser;
let page: puppeteer.Page;

before(async () => {
const app = express();
app.get('/', (_req: any, res: any) => {
res.send('<html><title>Empty test HTML</title></html>');
});
server = app.listen(0, () => {
remusao marked this conversation as resolved.
Show resolved Hide resolved
const addressInfo = server.address() as AddressInfo;
port = addressInfo.port;
console.log('Test server listening on port', port);
});
browser = await puppeteer.launch();
console.log('Puppeteer browser launched.');
page = await browser.newPage();
console.log('Puppeteer page opened.');
});

it('does not inject styles into original content', async () => {
const url = `http://localhost:${port}`;
const stylesInjectionPrefix = '<style';
const blocker = await PuppeteerBlocker.parse('###Meebo\\:AdElement\\.Root');

await blocker.enableBlockingInPage(page);
await page.goto(url, {waitUntil: 'networkidle2'});
const contentWithoutAds = await page.content();

await blocker.disableBlockingInPage(page);
await page.goto(url, {waitUntil: 'networkidle2'});
const content = await page.content();

expect(contentWithoutAds).to.contain(stylesInjectionPrefix);
expect(content).not.to.contain(stylesInjectionPrefix);
});

after(async () => {
await page.close();
console.log('Puppeteer page closed.');
await browser.close();
console.log('Puppeteer browser closed.');
server.close(() => {
console.log('Test server closed.')
});
});
});