-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Disabling rulesets that fail the comprehensive fetch test across mult… #10538
Conversation
…iple locations (Singapore, London, San Francisco) and across the span of a week
@Bisaloo and @cschanaj have been patiently going through existing |
😦 please revert your changes until they can respond. |
It's fine to overwrite the changes here if there are more granular fixes to be made. I've announced that this was happening numerous times. We've done this in years past, as well. |
Five days ago @cschanaj wrote in #9906 (comment) that:
I'm not sure where you announced this would happen but it would have been helpful for you to coordinate your efforts in issues These volunteers have done a tremendous amount of work over months working on these |
This PR doubled the number of As for #10516 like changes, now I do it semi-automatically. A script is used to get a list of candidates, then check ranking and perform a secondary fetch manually. It can be speed up by fully automating the process, but I am not sure that I have the time to do the code recently. Besides, I think you should whitelist (at least for @Hainish |
@cschanaj This used to be around 10%. This means that at least 20% of rulesets were enabled by default when they were causing problems with at least one of their targets. Although obviously the majority of the changes here are moving from enabled to disabled, there are some rulesets that have been re-enabled after long having been disabled. Most of these only have one or a few hosts. If we delete rulesets rather than disabling them, this 1) makes it so that users can't manually re-enable these rulesets, and 2) makes it so that future runs of the
It seems important that the fetch test not whitelist rulesets that are in the alexa top 1k, for the very fact that HTTPS Everywhere may well be causing problems for these sites. Your example is one with many hosts and just one failure. But what happens when we have one of the top 1k sites with only a few hosts, and it is causing a failure? We won't necessarily see it unless someone reports it. Better, in this case, to disable that rule completely until someone can take a look at it.
Fixed this issue and tests seem to be passing now. |
@Hainish thanks for fixing Travis! |
I was expecting a specific issue before that happens but okay. For the record and for the next time you run this :
|
@Bisaloo I will attempt to modify the script to implement (1). For (2), great idea as well. |
@Hainish I ran the following command and noted that around 900+ rulesets failed the In particular, the messages for user@host:~/https-everywhere/rules$ for file in *; do TMP=$(grep 'Disabled by https-everywhere-checker because' $file | wc -l); if [ $TMP -gt 1 ]; then echo $file; fi; done > ~/failed_more_than_once.txt Note: At the moment, there is no existing ruleset failed the I agree with @Bisaloo's idea, but I prefer |
Also, as @jeremyn pointed it out, it would have been really useful for us to know when this was going to happen. It's true you mentioned it in multiple issues but I had no idea it would be coming so soon. If you could open an issue to announce it next time, as you do for the releases, I would really appreciate it. |
I agree with @Bisaloo, It could be better if it could be announced, say 2-3 weeks (even 1-2 months) before this is happening, since a quite number of my PRs get merge conflict now. It is sort of annoying to resolve them all. |
#10545 might worth some attentions. |
This is because currently there is no graceful way to remove comments from previous runs of the disabling @Bisaloo In the future I'd like to have a this happen continuously, say every week. We've gotten lots of reports to the effect of "HTTPS Everywhere breaks my site" and these issues sometimes linger for months. That's only the ones that are actually reported. If we're able to have a good system in place which keeps the ruleset coverage accurate on a continual basis, those problems wouldn't arise as much. Plus, every diff would be much much smaller - note that the last time this was run was two years ago. This will result in very few PRs resulting in merge conflicts. We have to account for temporary and regional outages, as well. My ideal schedule would be:
and only disable those rulesets (or targets, if we change the script) that are problematic for each one of them. This is pretty much what we did here, but not on such a strict schedule. Resolving the merge conflicts should be easy. The more granular changes that you've been working on in pull requests should always overwrite the |
This sounds good! It's also worth noticing that we sometimes get issues from users saying that HTTPS-Everywhere is breaking a website and that the webmasters obviously know it because there is a warning on the page 'Please disable HTTPS-Everywhere to use this site'. But no one bothered reporting it... So I am totally with you on the fact that we can't trust reports to detect broken websites. |
What's the deal with |
@Bisaloo https://github.com/efforg/https-everywhere/blob/bb84ab16a4d95134c72aae487d9ce7fb1410ddd0/test/rules/src/https_everywhere_checker/check_rules.py#L144 is the relevant line here. If the Looks like the response codes have changed since I ran the test, though. http://www.gitlab.com/ returns a |
https://github.com/efforg/https-everywhere/blob/bb84ab16a4d95134c72aae487d9ce7fb1410ddd0/test/rules/src/https_everywhere_checker/http_client.py#L379 seems to be the culprit. 308 redirects are not followed. This is a relatively new HTTP code, standardized in 2015: https://tools.ietf.org/html/rfc7538 This has been fixed in c284a7c and I've restarted the fetch test. |
@Bisaloo I think you need to rebase for this to take effect. |
@Hainish I think that |
Just FYI, the tests led to false positives because they ran on an outdated version of the rules. See for example #10560 (comment). I am not sure what can be done about it, excepted merging the changes as soon as the tests finished running. |
Is it possible to trace which HEAD |
@Bisaloo the rules that have been disabled were failing the fetch test across all three locations, but the comments are derived from the San Francisco test. Looking now, I can see that When I just re-enabled and tested user@https-everywhere ~/workspace/https-everywhere (master) $ sudo docker run --rm -ti -v $(pwd):/opt -e RULESETS_CHANGED="src/chrome/content/rules/leboncoin.fr.xml" --privileged httpse bash -c "service miredo start && test/fetch.sh"
* Starting Teredo IPv6 tunneling daemon miredo [ OK ]
INFO Finished comparing http://leboncoin.fr/ -> https://leboncoin.fr/. Rulefile: src/chrome/content/rules/leboncoin.fr.xml.
INFO Finished comparing http://beta.leboncoin.fr/ -> https://beta.leboncoin.fr/. Rulefile: src/chrome/content/rules/leboncoin.fr.xml.
INFO Finished comparing http://compteperso.leboncoin.fr/ -> https://compteperso.leboncoin.fr/. Rulefile: src/chrome/content/rules/leboncoin.fr.xml.
INFO Finished comparing http://comptepro.leboncoin.fr/ -> https://comptepro.leboncoin.fr/. Rulefile: src/chrome/content/rules/leboncoin.fr.xml.
INFO Finished comparing http://corporate.leboncoin.fr/ -> https://corporate.leboncoin.fr/. Rulefile: src/chrome/content/rules/leboncoin.fr.xml.
INFO Finished comparing http://deliv.leboncoin.fr/ -> https://deliv.leboncoin.fr/. Rulefile: src/chrome/content/rules/leboncoin.fr.xml.
INFO Finished comparing http://ftp.leboncoin.fr/ -> https://ftp.leboncoin.fr/. Rulefile: src/chrome/content/rules/leboncoin.fr.xml.
INFO Finished comparing http://img0.leboncoin.fr/ -> https://img0.leboncoin.fr/. Rulefile: src/chrome/content/rules/leboncoin.fr.xml.
INFO Finished comparing http://img1.leboncoin.fr/ -> https://img1.leboncoin.fr/. Rulefile: src/chrome/content/rules/leboncoin.fr.xml.
INFO Finished comparing http://img2.leboncoin.fr/ -> https://img2.leboncoin.fr/. Rulefile: src/chrome/content/rules/leboncoin.fr.xml.
INFO Finished comparing http://img3.leboncoin.fr/ -> https://img3.leboncoin.fr/. Rulefile: src/chrome/content/rules/leboncoin.fr.xml.
INFO Finished comparing http://img4.leboncoin.fr/ -> https://img4.leboncoin.fr/. Rulefile: src/chrome/content/rules/leboncoin.fr.xml.
INFO Finished comparing http://img5.leboncoin.fr/ -> https://img5.leboncoin.fr/. Rulefile: src/chrome/content/rules/leboncoin.fr.xml.
INFO Finished comparing http://img6.leboncoin.fr/ -> https://img6.leboncoin.fr/. Rulefile: src/chrome/content/rules/leboncoin.fr.xml.
INFO Finished comparing http://img7.leboncoin.fr/ -> https://img7.leboncoin.fr/. Rulefile: src/chrome/content/rules/leboncoin.fr.xml.
INFO Finished comparing http://lesincroyablesrencontres.leboncoin.fr/ -> https://lesincroyablesrencontres.leboncoin.fr/. Rulefile: src/chrome/content/rules/leboncoin.fr.xml.
INFO Finished comparing http://mobile.leboncoin.fr/ -> https://mobile.leboncoin.fr/. Rulefile: src/chrome/content/rules/leboncoin.fr.xml.
INFO Finished comparing http://ns1.leboncoin.fr/ -> https://ns1.leboncoin.fr/. Rulefile: src/chrome/content/rules/leboncoin.fr.xml.
INFO Finished comparing http://rss.leboncoin.fr/ -> https://rss.leboncoin.fr/. Rulefile: src/chrome/content/rules/leboncoin.fr.xml.
INFO Finished comparing http://sondage.leboncoin.fr/ -> https://sondage.leboncoin.fr/. Rulefile: src/chrome/content/rules/leboncoin.fr.xml.
INFO Finished comparing http://static.leboncoin.fr/ -> https://static.leboncoin.fr/. Rulefile: src/chrome/content/rules/leboncoin.fr.xml.
INFO Finished comparing http://support.leboncoin.fr/ -> https://support.leboncoin.fr/. Rulefile: src/chrome/content/rules/leboncoin.fr.xml.
INFO Finished comparing http://www.leboncoin.fr/ -> https://www.leboncoin.fr/. Rulefile: src/chrome/content/rules/leboncoin.fr.xml.
INFO Finished comparing http://www2.leboncoin.fr/ -> https://www2.leboncoin.fr/. Rulefile: src/chrome/content/rules/leboncoin.fr.xml.
INFO Finished comparing http://crm.leboncoin.fr/ -> https://crm.leboncoin.fr/. Rulefile: src/chrome/content/rules/leboncoin.fr.xml.
ERROR src/chrome/content/rules/leboncoin.fr.xml: Fetch error: http://deliv.leboncoin.fr/ => https://deliv.leboncoin.fr/: (60, 'SSL certificate problem: unable to get local issuer certificate')
ERROR src/chrome/content/rules/leboncoin.fr.xml: Fetch error: http://ns1.leboncoin.fr/ => https://ns1.leboncoin.fr/: (28, 'Connection timed out after 20001 milliseconds')
ERROR src/chrome/content/rules/leboncoin.fr.xml: Fetch error: http://rss.leboncoin.fr/ => https://rss.leboncoin.fr/: (28, 'Operation timed out after 30001 milliseconds with 0 bytes received')
ERROR src/chrome/content/rules/leboncoin.fr.xml: Fetch error: http://support.leboncoin.fr/ => https://support.leboncoin.fr/: (60, 'SSL certificate problem: unable to get local issuer certificate')
ERROR src/chrome/content/rules/leboncoin.fr.xml: Fetch error: http://crm.leboncoin.fr/ => https://crm.leboncoin.fr/: (60, 'SSL certificate problem: unable to get local issuer certificate')
INFO Finished in 150.74 seconds. Loaded rulesets: 1, URL pairs: 25.
Test URL test failed. |
In London: <!--
Disabled by https-everywhere-checker because:
Fetch error: http://deliv.leboncoin.fr/ => https://deliv.leboncoin.fr/: (60, 'SSL certificate problem: unable to get local issuer certificate')
Fetch error: http://ns1.leboncoin.fr/ => https://ns1.leboncoin.fr/: (28, 'Connection timed out after 20000 milliseconds')
Fetch error: http://rss.leboncoin.fr/ => https://rss.leboncoin.fr/: (28, 'Operation timed out after 30001 milliseconds with 0 bytes received')
Fetch error: http://support.leboncoin.fr/ => https://support.leboncoin.fr/: (60, 'SSL certificate problem: unable to get local issuer certificate')
Fetch error: http://crm.leboncoin.fr/ => https://crm.leboncoin.fr/: (60, 'SSL certificate problem: unable to get local issuer certificate')
--> In Singapore: <!--
Disabled by https-everywhere-checker because:
Fetch error: http://deliv.leboncoin.fr/ => https://deliv.leboncoin.fr/: (60, 'SSL certificate problem: unable to get local issuer certificate')
Fetch error: http://ns1.leboncoin.fr/ => https://ns1.leboncoin.fr/: (28, 'Connection timed out after 20003 milliseconds')
Fetch error: http://rss.leboncoin.fr/ => https://rss.leboncoin.fr/: (28, 'Operation timed out after 30001 milliseconds with 0 bytes received')
Fetch error: http://support.leboncoin.fr/ => https://support.leboncoin.fr/: (60, 'SSL certificate problem: unable to get local issuer certificate')
Fetch error: http://crm.leboncoin.fr/ => https://crm.leboncoin.fr/: (60, 'SSL certificate problem: unable to get local issuer certificate')
--> |
…iple locations (Singapore, London, San Francisco) and across the span of a week