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

[Christopher Liu]

Mr. Eckersley (et al),

Just to keep everyone updated on this newest issue...
I did manage to reproduce it in my (stable) Tor Browser Bundle.

The only additional extension I installed in my TBB is Live HTTP
Headers. It is also present in my normal Firefox installation, but
neither of the other extensions provided with TBB are (Torbutton,
NoScript).
I have one user ruleset in TBB and quite a few in normal Firefox.

Thus, Live HTTP Headers is the only extension suspected of causing a
conflict. It's also possible that this could be triggered by the
presence of any user rulesets. I still need to test for these
possibilities.

(To what value should the configuration entry
extensions.https_everywhere.LogLevel be set in order to see INFO
messages?)

C. Liu
P.S. To clarify about the Wikipedia discussion: The original poster
worked around his/her issue by downloading the Wikimedia ruleset from
the git repository and installing it as a user ruleset. Their issue
probably isn't related to mine, but the timing was awfully
suspicious...

On Sat, Aug 18, 2012 at 1:44 PM, Christopher Liu <cmliu00151 at gmail.com> wrote:
> 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