From 1d5517579c5dd2c0cc9be5d16373897e2f856f45 Mon Sep 17 00:00:00 2001 From: Konstantin Mosunov Date: Mon, 3 May 2021 17:41:09 +0300 Subject: [PATCH 1/2] adblocker-puppeteer: fix incomplete blocking disabling --- packages/adblocker-puppeteer/adblocker.ts | 37 ++++++++++--------- packages/adblocker-puppeteer/package.json | 7 +++- .../test/adblocker.test.ts | 34 ++++++++++++++++- yarn.lock | 7 ++++ 4 files changed, 65 insertions(+), 20 deletions(-) diff --git a/packages/adblocker-puppeteer/adblocker.ts b/packages/adblocker-puppeteer/adblocker.ts index 06ae4fed04..4fdb243b33 100644 --- a/packages/adblocker-puppeteer/adblocker.ts +++ b/packages/adblocker-puppeteer/adblocker.ts @@ -55,23 +55,23 @@ export function fromPuppeteerDetails(details: puppeteer.HTTPRequest): Request { */ export class BlockingContext { private readonly onFrameNavigated: (frame: puppeteer.Frame) => Promise; + private readonly onDomContentLoaded: () => Promise; 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 { - 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: @@ -87,13 +87,14 @@ export class BlockingContext { } public async disable(): Promise { - 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); } } } @@ -185,7 +186,7 @@ export class PuppeteerBlocker extends FiltersEngine { getRulesFromDOM: false, }); - if (active === false) { + if (!active) { return; } @@ -197,7 +198,7 @@ export class PuppeteerBlocker extends FiltersEngine { }); } - // Seconde step is to start monitoring the DOM of the page in order to + // Second step is to start monitoring the DOM of the page in order to // inject more specific selectors based on `id`, `class`, or `href` found on // nodes. We first query all of them, then monitor the DOM for a few // seconds (or until one of the stopping conditions is met, see below). @@ -223,7 +224,7 @@ export class PuppeteerBlocker extends FiltersEngine { }); // Abort if cosmetics are disabled - if (active === false) { + if (!active) { return; } @@ -261,14 +262,14 @@ export class PuppeteerBlocker extends FiltersEngine { break; } - if (foundNewFeatures === false) { + if (!foundNewFeatures) { break; } } catch (ex) { break; } - if (this.config.enableMutationObserver === false) { + if (!this.config.enableMutationObserver) { break; } @@ -278,7 +279,7 @@ export class PuppeteerBlocker extends FiltersEngine { public onRequest = (details: puppeteer.HTTPRequest): void => { const request = fromPuppeteerDetails(details); - if (this.config.guessRequestTypeFromUrl === true && request.type === 'other') { + if (this.config.guessRequestTypeFromUrl && request.type === 'other') { request.guessTypeOfRequest(); } @@ -310,7 +311,7 @@ export class PuppeteerBlocker extends FiltersEngine { contentType: redirect.contentType, }); } - } else if (match === true) { + } else if (match) { details.abort('blockedbyclient'); } else { details.continue(); @@ -386,5 +387,5 @@ export class PuppeteerBlocker extends FiltersEngine { } } -// Re-export symboles from @cliqz/adblocker for convenience +// Re-export symbols from @cliqz/adblocker for convenience export * from '@cliqz/adblocker'; diff --git a/packages/adblocker-puppeteer/package.json b/packages/adblocker-puppeteer/package.json index b49ac5db25..8bf19587b2 100644 --- a/packages/adblocker-puppeteer/package.json +++ b/packages/adblocker-puppeteer/package.json @@ -53,7 +53,8 @@ "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", + "cross-fetch": "^3.1.4" }, "contributors": [ { @@ -87,6 +88,10 @@ { "name": "Anton Lazarev", "email": "antonok35@gmail.com" + }, + { + "name": "Konstantin Mosunov", + "email": "mosunov.konstantin@huawei.com" } ] } diff --git a/packages/adblocker-puppeteer/test/adblocker.test.ts b/packages/adblocker-puppeteer/test/adblocker.test.ts index 10613741fb..59ae80abaa 100644 --- a/packages/adblocker-puppeteer/test/adblocker.test.ts +++ b/packages/adblocker-puppeteer/test/adblocker.test.ts @@ -2,8 +2,9 @@ import { expect } from 'chai'; import 'mocha'; import * as puppeteer from 'puppeteer'; +import fetch from 'cross-fetch'; -import { fromPuppeteerDetails, getHostnameHashesFromLabelsBackward } from '../adblocker'; +import {fromPuppeteerDetails, getHostnameHashesFromLabelsBackward, PuppeteerBlocker} from '../adblocker'; describe('#fromPuppeteerDetails', () => { const baseFrame: Partial = { @@ -36,3 +37,34 @@ describe('#fromPuppeteerDetails', () => { }); }); }); + +describe('#stylesInjection', () => { + + it('does not inject styles into original content', async () => { + const browser = await puppeteer.launch(); + try { + const page = await browser.newPage(); + try { + const url = 'https://example.com'; + const stylesInjectionPrefix = '