Fix request partiness identification when type is main_frame (#1805)

ghostery · Mar 25, 2021 · 0f4e488 · 0f4e488
1 parent 6060e1f
commit 0f4e488
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 6 deletions.
diff --git a/packages/adblocker/src/request.ts b/packages/adblocker/src/request.ts
@@ -189,8 +189,11 @@ function isThirdParty(
   domain: string,
   sourceHostname: string,
   sourceDomain: string,
+  type: RequestType,
 ): boolean {
-  if (domain.length !== 0 && sourceDomain.length !== 0) {
+  if (type === 'main_frame' || type === 'mainFrame') {
+    return false;
+  } else if (domain.length !== 0 && sourceDomain.length !== 0) {
     return domain !== sourceDomain;
   } else if (domain.length !== 0 && sourceHostname.length !== 0) {
     return domain !== sourceHostname;
@@ -331,7 +334,7 @@ export default class Request {
         : getEntityHashesFromLabelsBackward(sourceHostname, sourceDomain);
 
     // Decide on partiness
-    this.isThirdParty = isThirdParty(hostname, domain, sourceHostname, sourceDomain);
+    this.isThirdParty = isThirdParty(hostname, domain, sourceHostname, sourceDomain, type);
     this.isFirstParty = !this.isThirdParty;
 
     // Check protocol

diff --git a/packages/adblocker/test/matching.test.ts b/packages/adblocker/test/matching.test.ts
@@ -306,12 +306,14 @@ describe('#NetworkFilter.match', () => {
     expect(f`||foo$~third-party`).not.to.matchRequest({
       sourceUrl: 'http://baz.bar.com',
       url: 'https://foo.com/bar',
+      type: 'script',
     });
 
     // ~first-party
     expect(f`||foo$~first-party`).to.matchRequest({
       sourceUrl: 'http://baz.bar.com',
       url: 'https://foo.com/bar',
+      type: 'script',
     });
     expect(f`||foo$~first-party`).not.to.matchRequest({
       sourceUrl: 'http://baz.foo.com',
@@ -366,30 +368,37 @@ describe('#NetworkFilter.match', () => {
     expect(f`*$3p,denyallow=x.com|y.com,domain=a.com|b.com`).to.matchRequest({
       sourceUrl: 'https://a.com',
       url: 'https://z.com/bar',
+      type: 'script',
     });
     expect(f`*$3p,denyallow=x.com|y.com,domain=a.com|b.com`).to.matchRequest({
       sourceUrl: 'https://b.com',
       url: 'https://z.com/bar',
+      type: 'script',
     });
     expect(f`*$3p,denyallow=x.com|y.com,domain=a.com|b.com`).to.matchRequest({
       sourceUrl: 'https://sub.b.com',
       url: 'https://z.com/bar',
+      type: 'script',
     });
     expect(f`*$3p,denyallow=x.com|y.com,domain=a.com|b.com`).to.not.matchRequest({
       sourceUrl: 'https://a.com',
       url: 'https://x.com/bar',
+      type: 'script',
     });
     expect(f`*$3p,denyallow=x.com|y.com,domain=a.com|b.com`).to.not.matchRequest({
       sourceUrl: 'https://a.com',
       url: 'https://sub.y.com/bar',
+      type: 'script',
     });
     expect(f`*$3p,denyallow=x.com|y.com,domain=a.com|b.com`).to.not.matchRequest({
       sourceUrl: 'https://sub.b.com',
       url: 'https://sub.y.com/bar',
+      type: 'script',
     });
     expect(f`*$3p,denyallow=x.com|y.com,domain=a.com|b.com`).to.not.matchRequest({
       sourceUrl: 'https://c.com',
       url: 'https://sub.y.com/bar',
+      type: 'script',
     });
   });
 });

diff --git a/packages/adblocker/test/request.test.ts b/packages/adblocker/test/request.test.ts
@@ -233,9 +233,26 @@ describe('#Request', () => {
     });
 
     describe('finds partyness', () => {
-      it('correctly uses domains when available', () => {
+      it('considers as first-party if type is main_frame', () => {
         expect(
-          Request.fromRawDetails({ url: 'https://foo.com', sourceUrl: 'https://foo.com' }),
+          Request.fromRawDetails({
+            sourceUrl: 'https://sub1.sub2.bar.com',
+            url: 'https://foo.com',
+          }),
+        ).to.deep.include({
+          isFirstParty: true,
+          isThirdParty: false,
+        });
+
+      });
+
+      it('correctly uses domains when available if type not main_frame', () => {
+        expect(
+          Request.fromRawDetails({
+            url: 'https://foo.com',
+            sourceUrl: 'https://foo.com',
+            type: 'script',
+          }),
         ).to.deep.include({
           isFirstParty: true,
           isThirdParty: false,
@@ -245,6 +262,7 @@ describe('#Request', () => {
           Request.fromRawDetails({
             sourceUrl: 'https://sub1.sub2.foo.com',
             url: 'https://foo.com',
+            type: 'script',
           }),
         ).to.deep.include({
           isFirstParty: true,
@@ -255,6 +273,7 @@ describe('#Request', () => {
           Request.fromRawDetails({
             sourceUrl: 'https://sub1.sub2.bar.com',
             url: 'https://foo.com',
+            type: 'script',
           }),
         ).to.deep.include({
           isFirstParty: false,
@@ -265,28 +284,31 @@ describe('#Request', () => {
           Request.fromRawDetails({
             sourceUrl: 'https://localhost:4242/',
             url: 'https://foo.com',
+            type: 'script',
           }),
         ).to.deep.include({
           isFirstParty: false,
           isThirdParty: true,
         });
       });
 
-      it('falls-back to first-party if no sourceUrl', () => {
+      it('falls-back to first-party if no sourceUrl and type not main_frame', () => {
         expect(
           Request.fromRawDetails({
             url: 'https://foo.com',
+            type: 'script',
           }),
         ).to.deep.include({
           isFirstParty: true,
           isThirdParty: false,
         });
       });
 
-      it('falls-back to first-party if no url', () => {
+      it('falls-back to first-party if no url and type not main_frame', () => {
         expect(
           Request.fromRawDetails({
             sourceUrl: 'null',
+            type: 'script',
           }),
         ).to.deep.include({
           isFirstParty: true,