lib/csp.js - Issue 29772555: Issue 6647 - Stop converting domains from punycode to unicode

Side by Side Diff: lib/csp.js

Issue 29772555: Issue 6647 - Stop converting domains from punycode to unicode (Closed)

Patch Set: Created May 6, 2018, 2:42 p.m.

Left:
Right:

Use n/p to move between diff chunks; N/P to move between comments.

Jump to:

View unified diff | Download patch

OLD	NEW
1 /*	1 /*

2 * This file is part of Adblock Plus <https://adblockplus.org/>,	2 * This file is part of Adblock Plus <https://adblockplus.org/>,

3 * Copyright (C) 2006-present eyeo GmbH	3 * Copyright (C) 2006-present eyeo GmbH

4 *	4 *

5 * Adblock Plus is free software: you can redistribute it and/or modify	5 * Adblock Plus is free software: you can redistribute it and/or modify

6 * it under the terms of the GNU General Public License version 3 as	6 * it under the terms of the GNU General Public License version 3 as

7 * published by the Free Software Foundation.	7 * published by the Free Software Foundation.

8 *	8 *

9 * Adblock Plus is distributed in the hope that it will be useful,	9 * Adblock Plus is distributed in the hope that it will be useful,

10 * but WITHOUT ANY WARRANTY; without even the implied warranty of	10 * but WITHOUT ANY WARRANTY; without even the implied warranty of

11 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the	11 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the

12 * GNU General Public License for more details.	12 * GNU General Public License for more details.

13 *	13 *

14 * You should have received a copy of the GNU General Public License	14 * You should have received a copy of the GNU General Public License

15 * along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>.	15 * along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>.

16 */	16 */

17	17

18 "use strict";	18 "use strict";

19	19

20 const {defaultMatcher} = require("../adblockpluscore/lib/matcher");	20 const {defaultMatcher} = require("../adblockpluscore/lib/matcher");

21 const {RegExpFilter, WhitelistFilter} =	21 const {RegExpFilter, WhitelistFilter} =

22 require("../adblockpluscore/lib/filterClasses");	22 require("../adblockpluscore/lib/filterClasses");

23 const {extractHostFromFrame, getDecodedHostname,	23 const {extractHostFromFrame, isThirdParty} = require("./url");

24 isThirdParty, stringifyURL} = require("./url");

25 const {checkWhitelisted} = require("./whitelisting");	24 const {checkWhitelisted} = require("./whitelisting");

26 const {FilterNotifier} = require("filterNotifier");	25 const {FilterNotifier} = require("filterNotifier");

27 const devtools = require("./devtools");	26 const devtools = require("./devtools");

28	27

29 const {typeMap} = RegExpFilter;	28 const {typeMap} = RegExpFilter;

30	29

