Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable Wayback Machine for ".onion" addresses #5306

Merged
merged 1 commit into from
Apr 22, 2020
Merged

Disable Wayback Machine for ".onion" addresses #5306

merged 1 commit into from
Apr 22, 2020

Conversation

DanielKim1
Copy link
Contributor

@DanielKim1 DanielKim1 commented Apr 21, 2020

Resolves brave/brave-browser#9342

Submitter Checklist:

Test Plan:

  1. Open a new private window with Tor (Use the hamburger menu on the top right corner of the Brave window, or press option + command + N)
  2. Go to the 404 page for the New York Times (http://www.nytimes3xbfgragh.onion/404)
  3. Check that the "Sorry, that page is missing" banner does not appear
    NYT onion URL source: https://open.nytimes.com/https-open-nytimes-com-the-new-york-times-as-a-tor-onion-service-e0d0b67b7482

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

@simonhong
Copy link
Member

@DanielKim1 Welcome and thanks for contributing to Brave project :)
I don't know much about Tor. I'll pass review request to Tor/Security export (@fmarier @darkdh ).

@DanielKim1
Copy link
Contributor Author

Thanks Simon! I'm a big fan of this project :)

Copy link
Member

@darkdh darkdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, also request review from @riastradh-brave

@diracdeltas
Copy link
Member

@riastradh-brave is not actively working on tor anymore, but this lgtm

@simonhong
Copy link
Member

Hmm, how we make this PR run in CI builder?

@simonhong
Copy link
Member

@DanielKim1 #5312 is created to run this PR from CI.
We can merge this if CI is happy. :) Let's wait.

@simonhong
Copy link
Member

simonhong commented Apr 22, 2020

CI only failed with unrelated flaky test. So you can merge this @DanielKim1 🎉
I'm looking forward to see your contribution again 👍

@simonhong simonhong added this to the 1.10.x - Nightly milestone Apr 22, 2020
@DanielKim1
Copy link
Contributor Author

DanielKim1 commented Apr 22, 2020

Thanks Simon!
Unfortunately, it looks like I can't merge this PR:

Only those with write access to this repository can merge pull requests.

Could someone else merge this PR in, please? 🙏

@darkdh darkdh merged commit dfb3743 into brave:master Apr 22, 2020
@DanielKim1 DanielKim1 deleted the disable-wayback-machine-for-onion-addresses branch April 22, 2020 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disable wayback machine for ".onion" addresses
4 participants