Fix missing 'domcontentloaded' event listener removal from disableBlockingInPage

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<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:
@@ -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);
     }
   }
 }
@@ -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';

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"
     }
   ]
 }

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<puppeteer.Frame> = {
@@ -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 = '<style type="text/css">#Meebo\\:AdElement\\.Root';
+        const blocker = await PuppeteerBlocker.fromPrebuiltFull(fetch);
+
+        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);
+      } finally {
+        await page.close();
+      }
+    } finally {
+      await browser.close();
+    }
+  });
+
+});

yarn.lock

@@ -2515,6 +2515,13 @@ create-require@^1.1.0:
   resolved "https://registry.yarnpkg.com/create-require/-/create-require-1.1.1.tgz#c1d7e8f1e5f6cfc9ff65f9cd352d37348756c333"
   integrity sha512-dcKFX3jn0MpIaXjisoRvexIJVEKzaq7z2rZKxf+MSr9TkdmHmsU4m2lcLojrj/FHl8mk5VxMmYA+ftRkP/3oKQ==
 
+cross-fetch@^3.1.4:
+  version "3.1.4"
+  resolved "https://registry.yarnpkg.com/cross-fetch/-/cross-fetch-3.1.4.tgz#9723f3a3a247bf8b89039f3a380a9244e8fa2f39"
+  integrity sha512-1eAtFWdIubi6T4XPy6ei9iUFoKpUkIF971QLN8lIvvvwueI65+Nw5haMNKUwfJxabqlIIDODJKGrQ66gxC0PbQ==
+  dependencies:
+    node-fetch "2.6.1"
+
 cross-spawn@^7.0.0, cross-spawn@^7.0.3:
   version "7.0.3"
   resolved "https://registry.yarnpkg.com/cross-spawn/-/cross-spawn-7.0.3.tgz#f73a85b9d5d41d045551c177e2882d4ac85728a6"