Merge pull request #110 from cowlicks/gh-89

Improve isRequestThirdParty using details.initiator
cowlicks · Sep 2, 2018 · 535afd6 · 535afd6
2 parents 1d66ebf + 5671e68
commit 535afd6
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 15 deletions.
diff --git a/src/js/tabs.js b/src/js/tabs.js
@@ -218,6 +218,14 @@ class Tabs {
     }
   }
 
+  getTabHostname(tabId) {
+    try {
+      return this.getFrame(tabId, 0).urlObj.hostname
+    } catch(e) {
+      return undefined;
+    }
+  }
+
   getTabUrl(tabId) {
     try {
       return this.getFrameUrl(tabId, 0);
@@ -238,26 +246,23 @@ class Tabs {
     return this.getTab(tabId).get(frameId);
   }
 
-  isRequestThirdParty(details) {
-    let {tabId, initiator, urlObj: {hostname}} = details;
+  isRequestThirdParty({tabId, initiator, urlObj: {hostname} = {}}) {
+    if (typeof initiator !== 'undefined') {
+      return isThirdParty((new URL(initiator)).hostname, hostname);
+    }
     if (tabId === -1) {
-      if (typeof initiator !== 'undefined') {
-        let initiatorHostname = (new URL(initiator)).hostname;
-        return isThirdParty(initiatorHostname, hostname);
-      }
-      return false; // no associated tab, so 3rd party isn't applicable
+      return false; // no associated tab and no initiator info so we don't know
     }
     return this.isThirdParty(tabId, hostname);
   }
 
   isThirdParty(tabId, hostname) {
-    try {
-      let tabhost = this.getFrame(tabId, 0).urlObj.hostname
-      return isThirdParty(tabhost, hostname);
-    } catch (e) {
-      log(`error getting tab data for tabId ${tabId} with error ${e.stack}`);
+    let tabHost = this.getTabHostname(tabId);
+    if (!tabHost) {
+      log(`could not calculate tabhost for isThirdParty with hostname ${hostname} on tabId ${tabId}`);
       return false;
     }
+    return isThirdParty(tabHost, hostname);
   }
 
   hasResource({tabId, frameId, url, type}) {

diff --git a/src/js/test/tabs_test.js b/src/js/test/tabs_test.js
@@ -8,7 +8,8 @@ let assert = require('chai').assert,
   {Tab, Tabs} = require('../tabs');
 
 const tabId = 1,
-  main_frame = {frameId: 0, url: 'https://google.com/', tabId, parentFrameId: -1, type: 'main_frame'},
+  firstParty = 'https://google.com/', thirdParty = 'https://third.com/',
+  main_frame = {frameId: 0, url: firstParty, tabId, parentFrameId: -1, type: 'main_frame'},
   sub_frame = {frameId: 1, url: 'about:blank', tabId, parentFrameId: 0, type: 'sub_frame'};
 
 describe('tabs.js', function() {
@@ -20,8 +21,6 @@ describe('tabs.js', function() {
       this.tab = this.tabs.getTab(main_frame.tabId);
     });
     describe('#getCurrentData', function() {
-      beforeEach(function() {
-      });
       it('does not get frames from "discarded" tabs', async function() {
         let discarded = true, id = 2, url = 'https://url.com/';
         tabsQuery.tabs = [{id, discarded, url}];
@@ -34,6 +33,26 @@ describe('tabs.js', function() {
         assert.isUndefined(this.tabs.getFrame(id, 1));
       });
     });
+    describe('#isRequestThirdParty', function() {
+      it('no initiator', function() {
+        let details = {tabId: 1, urlObj: new URL(thirdParty)};
+        assert.isTrue(this.tabs.isRequestThirdParty(details));
+      });
+      it('has initiator', function() {
+        let isFirst = {initiator: firstParty, urlObj: new URL(firstParty)},
+          isThird = {initiator: firstParty, urlObj: new URL(thirdParty)};
+        assert.isFalse(this.tabs.isRequestThirdParty(isFirst));
+        assert.isTrue(this.tabs.isRequestThirdParty(isThird));
+      });
+      it('no initiator tabId = -1', function() {
+        assert.isFalse(this.tabs.isRequestThirdParty({tabId: -1}));
+      });
+      it('no initiator, no prexisting data about tab', function() {
+        this.tabs = new Tabs();
+        let details = {tabId: 1, urlObj: new URL(thirdParty)};
+        assert.isFalse(this.tabs.isRequestThirdParty(details));
+      });
+    });
 
     it('#getTabUrl', function() {
       assert.equal(this.tabs.getTabUrl(1), 'https://google.com/');