Fix missing 'domcontentloaded' event listener removal from `disableBl…

…ockingInPage` (#1903)
ghostery · May 4, 2021 · e1a676a · e1a676a
1 parent ee577f7
commit e1a676a
Show file tree

Hide file tree

Showing 4 changed files with 465 additions and 27 deletions.
diff --git 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<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);
     }
   }
 }

diff --git a/packages/adblocker-puppeteer/package.json b/packages/adblocker-puppeteer/package.json
@@ -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": [
     {
@@ -87,6 +89,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
@@ -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> = {
@@ -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, () => {
+      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.')
+    });
+  });
+});