Posted by drupal-develope... on September 25, 2009 at 11:25am
| Project: | Google chart API |
| Version: | 7.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | critical |
| Assigned: | boombatower |
| Status: | closed (fixed) |
| Issue tags: | needs backport to D6 |
Issue Summary
When use securepages module - the path for chart images stays with http:// instead of https:// which causes SSL error. How chart images can be prefixed correctly respecting secure connection?
Comments
#1
why would you ever need https with this? 1. its not your domain, 2. its a auto-generated chart. not EVERYTHING needs to be https, that will just be slow. images for example should not be severed over https (most cases)
#2
It doesn't seem https:// on the front works with Google Chart API anyway.
#3
Actually it seems that they do, but not with the old url currently be used.
<?phpdefine('CHART_URI', 'http://chart.apis.google.com/chart');
?>
Would need to be:
<?phpdefine('CHART_URI', 'http://chart.googleapis.com/chart');
?>
In 7.x branch the url() function is used so it should go through standard alter hooks to change to https if you want...not sure what the best solution is here...although changing the base url seems like a good idea.
#4
Committed that change to 6.x-1.x and 7.x-1.x branches.
#5
It's pretty standard for 3rd-party integration modules to detect and match the protocol when embedding external resources. For example, google_analytics does this. Without this support, browsers display scary warnings (some more severe than others):
#6
D6 patch - this is the same condition, but the $is_https global isn't available in D6.
#7
+1 - I can confirm that these browser security warnings are very, very scary.
#8
It seems to me that the D6 solution is to use the new URL and always specify https (which is what google seems to recommend) - or did I miss something?
--Scott
#9
Perhaps it is the SSL setup on my development server, but the patch in #5 isn't working. Can someone confirm the patch works for them?
Here is how the securepages module does it the HTTPS check.
<?php/**
* Check if the current page is SSL
*/
function securepages_is_secure() {
return (isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'] == 'on') ? TRUE : FALSE;
}
?>
http://drupalcode.org/project/securepages.git/blob/refs/heads/7.x-1.x:/s...
#10
eosrei: drupal_settings_initialize uses the same check to set the $is_https global. Are you using a reverse proxy, non-apache, or other unusual config? What is the value of $_SERVER['HTTPS'] on your box? (you can check by visiting /admin/reports/status/php, under the "apache environment" section).
There are some notes at http://www.metaltoad.com/blog/running-drupal-secure-pages-behind-proxy for dealing with these situations, but I'm certain that
if ($is_https)is the right test.#11
Ok. My server runs Varnish, I figured it was the issue. I don't need this functionality right now, so we won't be setting up Varnish for it.
I can't help but wonder: should both URLs be defined, and this check done in chart_url()?
#12
Here's a better solution:
<?phpdefine('CHART_URI', '//chart.googleapis.com/chart');
?>
Use a protocol-relative URI and let the user's browser decide whether or not it should be accessed securely. Works great with Varnish!
#13
Funny, I just stumbled into this last week. It is a better solution. http://paulirish.com/2010/the-protocol-relative-url/
#14
Seems like a good approach.
#15
Looks like changing it like #12 results in src="///chart..." so line 152 inside chart_url() should probably
<?phpfunction chart_url($chart) {
if ($data = chart_build($chart)) {
return url(CHART_URI, array('query' => $data));
}
return FALSE;
}
?>
#16
#17
Committed, thanks.
#18
Automatically closed -- issue fixed for 2 weeks with no activity.