Create site hacks network delegate helper #68

bbondy · 2018-03-26T14:12:45Z

Implements some of the needed site-hacks from browser-laptop.
Implements these site hacks as a new network delegate helper.

This partially implements the following issue: brave/brave-browser#84.

An alternate strategy for this task using an extension was considered and partially implemented in this branch: site-hacks-extension, but we aren't sure if we'll have support for extensions in Android yet, and it seemed easy enough to do these via C++.

In this PR:

Empty data URL redirect for sp1.nypost.com
Empty data URL redirect for sp.nasdaq.com
Google tag services stub for www.googletagservices.com/tag/js/gpt.js
Google tag manager stub for http://www.googletagmanager.com/gtm.js
Block https://www.lesechos.fr/xtcore.js
Block https://*.y8.com/js/sdkloader/outstream.js

In a follow up PRs:

Cookie exceptions
Referrer exceptions
Local storage exceptions
Forbes custom cookie
Twitter NoScript redirect
Brave UA whitelist

bbondy · 2018-03-26T15:46:39Z

I'm still working on a test for the above, you can review before but I'll do the test before merging.

bbondy · 2018-03-26T16:49:47Z

Test pushed, ready for review.

This re-uses the static network redirect helper class for both the profile network delegate and the system network delegate.

Allows a list of hosts to be redirected to empty data URLs. The list is initially: - sp1.nypost.com - sp.nasdaq.com

Matches requests for Google tag manager and Google tag services, and uses our polyfill instead.

Creates a different network delegate helper so the system one is not re-used. This is for 2 reasons: 1) To avoid accidentally leaking things like geolocation redirects to the profile network context. 2) To not need to slow down the system network context with other site hacks meant for the profile network context.

Allows the SiteHacks delegate to block things. These should eventually be moved to the ad-block lists, but for now I'm just porting things 1:1 from browser-laptop.

garrettr

As we discussed on Slack, this needed a rebase to pick up some of the recent rename-related changes. With the rebase, I am able to run yarn test brave_unit_tests successfully.

The code looks good and the new tests look great. LGTM, I mentioned some concerns/nits in line comments but they do not need to block merge.

garrettr · 2018-03-27T21:16:21Z

browser/net/brave_site_hacks_network_delegate_helper.cc

+  str.append(": noopthisfn, updateCorrelator: noopfn }; var SizeMappingBuilder = function() { }; p = SizeMappingBuilder.prototype; p.addSize = noopthisfn; p.build = noopnullfn; var Slot = function() { }; p = Slot.prototype; p.addService = noopthisfn; p.clearCategoryExclusions = noopthisfn; p.clearTargeting = noopthisfn; p.defineSizeMapping = noopthisfn; p.get = noopnullfn; p.getAdUnitPath = nooparrayfn; p.getAttributeKeys = nooparrayfn; p.getCategoryExclusions = nooparrayfn; p.getDomId = noopstrfn; p.getSlot");
+  str.append("ElementId = noopstrfn; p.getSlotId = noopthisfn; p.getTargeting = nooparrayfn; p.getTargetingKeys = nooparrayfn; p.set = noopthisfn; p.setCategoryExclusion = noopthisfn; p.setClickUrl = noopthisfn; p.setCollapseEmptyDiv = noopthisfn; p.setTargeting = noopthisfn; var gpt = window.googletag || {}; window.googletag.destroySlots = function () { }; var cmd = gpt.cmd || []; gpt.apiReady = true; gpt.cmd = []; gpt.cmd.push = function(a) { try { a(); } catch (ex) { } return 1; }; gpt.companionAds = func");
+  str.append("tion() { return companionAdsService; }; gpt.content = function() { return contentService; }; gpt.defineOutOfPageSlot = function() { return new Slot(); }; gpt.defineSlot = function() { return new Slot(); }; gpt.disablePublisherConsole = noopfn; gpt.display = noopfn; gpt.enableServices = noopfn; gpt.getVersion = noopstrfn; gpt.pubads = function() { return pubAdsService; }; gpt.pubadsReady = true; gpt.sizeMapping = function() { return new SizeMappingBuilder(); }; window.googletag = gpt; while ( cm");
+  str.append("d.length !== 0 ) { gpt.cmd.push(cmd.shift()); } })();");


It's too bad C++ doesn't have something akin to Rust's include_str! (as far as I know). Perhaps GN has something that can help us out with including large strings like this. I think all these str.appends are fine for now, but may be difficult to maintain over time.

How about getting this script from file in resource pak?

That's a great idea. I won't block on it but I"ll do it in this new issue I just posted:
brave/brave-browser#128

garrettr · 2018-03-27T21:16:44Z

browser/net/brave_site_hacks_network_delegate_helper.cc

+  str.append("d.length !== 0 ) { gpt.cmd.push(cmd.shift()); } })();");
+  base64_output.reserve(4668);
+  Base64UrlEncode(str, base::Base64UrlEncodePolicy::OMIT_PADDING, &base64_output);
+  base64_output= std::string(kJSDataURLPrefix) + base64_output;


Nit: space before =

will fix before landing, thx.

bbondy · 2018-03-27T21:39:32Z

Thanks for the review.

As we discussed on Slack, this needed a rebase to pick up some of the recent rename-related changes. With the rebase, I am able to run yarn test brave_unit_tests successfully.

The old branch was also runnable on its own, but I rebased for ease of testing.
To get it to work before you'd have to npm run init and then build.

As long as it's buildable in general though on its own with init and build, rebases are not required because otherwise it'll take up everyone's time constantly rebasing with scale.

bbondy · 2018-03-27T21:50:05Z

We probably have some more work to do on the tooling, because I think the sync that you did didn't reset things properly. Maybe because using the different repo name in DEPS, it probably doesn't have proper support for that.

simonhong · 2018-03-28T00:10:03Z

browser/net/brave_site_hacks_network_delegate_helper.h

+#ifndef BRAVE_BROWSER_NET_BRAVE_SITE_HACKS_NETWORK_DELEGATE_H_
+#define BRAVE_BROWSER_NET_BRAVE_SITE_HACKS_NETWORK_DELEGATE_H_
+
+#include "chrome/browser/net/chrome_network_delegate.h"


nit: this header can be removed

simonhong · 2018-03-28T00:14:58Z

browser/net/brave_site_hacks_network_delegate_helper.cc

+#include "extensions/common/url_pattern.h"
+#include "net/url_request/url_request.h"
+
+


nit: two blank lines
Or did you add one more line intentionally?

We need to introduce .cc linting soon. Ya I did it intentionally but I see now most chromium files do not do that. I'll update before merging this.

Add scope to methods which modify content settings

Add upload command

bbondy self-assigned this Mar 26, 2018

bbondy force-pushed the site-hacks branch from 935a08f to 179d3e4 Compare March 26, 2018 14:25

bbondy requested a review from garrettr March 26, 2018 14:32

bbondy added the pr/work-in-progress label Mar 26, 2018

bbondy removed the pr/work-in-progress label Mar 26, 2018

bbondy requested a review from yrliou March 27, 2018 01:22

bbondy mentioned this pull request Mar 27, 2018

Add missing build target for brave_unit_tests #73

Closed

9 tasks

bbondy added 5 commits March 27, 2018 16:56

Add profile static redirect checks

3927227

This re-uses the static network redirect helper class for both the profile network delegate and the system network delegate.

Add empty data URL redirect site hacks

b72847d

Allows a list of hosts to be redirected to empty data URLs. The list is initially: - sp1.nypost.com - sp.nasdaq.com

Add pollyfill sitehacks for google tag manager and google tag service

0141219

Matches requests for Google tag manager and Google tag services, and uses our polyfill instead.

Add ability to block resources in site-hacks

c09114f

Allows the SiteHacks delegate to block things. These should eventually be moved to the ad-block lists, but for now I'm just porting things 1:1 from browser-laptop.

bbondy force-pushed the site-hacks branch from 793ab8b to ae7cb82 Compare March 27, 2018 20:56

garrettr previously approved these changes Mar 27, 2018

View reviewed changes

simonhong reviewed Mar 28, 2018

View reviewed changes

Add site hacks unit tests

dcb69d0

bbondy dismissed garrettr’s stale review via dcb69d0 March 28, 2018 01:19

bbondy force-pushed the site-hacks branch from ae7cb82 to dcb69d0 Compare March 28, 2018 01:19

bbondy merged commit fd1f430 into master Mar 28, 2018

bsclifton deleted the site-hacks branch June 18, 2018 06:29

bbondy pushed a commit that referenced this pull request Feb 18, 2019

Merge pull request #68 from brave/content-settings-add-scope

0a62825

Add scope to methods which modify content settings

fmarier pushed a commit that referenced this pull request Oct 29, 2019

Merge pull request #68 from brave/create-dist-upload

e91abd6

Add upload command

petemill pushed a commit that referenced this pull request Jul 27, 2020

Merge pull request #68 from brave/create-dist-upload

d196aea

Add upload command

petemill pushed a commit that referenced this pull request Jul 28, 2020

Merge pull request #68 from brave/create-dist-upload

34de6b5

Add upload command

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create site hacks network delegate helper #68

Create site hacks network delegate helper #68

bbondy commented Mar 26, 2018 •

edited

Loading

bbondy commented Mar 26, 2018

bbondy commented Mar 26, 2018

garrettr left a comment

garrettr Mar 27, 2018

simonhong Mar 27, 2018

bbondy Mar 28, 2018

garrettr Mar 27, 2018

bbondy Mar 27, 2018

bbondy commented Mar 27, 2018

bbondy commented Mar 27, 2018

simonhong Mar 28, 2018

simonhong Mar 28, 2018

bbondy Mar 28, 2018

		#include "extensions/common/url_pattern.h"
		#include "net/url_request/url_request.h"

Create site hacks network delegate helper #68

Create site hacks network delegate helper #68

Conversation

bbondy commented Mar 26, 2018 • edited Loading

bbondy commented Mar 26, 2018

bbondy commented Mar 26, 2018

garrettr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bbondy commented Mar 27, 2018

bbondy commented Mar 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bbondy commented Mar 26, 2018 •

edited

Loading