31 browser.webRequest.onHeadersReceived.addListener(details =>	30 browser.webRequest.onHeadersReceived.addListener(details =>

32 {	31 {

33 let url = new URL(details.url);	32 let url = new URL(details.url);

34 let urlString = stringifyURL(url);

35 let parentFrame = ext.getFrame(details.tabId, details.parentFrameId);	33 let parentFrame = ext.getFrame(details.tabId, details.parentFrameId);

36 let hostname = extractHostFromFrame(parentFrame) \|\| getDecodedHostname(url);	34 let hostname = extractHostFromFrame(parentFrame) \|\| url.hostname;

37 let thirdParty = isThirdParty(url, hostname);	35 let thirdParty = isThirdParty(url, hostname);

38	36

39 let cspMatch = defaultMatcher.matchesAny(urlString, typeMap.CSP, hostname,	37 let cspMatch = defaultMatcher.matchesAny(details.url, typeMap.CSP, hostname,
	Manish Jethani 2018/05/06 18:37:13 For a URL like https://user:pass@example.com:8080/ For a URL like https://user:pass@example.com:8080/foo?bar=1#hash details.url will also contains the #hash part. I think we should strip it out to be compatible with stringifyURL. Sebastian Noack 2018/05/06 18:47:40 IIRC, the webRequest API is agnostic of any hash g Show quoted text On 2018/05/06 18:37:13, Manish Jethani wrote: > For a URL like https://user:pass@example.com:8080/foo?bar=1#hash details.url > will also contains the #hash part. I think we should strip it out to be > compatible with stringifyURL. IIRC, the webRequest API is agnostic of any hash given in the URL. Please correct me if I'm wrong. But otherwise stripping the hash seems redundant. Manish Jethani 2018/05/07 17:03:45 At least on Chrome 67 (Canary), if you have a docu Show quoted text On 2018/05/06 18:47:40, Sebastian Noack wrote: > On 2018/05/06 18:37:13, Manish Jethani wrote: > > For a URL like https://user:pass@example.com:8080/foo?bar=1#hash details.url > > will also contains the #hash part. I think we should strip it out to be > > compatible with stringifyURL. > > IIRC, the webRequest API is agnostic of any hash given in the URL. Please > correct me if I'm wrong. But otherwise stripping the hash seems redundant. At least on Chrome 67 (Canary), if you have a document like this: <!DOCTYPE html> <iframe src="https://example.com/#foo"></iframe> webRequest.onBeforeRequest will include the "#foo" part in details.url. For what it's worth, I think the hash part is significant for documents. Modern documents (post Ajax) will display different content based on the hash part. They treat it effectively like it's part of the query string, but on the client side. The hash part is also relevant for Flash (SWF) objects (incidentally I have a patent related to this). I'm not sure the hash part is relevant for ad content loaded in embedded frames though, but if you want to keep it I'm OK with it. It just might break some filters, that's all. Sebastian Noack 2018/05/08 06:51:30 That is interesting. Show quoted text On 2018/05/07 17:03:45, Manish Jethani wrote: > On 2018/05/06 18:47:40, Sebastian Noack wrote: > > On 2018/05/06 18:37:13, Manish Jethani wrote: > > > For a URL like https://user:pass@example.com:8080/foo?bar=1#hash details.url > > > will also contains the #hash part. I think we should strip it out to be > > > compatible with stringifyURL. > > > > IIRC, the webRequest API is agnostic of any hash given in the URL. Please > > correct me if I'm wrong. But otherwise stripping the hash seems redundant. > > At least on Chrome 67 (Canary), if you have a document like this: > > <!DOCTYPE html> > <iframe src="https://example.com/#foo"></iframe> > > webRequest.onBeforeRequest will include the "#foo" part in details.url. That is interesting. Show quoted text > For what it's worth, I think the hash part is significant for documents. Modern > documents (post Ajax) will display different content based on the hash part. > They treat it effectively like it's part of the query string, but on the client > side. The hash part is also relevant for Flash (SWF) objects (incidentally I > have a patent related to this). The only thing is if the hash (unlike the query string) changes (for an HTML document), we are not going to see a new request in the webRequest API. Still I am uncertain if its worth to bother. Show quoted text > It just might break some filters, that's all. I don't think that it will break much (if anything). Let's say we have https://example.com/#foo, so all of following filters will still match as they did before: \|\|example.com \|\|example.com^ \|\|example.com/ \|\|example.com/^ Manish Jethani 2018/05/08 12:23:31 Acknowledged. Show quoted text On 2018/05/08 06:51:30, Sebastian Noack wrote: > On 2018/05/07 17:03:45, Manish Jethani wrote: > > On 2018/05/06 18:47:40, Sebastian Noack wrote: > > > On 2018/05/06 18:37:13, Manish Jethani wrote: > > > > For a URL like https://user:pass@example.com:8080/foo?bar=1#hash > details.url > > > > will also contains the #hash part. I think we should strip it out to be > > > > compatible with stringifyURL. > > > > > > IIRC, the webRequest API is agnostic of any hash given in the URL. Please > > > correct me if I'm wrong. But otherwise stripping the hash seems redundant. > > > > At least on Chrome 67 (Canary), if you have a document like this: > > > > <!DOCTYPE html> > > <iframe src="https://example.com/#foo"></iframe> > > > > webRequest.onBeforeRequest will include the "#foo" part in details.url. > > That is interesting. > > > For what it's worth, I think the hash part is significant for documents. > Modern > > documents (post Ajax) will display different content based on the hash part. > > They treat it effectively like it's part of the query string, but on the > client > > side. The hash part is also relevant for Flash (SWF) objects (incidentally I > > have a patent related to this). > > The only thing is if the hash (unlike the query string) changes (for an > HTML document), we are not going to see a new request in the webRequest API. > Still I am uncertain if its worth to bother. > > > It just might break some filters, that's all. > > I don't think that it will break much (if anything). Let's say we have > https://example.com/#foo, so all of following filters will still match > as they did before: > > \|\|example.com > \|\|example.com^ > \|\|example.com/ > \|\|example.com/^ Acknowledged.
40 thirdParty, null, false);	38 thirdParty, null, false);

41 if (cspMatch)	39 if (cspMatch)

42 {	40 {

43 let page = new ext.Page({id: details.tabId, url: details.url});	41 let page = new ext.Page({id: details.tabId, url: details.url});

44 let frame = ext.getFrame(details.tabId, details.frameId);	42 let frame = ext.getFrame(details.tabId, details.frameId);

45	43

46 if (checkWhitelisted(page, frame))	44 if (checkWhitelisted(page, frame))

47 return;	45 return;

48	46

49 // To avoid an extra matchesAny for the common case we assumed no	47 // To avoid an extra matchesAny for the common case we assumed no

50 // $genericblock filters applied when searching for a matching $csp filter.	48 // $genericblock filters applied when searching for a matching $csp filter.

51 // We must now pay the price by first checking for a $genericblock filter	49 // We must now pay the price by first checking for a $genericblock filter

52 // and if necessary that our $csp filter is specific.	50 // and if necessary that our $csp filter is specific.

53 let specificOnly = !!checkWhitelisted(page, frame, null,	51 let specificOnly = !!checkWhitelisted(page, frame, null,

54 typeMap.GENERICBLOCK);	52 typeMap.GENERICBLOCK);

55 if (specificOnly)	53 if (specificOnly)

56 {	54 {

57 cspMatch = defaultMatcher.matchesAny(urlString, typeMap.CSP, hostname,	55 cspMatch = defaultMatcher.matchesAny(details.url, typeMap.CSP, hostname,

58 thirdParty, null, specificOnly);	56 thirdParty, null, specificOnly);

59 if (!cspMatch)	57 if (!cspMatch)

60 return;	58 return;

61 }	59 }

62	60

63 devtools.logRequest([details.tabId], urlString, "CSP", hostname,	61 devtools.logRequest([details.tabId], details.url, "CSP", hostname,

64 thirdParty, null, specificOnly, cspMatch);	62 thirdParty, null, specificOnly, cspMatch);

65 FilterNotifier.emit("filter.hitCount", cspMatch, 0, 0, [details.tabId]);	63 FilterNotifier.emit("filter.hitCount", cspMatch, 0, 0, [details.tabId]);

66	64

67 if (cspMatch instanceof WhitelistFilter)	65 if (cspMatch instanceof WhitelistFilter)

68 return;	66 return;

69	67

70 details.responseHeaders.push({	68 details.responseHeaders.push({

71 name: "Content-Security-Policy",	69 name: "Content-Security-Policy",

72 value: cspMatch.csp	70 value: cspMatch.csp

73 });	71 });

74	72

75 return {responseHeaders: details.responseHeaders};	73 return {responseHeaders: details.responseHeaders};

76 }	74 }

77 }, {	75 }, {

78 urls: ["http:///", "https:///"],	76 urls: ["http:///", "https:///"],

79 types: ["main_frame", "sub_frame"]	77 types: ["main_frame", "sub_frame"]

80 }, ["blocking", "responseHeaders"]);	78 }, ["blocking", "responseHeaders"]);

OLD	NEW

« no previous file with comments | « .eslintignore ('k') | lib/filterComposer.js » ('j') | lib/options.js » ('J')