Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding return after rejects in a few places. Implementing Ghostery side of GH-885. Fix for GH-871. #10

Merged
merged 10 commits into from Mar 23, 2018

Update JSDocs and add missing block reason.

  • Loading branch information
jsignanini committed Mar 23, 2018
commit 6ee32ae249a5acc239a6f65560b8199f5effb487
@@ -23,7 +23,7 @@ import conf from './Conf';
import foundBugs from './FoundBugs';
import globals from './Globals';
import latency from './Latency';
import Policy, { BLOCK_REASON_SS_UNBLOCKED } from './Policy';
import Policy, { BLOCK_REASON_SS_UNBLOCKED, BLOCK_REASON_C2P_ALLOWED_THROUGH } from './Policy';
import PolicySmartBlock from './PolicySmartBlock';
import PurpleBox from './PurpleBox';
import surrogatedb from './SurrogateDb';
@@ -754,6 +754,12 @@ class EventHandlers {
return true;
}

/**
* @typedef {Object} BlockWithReason
* @property {boolean} block indicates if the tracker should be blocked.
* @property {string} reason indicates the reason for the block result.
*/

/**
* Determine whether this request should be blocked
*
@@ -765,7 +771,7 @@ class EventHandlers {
* @param {string} tab_host tab url host
* @param {string} page_url full tab url
* @param {number} request_id request id
* @return {Object} {block, ss_unblock}
* @return {BlockWithReason} block result with reason
*/
_checkBlocking(app_id, cat_id, tab_id, tab_host, page_url, request_id) {

This comment has been minimized.

@christophertino

christophertino Mar 23, 2018
Member

this should have an object return, correct? looks to just be returning block at the moment, instead of return {block:true, ss_unblock:false} etc

See line 793

This comment has been minimized.

@zarembsky

zarembsky Mar 23, 2018
Author Contributor

block IS object, as it is returned by shouldBlock, which returns object.

const fromRedirect = globals.REDIRECT_MAP.has(request_id);
@@ -774,7 +780,7 @@ class EventHandlers {
// If we let page-level c2p trackers through, we don't want to block it
// along with all subsequent top-level redirects.
if (fromRedirect && globals.LET_REDIRECTS_THROUGH) {
block = { block: false };
block = { block: false, reason: BLOCK_REASON_C2P_ALLOWED_THROUGH };
} else {
block = this.policy.shouldBlock(app_id, cat_id, tab_id, tab_host, page_url);
}
ProTip! Use n and p to navigate between commits in a pull request.