[HTTPS-Everywhere] Release 2.2 for Firefox seems to ignore default_off

[Christopher Liu]

Mr. Eckersley (et al),

I have encountered another problem with 2.2.1 - my preferences for
ruleset enable/disable are being reset at every startup, with the
accompanying message logged in the Error Console ("Detected a
buggy-looking configuration..."), even though all 10 rulesets listed
in the relevant code are disabled as they should be.
By "relevant code" I mean the HTTPSRules.js change in:
https://gitweb.torproject.org/https-everywhere.git/commitdiff/6aea0dec3f97c60fc5d7d3b3e1f9369a81be08ed

Could you clarify what the "tolerate their absence" comment was
intended to mean?
Specifically, I noticed that the code doesn't increment nonBuggy for
rulesets that are absent. This essentially means that an absent
ruleset will be counted as buggy. If eight or more of the offending
rulesets are removed in the future, that would cause everybody's
preferences to be reset. I don't think that was intended.

My guess is that some race condition at startup was causing all(?) of
the offending rulesets to be mistakenly detected as absent. I regret
that I am unable to troubleshoot further at the moment, as I have
installed 3.0development.6 to work around the problem. However, I will
try to see if this occurs in the Tor Browser Bundle.

Regardless whether we manage to diagnose the actual problem, we should
probably define a new configuration entry to record whether this
codepath has been run. This is meant to ensure that the check will
only run once for a given browser installation.

As usual, thank you very much for your help.
C. Liu

P.S. It sounds like someone's Wikipedia ruleset got disabled, and they
had trouble reenabling it. The description isn't very clear though,
and I can't reproduce their problem. Might this be related?
https://en.wikipedia.org/w/index.php?title=Wikipedia:Village_pump_(technical)&oldid=508029101#Firefox_login_bug

On Fri, Aug 17, 2012 at 1:40 PM, Peter Eckersley <pde at eff.org> wrote:
> Yes, it looks like the problem was a cherry-pick that I made of your original
> commit into the 2.0 branch.  Although git automerged everything, the
> RuleSet() constructor had changed between 2.0 and master.
>
> If only we could write our browser extensions in a high-level, statically
> typed language.  Sigh :)
>
> I'm going to implement a cleanup routine so that users who installed 2.2 as
> their first version of HTTPS Everywhere will have a "Reset to Defaults"
> executed for them when they upgrade.
>
> On Fri, Aug 17, 2012 at 12:58:54PM -0700, Seth David Schoen wrote:
>> Peter Eckersley writes:
>>
>> > The problem is the call to the Ruleset constructor in that patch.  It has an
>> > extra parameter, xmlruleset.getAttribute("f"), which isn't expected.
>> >
>> > Seth, can you look to see whether you intended to do something else with that
>> > parameter, or whether we should just pull it out?
>>
>> Hi Peter,
>>
>> The commit you linked to seems very different to me from the one that I originally
>> made in June.  My commit was fc91c08f4a4260a123d3563fd0bd9765a7498d3d and it
>> replaced code that previously _did_ invoke this constructor this way, as follows:
>>
>> -    if (xmlruleset. at match_rule.length() > 0) match_rl = xmlruleset. at match_rule;
>> -    if (xmlruleset. at default_off.length() > 0) dflt_off = xmlruleset. at default_off;
>> -    if (xmlruleset. at platform.length() > 0) platform = xmlruleset. at platform;
>> -    var rs = new RuleSet(xmlruleset. at name, xmlruleset. at f, match_rl, dflt_off, platform);
>> +    this.log(DBUG, "Parsing " + xmlruleset.getAttribute("name") + " from " + file.path);
>> +
>> +    var match_rl = xmlruleset.getAttribute("match_rule");
>> +    var dflt_off = xmlruleset.getAttribute("default_off");
>> +    var platform = xmlruleset.getAttribute("platform");
>> +    var rs = new RuleSet(xmlruleset.getAttribute("name"), xmlruleset.getAttribute("f"), match_rl, dflt_off, platform);
>>
>> I don't know why the version in a1d02f16794ed4d593863e5921b4b05c93badf27 is so
>> different.
>>
>> The reason for my patch using xmlruleset.getAttribute("f") is not because I was
>> introducing it into the codebase, but because the version I was patching used
>> xmlruleset. at f in the same context. :-)
>>
>> --
>> Seth Schoen  <schoen at eff.org>
>> Senior Staff Technologist                       https://www.eff.org/
>> Electronic Frontier Foundation                  https://www.eff.org/join
>> 454 Shotwell Street, San Francisco, CA  94110   +1 415 436 9333 x107
>
> --
> Peter Eckersley                            pde at eff.org
> Technology Projects Director      Tel  +1 415 436 9333 x131
> Electronic Frontier Foundation    Fax  +1 415 436 9993