Skip to content

Commit 6c89af1

Browse files
pdepde
committed
Many improvements
 - avoid duplicate moot rules - much better (and more efficient) categorisation of moot and inactive rules - we still have some alist proliferation bugs....
1 parent 56bc73d commit 6c89af1

4 files changed

Lines changed: 60 additions & 42 deletions

File tree

‎src/chrome/content/code/ApplicableList.js‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ function ApplicableList(logger, home) {
1212
this.moot={}; // rulesets that might be applicable but uris are already https
1313
serial_number += 1;
1414
this.serial = serial_number;
15-
this.log(WARN,"Alist serial #" + this.serial);
15+
this.log(DBUG,"Alist serial #" + this.serial + " for " + this.home);
1616
};
1717

1818
ApplicableList.prototype = {
@@ -39,7 +39,7 @@ ApplicableList.prototype = {
3939
},
4040

4141
populate_menu: function(document, alert) {
42-
this.log(WARN, "populating using alist #" + this.serial);
42+
this.log(DBUG, "populating using alist #" + this.serial);
4343

4444
// get the menu popup
4545
var menupopup = document.getElementById('https-everywhere-context');
@@ -68,19 +68,19 @@ ApplicableList.prototype = {
6868
var command = document.createElement("command");
6969
command.setAttribute('id', rule.id+'-command');
7070
command.setAttribute('label', rule.name);
71-
// oh god, why can't we just say rule.toggle???
7271
command.setAttribute('oncommand', 'toggle_rule("'+rule.id+'")');
7372
commandset.appendChild(command);
7473
}
7574
for(var x in this.active) {
7675
add_command(this.active[x]);
7776
}
78-
for(var x in this.inactive) {
79-
add_command(this.inactive[x]);
80-
}
8177
for(var x in this.moot) {
8278
add_command(this.moot[x]);
8379
}
80+
for(var x in this.inactive) {
81+
add_command(this.inactive[x]);
82+
}
83+
8484

8585
// add a menu item for a rule -- type is "active", "inactive", or "moot"
8686
function add_menuitem(rule, type) {
@@ -115,9 +115,6 @@ ApplicableList.prototype = {
115115
for(var x in this.active) {
116116
add_menuitem(this.active[x], 'active');
117117
}
118-
for(var x in this.moot) {
119-
add_menuitem(this.moot[x], 'moot');
120-
}
121118
for(var x in this.moot) {
122119
if(!(x in this.active) ) {
123120
// rules that are active for some uris are not really moot

‎src/chrome/content/code/HTTPSRules.js‎

Lines changed: 42 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ function RuleSet(name, match_rule, default_off) {
5050

5151
RuleSet.prototype = {
5252
_apply: function(urispec) {
53-
// return 0 would have applied but is inactive, null if it does not apply
53+
// return null if it does not apply
5454
// and the new url if it does apply
5555
var i;
5656
var returl = null;
@@ -60,20 +60,19 @@ RuleSet.prototype = {
6060
return null;
6161
}
6262
// Even so, if we're covered by an exclusion, go home
63-
for(i = 0; i < this.exclusions.length; ++i) {
63+
for (i = 0; i < this.exclusions.length; ++i) {
6464
if (this.exclusions[i].pattern_c.test(urispec)) {
6565
this.log(DBUG,"excluded uri " + urispec);
6666
return null;
6767
}
6868
}
6969
// Okay, now find the first rule that triggers
70-
for(i = 0; i < this.rules.length; ++i) {
70+
for (i = 0; i < this.rules.length; ++i) {
71+
// This is just for displaying inactive rules
7172
returl = urispec.replace(this.rules[i].from_c, this.rules[i].to);
72-
if (returl != urispec) {
73-
if (this.active) return returl
74-
else return 0;
75-
}
73+
if (returl != urispec) return returl;
7674
}
75+
7776
if (this.ruleset_match_c) {
7877
// This is not an error, because we do not insist the matchrule
7978
// precisely describes to target space of URLs ot redirected
@@ -85,13 +84,33 @@ RuleSet.prototype = {
8584
log: function(level, msg) {
8685
https_everywhereLog(level, msg);
8786
},
87+
88+
wouldMatch: function(hypothetical_uri, alist) {
89+
// return true if this ruleset would match the uri, assuming it were http
90+
// used for judging moot / inactive rulesets
91+
this.log(DBUG,"Would " +this.name + " match " +hypothetical_uri.spec +"? serial " + alist.serial);
92+
var uri = hypothetical_uri.clone();
93+
if (uri.scheme == "https") uri.scheme = "http";
94+
var urispec = uri.spec;
95+
96+
if (this.ruleset_match_c && !this.ruleset_match_c.test(urispec))
97+
return false;
98+
99+
for (i = 0; i < this.exclusions.length; ++i)
100+
if (this.exclusions[i].pattern_c.test(urispec)) return false;
101+
102+
for (i = 0; i < this.rules.length; ++i)
103+
if (this.rules[i].from_c.test(urispec)) return true;
104+
this.log(DBUG, "No, we fell through");
105+
return false;
106+
},
88107

89108
transformURI: function(uri) {
90109
// If no rule applies, return null; if a rule would have applied but was
91110
// inactive, return 0; otherwise, return a fresh uri instance
92111
// for the target
93112
var newurl = this._apply(uri.spec);
94-
if (null == newurl)
113+
if (null == newurl)
95114
return null;
96115
if (0 == newurl)
97116
return 0;
@@ -324,37 +343,32 @@ const HTTPSRules = {
324343
}
325344
},
326345

327-
rewrittenURI: function(applicable_list, uri) {
346+
rewrittenURI: function(alist, uri) {
328347
// This function oversees the task of working out if a uri should be
329348
// rewritten, what it should be rewritten to, and recordkeeping of which
330349
// applicable rulesets are and aren't active.
331350
var i = 0;
332351
var newuri = null
333352
var rs = this.potentiallyApplicableRulesets(uri.host);
334-
if (!applicable_list)
353+
if (!alist)
335354
this.log(DBUG, "No applicable list rewriting " + uri.spec);
336355
for(i = 0; i < rs.length; ++i) {
356+
if (!rs[i].active) {
357+
if (alist && rs[i].wouldMatch(uri, alist))
358+
alist.inactive_rule(rs[i]);
359+
continue;
360+
}
361+
if (uri.scheme == "https" && alist) {
362+
// we didn't rewrite but the rule applies to this domain and the
363+
// requests are going over https
364+
if (rs[i].wouldMatch(uri, alist)) alist.moot_rule(rs[i]);
365+
continue;
366+
}
337367
newuri = rs[i].transformURI(uri);
338368
if (newuri) {
339369
// we rewrote the uri
340-
if (applicable_list) {
341-
this.log("Adding active rule: " + rs[i].name);
342-
applicable_list.active_rule(rs[i]);
343-
//applicable_list.show_applicable();
344-
}
370+
if (alist) alist.active_rule(rs[i]);
345371
return newuri;
346-
} else if (0 == newuri) {
347-
// the ruleset is inactive
348-
if (applicable_list) {
349-
this.log("Adding inactive rule: " + rs[i].name);
350-
applicable_list.inactive_rule(rs[i]);
351-
//applicable_list.show_applicable();
352-
}
353-
} else if (uri.scheme == "https" && applicable_list && rs[i].active) {
354-
// we didn't rewrite but the rule applies to this domain and the
355-
// requests are going over https
356-
applicable_list.moot_rule(rs[i]);
357-
//applicable_list.show_applicable();
358372
}
359373
}
360374
return null;
@@ -383,7 +397,7 @@ const HTTPSRules = {
383397
if (this.targets[t])
384398
results = results.concat(this.targets[t]);
385399
}
386-
this.log(DBUG,"Applicable rules for " + host + ":");
400+
this.log(DBUG,"Potentially applicable rules for " + host + ":");
387401
for (i = 0; i < results.length; ++i)
388402
this.log(DBUG, " " + results[i].name);
389403
return results;

‎src/chrome/content/toolbar_button.js‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,11 @@ function https_everywhere_load() {
3838
function show_applicable_list() {
3939
var domWin = content.document.defaultView.top;
4040
if (!(domWin instanceof CI.nsIDOMWindow)) {
41-
alert(domWin + " is not an nsICDOMWindow");
41+
alert(domWin + " is not an nsIDOMWindow");
4242
return null;
4343
}
4444

45-
HTTPSEverywhere = CC["@eff.org/https-everywhere;1"].getService(Components.interfaces.nsISupports).wrappedJSObject;
45+
var HTTPSEverywhere = CC["@eff.org/https-everywhere;1"].getService(Components.interfaces.nsISupports).wrappedJSObject;
4646
var alist = HTTPSEverywhere.getExpando(domWin.document,"applicable_rules", null);
4747

4848
if (alist) {
@@ -60,7 +60,7 @@ function show_applicable_list() {
6060

6161
function toggle_rule(rule_id) {
6262
// toggle the rule state
63-
HTTPSEverywhere = CC["@eff.org/https-everywhere;1"]
63+
var HTTPSEverywhere = CC["@eff.org/https-everywhere;1"]
6464
.getService(Components.interfaces.nsISupports)
6565
.wrappedJSObject;
6666
HTTPSEverywhere.https_rules.rulesetsByID[rule_id].toggle();
@@ -70,7 +70,7 @@ function toggle_rule(rule_id) {
7070
function reload_window(HTTPSEverywhere) {
7171
var domWin = content.document.defaultView.top;
7272
if (!(domWin instanceof CI.nsIDOMWindow)) {
73-
HTTPSEverywhere.log(WARN, domWin + " is not an nsICDOMWindow");
73+
HTTPSEverywhere.log(WARN, domWin + " is not an nsIDOMWindow");
7474
return null;
7575
}
7676
try {
@@ -81,5 +81,7 @@ function reload_window(HTTPSEverywhere) {
8181
HTTPSEverywhere.log(WARN,"failed to get webNav");
8282
return null;
8383
}
84+
// This choice of flags comes from NoScript's quickReload function; not sure
85+
// if it's optimal
8486
webNav.reload(webNav.LOAD_FLAGS_CHARSET_CHANGE);
8587
}

‎src/components/https-everywhere.js‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,12 @@ HTTPSEverywhere.prototype = {
252252

253253
getWindowForChannel: function(channel) {
254254
// Obtain an nsIDOMWindow from a channel
255-
var nc = channel.notificationCallbacks ? channel.notificationCallbacks : channel.loadGroup.notificationCallbacks;
255+
try {
256+
var nc = channel.notificationCallbacks ? channel.notificationCallbacks : channel.loadGroup.notificationCallbacks;
257+
} catch(e) {
258+
this.log(WARN,"no loadgroup notificationCallbacks for "+channel.URI.spec);
259+
return null;
260+
}
256261
if (!nc) {
257262
this.log(WARN, "no window for " + channel.URI.spec);
258263
return null;

0 commit comments

Comments
 (0)