Full URL in Imagefield path causes SSL page / https warning message

Are you using Private or Public file mode (as set at admin/settings/file-system)? If you're using public, ImageField should be outputting a relative path starting with "/", instead of the full URL. If using private, ImageField should use a full URL. As far as I know, this should respect https protocol, but you may need to manually set $conf['base_url'] in settings.php to use https.

Sorry I mislead you with my last comment. Apparently $base_url is taken into account for both public and private files, however knowing that you're on public files helped me track down the right code that is affecting you.

So right now setting $base_url in settings.php like you have is correct, however it forces https URLs created with file_create_url() to use http. So to fix the problem I think you just need to set your base_url according to which protocol is currently being used:

$base_url = (isset($_SERVER["HTTPS"]) ? 'https' : 'http') . '://www.example.com';

Wouldnt the correct way to fix this be for the imagefile module do the checking rather than force the user to find this issue and have to change it by hand? IMHO using full urls is just bad practice anyways. Why does it use a full path to begin with? And if it has to use the full path, then a check for SSL would make sense so that it doesnt turn the lock into an ! mark.

The source of this problem is not in ImageField, it's in the core file_create_url() function and it only occurs if you've manually forced your $base_url to be a certain value in settings.php. You may be able to avoid the problem entirely just by letting Drupal set the $base_url automatically and not specify anything in settings.php.

hmm, so specifying the base_url breaks file_create_url()? I've not tried it before.

hmm, so specifying the base_url breaks file_create_url()? I've not tried it before.

Specifying $base_url in settings.php does exactly what it's supposed to: it forces all absolute URLs of your site to begin with that URL. It doesn't "break" anything, it just makes it so that even if you're on an https page, the base URL will still be used even if it begins with http instead of https.

Hmm, I just looked at the api page for that function and it does indeed have a missing check for SSL mode.

function file_create_url($path) {
  // Strip file_directory_path from $path. We only include relative paths in urls.
  if (strpos($path, file_directory_path() .'/') === 0) {
    $path = trim(substr($path, strlen(file_directory_path())), '\\/');
  }
  switch (variable_get('file_downloads', FILE_DOWNLOADS_PUBLIC)) {
    case FILE_DOWNLOADS_PUBLIC:
      return $GLOBALS['base_url'] .'/'. file_directory_path() .'/'. str_replace('\\', '/', $path);
    case FILE_DOWNLOADS_PRIVATE:
      return url('system/files/'. $path, array('absolute' => TRUE));
  }
}

should be changed to

function file_create_url($path) {
  // Strip file_directory_path from $path. We only include relative paths in urls.
  if (strpos($path, file_directory_path() .'/') === 0) {
    $path = trim(substr($path, strlen(file_directory_path())), '\\/');
  }
  switch (variable_get('file_downloads', FILE_DOWNLOADS_PUBLIC)) {
    case FILE_DOWNLOADS_PUBLIC:
       //Are we in SSL mode?  If so, we should ALWAYS honor this to respect security.  But we never do this in reverse (allow http when base_url specifies https)
       if (isset($_SERVER["HTTPS"]))
       {
         return str_replace('http://', 'https://', $GLOBALS['base_url']) .'/'. file_directory_path() .'/'. str_replace('\\', '/', $path);  
       }
      return $GLOBALS['base_url'] .'/'. file_directory_path() .'/'. str_replace('\\', '/', $path);
    case FILE_DOWNLOADS_PRIVATE:
      return url('system/files/'. $path, array('absolute' => TRUE));
  }
}

So it seems to me that the whole point of base_url is to be used for https type url's since it's trying to "force" something no matter what the protocol is. Thus it would seem to me that the above code would make sense would it not? We want people to be able to specify https there and have the site NEVER deviate from it to be secure right? Well then we want to make sure the user still has the option to be secure but NEVER have the option to be non-secure. I believe this was the intention of that variable. So knowing this, it's probably best to NOT use the file_create_url() function if it's not functioning properly and use a secure method until that function is fixed? I dont know, just my opinion I guess. I'd rather be secure than not secure.

Sigh. Okay if you want to force using https you just do this:

$base_url = 'https://www.example.com';

That it, your URLs are now forced to start with https://www.example.com. You should of course also redirect your traffic using .htaccess from http to https. No modifications to core are necessary.

The problem highrockmedia was experiencing was because his base_url started with http and he was viewing the page from https. Since he'd told Drupal to force-use http, that's what it did. My suggestion of doing a conditional is only if you want to force your base URL to adjust based on which protocol is being used. And again, you could just leave it out entirely and Drupal will automatically make the right base_url based on the current protocol..

I think you missed the point. The point is that he knows he can force https using base_url, but he also knows that SSL is taxing on a server page and forcing the whole site to SSL is impractical. Why he uses base_url to begin with I dont know. Maybe he uses multisite? But the reason is really irrelevant. "Just leaving it out entirely" is not really an acceptable "fix" to any kind of problem because maybe he CANT leave it out for one reason or another (multisite for example, or maybe his server runs on a strange port, etc, there are several "reasons" for it other than just forcing a protocol). The bottom line is that its a bug that needs to be fixed by someone. But in any case, it's filled in and functioning improperly when viewing an SSL url. Under no circumstance should base_url interfere with anything that is encrypted (like ssl). That means there needs to be a check for SSL mode. Now it DOES make sense however to not work in reverse, that is, do NOT allow non-SSL based urls if the base_url is set to an SSL url.

He was using my uc_ssl module which purposely allows people to run in mixed-ssl mode (that is secure only those pages that really need it) and I told him that in order to fix it he would need to 1. get drupal core team to fix it (not likely), or 2. get the imagefield dev to change it (more likely). But he could not get it working because this module (imagefield) does not properly create paths which resulted in his browser saying "There are insecure items on this page" or whatever it says. It was creating full URLs that include http:// and we now know its because of the use of a core function that does not allow SSL use for one reason or another when base_url is specified with http in it. The bottom line is that it's a bug and it should be fixed, either by this module's author or by the drupal core team. The fastest way to get it fixed is obviously to ask the module developer to do it since the core team has better things to be doing. So the proper way to fix it would be to stop using the core function until it's fixed by the core team and create the url manually the correct way which does check for SSL mode. I really fail to see why such a concept is so hard to understand. It's a 1 line fix. If I had CVS access to this module I'd fix it myself. It's a very simple fix, would break nothing, and would allow people to use imagefield's in a secure website like this guy regardless of what base_url is set to. On his site, he has some blocks that are displayed on cart checkout that makes his site look nice or whatever. But come on now, telling him to just comment base_url and ignore that theres a problem isnt the right approach. I hate to be argumentative but geez I didnt expect something so simple to turn into such an impossible nightmare.

Feel free to move this issue to the core issue queue if you like, however it's not a problem with ImageField.

Moved to D6. This problem needs to be looked at since the module developer feels its a core issue, which it is.

If you specify $base_url, Drupal will respect it. It's difficult to qualify this as a bug.

I dont think it's difficult to qualify this as a bug at all. That's about the same as saying "Mixed SSL mode in a website is stupid". So since we all know thats a dumb statement, I'll just assume that the intention of $base_url was not to break security but instead it was meant to increase it. IMHO $base_url should not include the http part at all and that would fix everything.

//Example 1, lets use walmart since it's the most common example of a 'mixed SSL' website.
$base_url = 'http://walmart.com';

Ok, so drupal encounters this and it knows that walmart.com is the base url. So, a user tries to go to https://walmart.com/checkout and the site redirects it to http://walmart.com/checkout.

Can you in any circumstance tell me why this shouldnt be allowed? I cannot think of any.

Ok, so, now lets REVERSE it.

//Example 2, lets use walmart since it's the most common example of a 'mixed SSL' website.
$base_url = 'https://walmart.com';

Ok, so drupal encounters this and it knows that walmart.com is the base url. So, a user tries to go to http://walmart.com/checkout and the site redirects it to https://walmart.com/checkout. This IMHO would be the correct behavior which is different than the 1st example.

