Skip to content
This repository was archived by the owner on Nov 6, 2023. It is now read-only.

rm Blockchain.info.xml - #7094

Merged
jeremyn merged 4 commits into
EFForg:masterfrom
gloomy-ghost:gloomy-ghost-rm-blockchain_info
Sep 27, 2016
Merged

jeremyn merged 4 commits into
EFForg:masterfrom
gloomy-ghost:gloomy-ghost-rm-blockchain_info

Conversation

@gloomy-ghost

@gloomy-ghost gloomy-ghost commented Sep 25, 2016

Copy link
Copy Markdown
Collaborator

Preloaded

Chromium, Firefox, Tor

@jeremyn

jeremyn commented Sep 25, 2016

Copy link
Copy Markdown
Contributor

Please update (EDIT: the references in) Blockchainbdgpzk.onion.xml. Looks good otherwise.

@jeremyn jeremyn self-assigned this Sep 25, 2016
@gloomy-ghost

Copy link
Copy Markdown
Collaborator Author

platform="mixedcontent" won't skip the test.

@jeremyn

jeremyn commented Sep 26, 2016

Copy link
Copy Markdown
Contributor

I'm not sure what to do here.

@fuglede @J0WI A .onion ruleset has been changed and is failing Travis. How do we make the test pass?

@J0WI

J0WI commented Sep 26, 2016

Copy link
Copy Markdown
Contributor

I wonder that we even have .onion hosts in our ruleset.
It was discussed in #5085 (comment) that we should not redirect to a hidden service, but it this case it would only redirect a hidden service to https. Maybe @jsha can clarify his statement for this case?

@jsha

jsha commented Sep 26, 2016

Copy link
Copy Markdown
Member

I think a "simple" rule for an onion hostname is fine. I just don't want to become a registry for mapping DNS names to Onion Service names, because we don't have a good way to validate that mapping.

However, it probably makes sense to add a new platform for Tor Browser specifically, and skip such rulesets in the tests, since onion hostnames won't resolve. as a stopgap here I think it would be reasonable to just whitelist the hash of the updated ruleset.

@jeremyn

jeremyn commented Sep 26, 2016

Copy link
Copy Markdown
Contributor

Thanks @J0WI and @jsha .

@gloomy-ghost Following #7094 (comment) , please generate a sha256sum for the modified Blockchainbdgpzk.onion.xml and add it in the appropriate sorted location in utils/ruleset-coverage-whitelist.txt.

@J0WI

J0WI commented Sep 27, 2016

Copy link
Copy Markdown
Contributor

@jsha the changes in would #5085 also patch our test to fetch to use tor, so testing is possible.

@gloomy-ghost

Copy link
Copy Markdown
Collaborator Author

Well, I found some removed rulesets are still in the whitelist:

keybase.io Keybase.io.xml
changetip.com ChangeTip.com.xml
phpmyadmin.net PhpMyAdmin.net.xml
weblate.org Weblate.org.xml

@jeremyn

jeremyn commented Sep 27, 2016

Copy link
Copy Markdown
Contributor

@gloomy-ghost Don't worry about removed rulesets or duplicates in the whitelist. They're harmless, and there's a cleanup script that someone runs every once in a while.

@jeremyn jeremyn closed this Sep 27, 2016
@jeremyn jeremyn reopened this Sep 27, 2016
@gloomy-ghost

Copy link
Copy Markdown
Collaborator Author

However it is still failing…

@J0WI

J0WI commented Sep 27, 2016

Copy link
Copy Markdown
Contributor

Adding it to the whitelist does not help in this case. The whitelist is just for those rules, that have not enough test urls.

We need to patch our tests first before we can accept .onion rules.

@J0WI J0WI mentioned this pull request Sep 27, 2016
@jeremyn

jeremyn commented Sep 27, 2016

Copy link
Copy Markdown
Contributor

I'm getting a sha256 hash of 34c60fd37894f38c9b90375586055c39d11ed8fe2340606587ac2bcce29718c7 for the modified Blockchainbdgpzk.onion.xml, not 6deb04f50f7fdef16b0f2c8732f6c8043413383a8bbda21df5f0ff912b1990cb which is what was added to the whitelist.

@gloomy-ghost

Copy link
Copy Markdown
Collaborator Author
$ sha256sum Blockchainbdgpzk.onion.xml
6deb04f50f7fdef16b0f2c8732f6c8043413383a8bbda21df5f0ff912b1990cb *Blockchainbdgpzk.onion.xml

Quite strange

@jeremyn
jeremyn merged commit 698f691 into EFForg:master Sep 27, 2016
@jeremyn

jeremyn commented Sep 27, 2016

Copy link
Copy Markdown
Contributor

Thanks, merged.

@jeremyn jeremyn removed their assignment Sep 28, 2016
@gloomy-ghost
gloomy-ghost deleted the gloomy-ghost-rm-blockchain_info branch October 16, 2016 09:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants