Closed (fixed)
Project:
Google Chart Tools: Image Charts
Version:
7.x-1.x-dev
Component:
Code
Priority:
Critical
Category:
Bug report
Assigned:
Issue tags:
Reporter:
Created:
25 Sep 2009 at 11:25 UTC
Updated:
3 Jan 2014 at 00:29 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
tjholowaychuk commentedwhy 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)
Comment #2
boombatower commentedIt doesn't seem https:// on the front works with Google Chart API anyway.
Comment #3
boombatower commentedActually it seems that they do, but not with the old url currently be used.
Would need to be:
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.
Comment #4
boombatower commentedCommitted that change to 6.x-1.x and 7.x-1.x branches.
Comment #5
grendzy commentedIt'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):
Comment #6
grendzy commentedD6 patch - this is the same condition, but the $is_https global isn't available in D6.
Comment #7
raspberryman commented+1 - I can confirm that these browser security warnings are very, very scary.
Comment #8
ScottW commentedIt 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
Comment #9
13rac1 commentedPerhaps 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.
http://drupalcode.org/project/securepages.git/blob/refs/heads/7.x-1.x:/s...
Comment #10
grendzy commentedeosrei: 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.Comment #11
13rac1 commentedOk. 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()?
Comment #12
greenreaperHere's a better solution:
Use a protocol-relative URI and let the user's browser decide whether or not it should be accessed securely. Works great with Varnish!
Comment #13
grendzy commentedFunny, I just stumbled into this last week. It is a better solution. http://paulirish.com/2010/the-protocol-relative-url/
Comment #14
boombatower commentedSeems like a good approach.
Comment #15
boombatower commentedLooks like changing it like #12 results in src="///chart..." so line 152 inside chart_url() should probably
Comment #16
boombatower commentedComment #17
boombatower commentedCommitted, thanks.