Skip to content

SSL/TLSize all the things! (convert http:// to https:// where appropriate) #3687

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

Closed
wants to merge 1 commit into from

Conversation

reedloden
Copy link
Contributor

Update links to use https:// where it is supported. There's probably a lot more that could be fixed, but these are the core ones I found (especially the download links in order to prevent MITM attacks).

@zpao
Copy link
Member

zpao commented Apr 17, 2015

The gh-pages branch is autogenerated. Please make changes to files in https://github.com/facebook/react/tree/master/docs, which is what we use to generate the html.

GitHub will force you to make a new PR since you're targeting a difference branch, so closing this out.

@zpao zpao closed this Apr 17, 2015
@zpao
Copy link
Member

zpao commented Apr 17, 2015

(also, hi Reed!)

@jimfb
Copy link
Contributor

jimfb commented Apr 17, 2015

Also, @reedloden : There have been some discussions (and some fixes) of this stuff before, and while I haven't looked into all of these specifically (there are a lot of them) there were some reasons that we haven't switched everything over to https yet.

For instance, the download links point to a CDN, and while the FBME url is httpsable, it immediately redirects to a HTTP CDN, so making the FBME url https doesn't actually protect anyone (and might give people a false sense of security).

Also, for the embedded jsfiddles, switching to https will sometimes break the rendering of results, so we need to be careful with those (I don't remember the details, but I'm sure there are some PR/issues about it in the history).

Anyway, we'll need to look at the ramifications of switching individual URLs, it's not as easy as doing a global search-and-replace and expecting/hoping that everything will work. It's probably worth taking a look at that history so you can pre-empt some of the issues you're likely to hit when doing this migration.

@reedloden
Copy link
Contributor Author

@zpao -- thanks, will do.

@JSFB -- it was very much a targeted replacement. I didn't just s/http:/https:/ everywhere. Did it only for specific domains where I tested that https:// works correctly. :)

Even in the case of things like https://fb.me redirecting to http://, that's still not a reason why not to use https://, especially as when the redirect eventually/hopefully gets fixed, things will Just Work.

@reedloden
Copy link
Contributor Author

@JSFB actually, where are you seeing HTTP CDN?

$ curl -I https://fb.me/react-0.13.1.js
HTTP/1.1 301 Moved Permanently
Location: https://fbcdn-dragon-a.akamaihd.net/hphotos-ak-xpf1/t39.3284-6/10956897_1450117505279709_432772944_n.js

@jimfb
Copy link
Contributor

jimfb commented Apr 17, 2015

@reedloden For example you changed https://fb.me/react-with-addons-0.9.0-rc1.js

But yeah, anywhere we are do support HTTPS CDN, we should absolutely update those links to be HTTPS. I tend to agree that even if it redirects to an HTTP CDN, maybe we should update the urls in anticipation of that switch. I'm just saying that there has been some discussion about this in the past and there is a valid argument to be made that it could be confusing/misleading because https is redirecting to http. Just wanted to bring such discussions to your attention.

@zpao
Copy link
Member

zpao commented Apr 17, 2015

0.13.0 and under didn't use https on the redirect

~% curl -I https://fb.me/react-0.13.0.js
HTTP/1.1 301 Moved Permanently
Location: http://dragon.ak.fbcdn.net/hphotos-ak-xpa1/t39.3284-6/10734320_1571131556471061_497798403_n.js

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants