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

Conversation

@J0WI
Copy link
Contributor

@J0WI J0WI commented Jan 1, 2016

No description provided.

@fuglede
Copy link
Contributor

fuglede commented Jan 1, 2016

ERROR src/chrome/content/rules/Bernina.xml: Not enough tests (0 vs 1) for <Rule from '^http://bernina-zürich\.ch/' to 'https://www.bernina-zürich.ch/'>

Looks like a charset related bug in https-everywhere-checker.

@J0WI
Copy link
Contributor Author

J0WI commented Apr 6, 2016

Hmm, whitlelist didn't work. Is there another way?

@julianladisch
Copy link
Contributor

punycode: replace bernina-zürich.ch by xn--bernina-zrich-4ob.ch

@J0WI
Copy link
Contributor Author

J0WI commented Jul 28, 2016

This may work for our tests, but HTTPS Everywhere didn't trigger on bernina-zürich.ch then.

@J0WI
Copy link
Contributor Author

J0WI commented Feb 21, 2017

rebased

@jeremyn
Copy link
Contributor

jeremyn commented Jun 17, 2017

Here's what I think the ruleset should look like. I'm describing the final state, not just changes from the most recent commit 144140d, so for example I ask you to add a target for ^bernina.com even though 144140d already has that target.

Mark as Invalid certificate:

Add target:

Mark as Secure connection failed:

Mark as No working URL known:

Also:

  • Delete https://adfs.bernina.com , I guess it's an internal Active Directory subdomain we don't need to worry about See added Bernina.xml #3726 (comment)
  • I don't know what refused: www.youngfashion is telling me. Marking as Secure connection failed: www.youngfashion.bernina.com is fine.
  • Use full domain names in the top comment, which is easier to read and particularly important with multiple domains

@jeremyn jeremyn closed this Jun 17, 2017
@jeremyn jeremyn reopened this Jun 17, 2017
@jeremyn jeremyn self-assigned this Jun 17, 2017
@J0WI
Copy link
Contributor Author

J0WI commented Jun 18, 2017

done

@jeremyn
Copy link
Contributor

jeremyn commented Jun 18, 2017

I've updated the checklist through 9c4115f.

I see why you added the exclusion for www.bernina.com, but we don't need it (or the test) for ^ because ^ 301 redirects to www.

Should we have a redirect from (www\.)?bernina-zuerich.ch to (www\.)?bernina-zürich.ch?

Your adfs test https://adfs.bernina.com/adfs/portal/logo/logo.png gets a 503 for HTTP. Maybe we should note this? I'm still not convinced we should include this URL at all, but we can if you really want to.

@J0WI
Copy link
Contributor Author

J0WI commented Jun 18, 2017

Test urls are required, see https://travis-ci.org/EFForg/https-everywhere/jobs/244233730#L217
bernina-zuerich.ch and bernina-zürich.ch don't have the same content.

@jeremyn
Copy link
Contributor

jeremyn commented Jun 18, 2017

Oh okay, I see Travis failed on ^ so I agree we need the exclusion and test for both ^ and www. I wonder why it failed for ^ though, I don't see multiple redirects for ^ in the browser.

http://bernina-zuerich.ch is strange. It loads a full page borderless frame for http://www.bernina-zuerich.ch, which 301 redirects to http://www.bernina-zürich.ch , which 301 redirects to https://www.bernina-zürich.ch . In my Firefox configuration the resulting page is entirely blank due to Load denied by X-Frame-Options: https://www.bernina-zürich.ch/ does not permit cross-origin framing. I think this is logically a redirect even if it's implemented in this weird way, so a rewrite rule would be okay. What do you think?

I'm okay with listing adfs.bernina.com under Different content but could you please add https://adfs.bernina.com/adfs/portal/logo/logo.png to the comment as an example? The root URLs http://adfs.bernina.com and https://adfs.bernina.com have the same content.

@J0WI
Copy link
Contributor Author

J0WI commented Jun 18, 2017

Okay, I added a redirect for bernina-zuerich.ch

@jeremyn jeremyn merged commit 6ebb650 into EFForg:master Jun 18, 2017
@jeremyn
Copy link
Contributor

jeremyn commented Jun 18, 2017

Thanks, merged.

@jeremyn jeremyn removed their assignment Jun 18, 2017
@J0WI J0WI deleted the bernina branch June 18, 2017 22:05
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