Now this is the only instance I can think of that drupal should disallow the url because it's a potential security risk. However, example 1 is not a security risk so I believe it should be allowed. I guess the whole thing is moot since you could just comment that out, but then it brings up the question, if commenting it out fixes it, what is the purpose of that variable in the first place? Well, my guess is that the purpose of that variable is "security". Thus, since it's probably meant to force security, why in the world would it disallow secure urls when requested?

So maybe the next logical question should be, what is the purpose of the $base_url variable and should it include the protocol. I believe it should NOT include the protocol as it will interfere with security based modules that deal with https. I think opening this to a debate would be prudent as I'd like to see the reasons for it working the way it does.

@Highrockmedia You dont have to be sorry for anything. It's a good valid point and there are probably pros/cons to both sides. The only way a project such as drupal can improve is to have debates one which direction is best. This seems like a good candidate. I dont think this particular variable has ever been discussed before anyways and maybe it's time that it's put to the debate floor because it's clearly not functioning as well as it could in the opinions of some including myself. As a test I just put $base_url in a search on the drupal search box here and I can count no less than six issues related to $base_url messing things up because it's to restrictive and has no brains of it's own to determine whats secure and what isnt. So it's clearly something that could be debated.

I guess my point is that we shouldnt have to have a module in order to overwrite drupal core variables in order to increase security. If it worked the way I believe it should, we wouldnt have posts like this all over the issues forums:

$base_url is problematic as a seed parameter
http://drupal.org/node/778690#comment-2891744

There are modules that overwrite the $base_url parameter in order to enable sites to serve content from both http and https.
I.e. My user profiles (editing) are sent in HTTPS, therefore the $base_url is https://www... but the public profiles are sent in HTTP.

I can't tell you exactly how I came to this (currently I have secure pages disabled) but take a look at:

http://drupal.org/node/61099
http://crackingdrupal.com/blog/greggles/drupal-and-ssl-multiple-recipes-...
http://success.grownupgeek.com/index.php/2009/07/12/drupal-ssl/
http://drupal.org/node/629578
http://drupal.org/project/securepages

Drupal 7 already has explicit knowledge of SSL. There is nothing to fix here for Drupal 6, where the behavior is by design.

So the question still remains. If it's by design, what was it 'designed' to do? Like, what is it's purpose. Was security considered at all when it was designed, etc. If D7 has this fixed, why would it be out of line to look at it's predecessor since it's going to be in use for years to come anyways. The last release date I saw for D7 was sometime in 2011. I dont keep up on D7 since it's not something I deal with daily so I dont know what it's release date is. All I know is that it's no where in the near future, nor is it related to this bug anyways and thus I dont even know why you'd bring up D7 in the first place. The only thing I'm concerned with is looking at why $base_url is designed to work the way it does. I'd be interested to know why it doesnt take into consideration that mixed SSL is a real world application. Since it wont be change I suppose the only way to "fix" it is to use hook_boot() and use logic to change it's behavior. Kinda of a funky way to fix it but I think hook_boot() would be the correct place to do such a task. I'll just have to limp along with a hook_boot fix until someone realizes that it's current 'design' is flawed.

Changing this back to an issue for uc_ssl since it wont be fixed in the parent module/drupal core. This is in 6.x-1.15 and has been tested and it now works correctly. The following code fixed the problem that highrockmedia first encountered. I didnt want to have to do a fix like this, but here it is anyways.

//Since base_url has a design flaw in D5/D6 that seemingly will never be fixed, we fix it here.
function uc_ssl_boot()
{
   if ($GLOBALS['base_url'])
   {
      //If base_url exists and is set to a non-secure protocol while the page being called is secure, allow base_url to be changed to
      //the secure version of the website.  This allows modules such as CCK Imagefield to still use file_create_url() within an SSL/Non-SSL environment.
      if ($_SERVER['HTTPS'] && stristr($GLOBALS['base_url'], 'http://'))
      {
         $GLOBALS['base_url'] = str_ireplace('http://', 'https://', $GLOBALS['base_url']);
      }
   }
}

Fixed in 6.x-1.15