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

HTTPS sensitive URIs #2965

Merged
merged 1 commit into from
Feb 4, 2016
Merged

HTTPS sensitive URIs #2965

merged 1 commit into from
Feb 4, 2016

Conversation

kepta
Copy link
Collaborator

@kepta kepta commented Feb 3, 2016

change of all http:// URIs in js/id folder to //.

(closes #2960)

@@ -117,7 +117,7 @@ iD.BackgroundSource.Bing = function(data, dispatch) {
};

bing.logo = 'bing_maps.png';
bing.terms_url = 'http://opengeodata.org/microsoft-imagery-details';
bing.terms_url = '//opengeodata.org/microsoft-imagery-details';
Copy link
Member

Choose a reason for hiding this comment

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

@bhousel
Copy link
Member

bhousel commented Feb 3, 2016

see also discussion on #2681

For outbound links and api endpoints, we need to make sure the resource is available over both http and https. I commented in the diff the places where that's not true.

Given that iD can sometimes be loaded locally, and file:// urls can break in surprising ways, maybe we should instead add the location.protocol === 'http' ? 'http' : 'https' check to iD.detect()?

Thoughts @jfirebaugh ?

@kepta
Copy link
Collaborator Author

kepta commented Feb 3, 2016

This // will load the hyperlink depending on the iD's current connection status
if plain then // will go to http://
if encypted then // will go to https://

So I think we wont need to do
location.protocol === 'http' ? 'http' : 'https'

@jfirebaugh
Copy link
Member

Please don't use protocol relative URLs -- as @bhousel said they fail for file://, and https is preferred even when loading the referring page on http. Therefore:

  • If the site supports https, use https unconditionally.
  • If not, look for an alternative URL.
  • Failing that, bug the site owner for https support.
  • Failing that, stick to http.

@kepta kepta force-pushed the 2960 branch 2 times, most recently from 9ea00ff to 69262b0 Compare February 3, 2016 19:48
@kepta
Copy link
Collaborator Author

kepta commented Feb 3, 2016

@bhousel I have updated the patch.
Please have a look.

@bhousel
Copy link
Member

bhousel commented Feb 4, 2016

Thanks this looks good!

bhousel added a commit that referenced this pull request Feb 4, 2016
Support HTTPS URIs where available
@bhousel bhousel merged commit 5bf9028 into openstreetmap:master Feb 4, 2016
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.

Access Nominatim via HTTPS for location search
3 participants