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

Loop protection in HTTPS upgrades #59

Merged
merged 8 commits into from
Sep 29, 2021
Merged

Loop protection in HTTPS upgrades #59

merged 8 commits into from
Sep 29, 2021

Conversation

kdzwinel
Copy link
Member

Domain is HTTPS upgradable but page will constantly try to downgrade to HTTP. Client should detect that and allow page to load. Unfortunately our extension seems to have a bug and doesn't do it.

https://app.asana.com/0/1186013049913869/1201057222436407/f

@@ -174,7 +174,7 @@ function downloadTheResults () {
const a = document.createElement('a');
const url = window.URL.createObjectURL(new Blob([data], { type: 'application/json' }));
a.href = url;
a.download = 'fingerprinting-results.json';
a.download = 'https-upgrades-results.json';
Copy link
Member Author

Choose a reason for hiding this comment

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

unrelated change - renaming downloaded file to match test name

</head>
<body>
<script>
const MAX = 15;
Copy link
Member Author

Choose a reason for hiding this comment

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

if loop protection doesn't kick in after 15 attempts - give up

const tests = [
{
id: 'upgrade-navigation',
run: () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

most of this file, besides the contents of this test, is just a template. We should probably extract it to some reusable helpers instead of copying it over and over

Copy link
Collaborator

@jonathanKingston jonathanKingston left a comment

Choose a reason for hiding this comment

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

This is really great, thanks for changing the wording to make the test clearer :). Also thanks for the PR comments yourself, I think this helps often.

@kdzwinel kdzwinel merged commit c7c1e54 into main Sep 29, 2021
@kdzwinel kdzwinel deleted the konrad/https-loop branch September 29, 2021 13:05
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.

2 participants