Using MAMP with PHP 5.2, there is a bug when setting a custom image (or favicon) for a theme.

To reproduce:
Go to theme-specific Appearance Settings (For Bartik: admin/appearance/settings/bartik)
Under Logo Image Settings, uncheck "Use the default logo"
Upload a custom image ("Upload logo image")
Click "Save configuration" twice (either right away, or at any other time without changing the image settings)

Expected result:
Custom image as the logo, with a url of "example.com/sites/default/files/FILENAME"

Actual result:
No image, the img tag still exists, but with a url of "example.com/FILENAME"

Cause:
The changes to drupal_realpath() from #700160: drupal_realpath does not always work as expected are only partially correct. In PHP5.2, realpath() on BSD does not return FALSE if the path does not exist, not just if !empty(path).

I've opened this against _system_theme_settings_validate_path() because that's how I found the bug, but this is really an issue with drupal_realpath() itself.

Comments

tim.plunkett’s picture

Status: Active » Needs review
StatusFileSize
new722 bytes

chx suggested file_exists().
I'm pretty sure there is a way to do this without two calls to drupal_realpath(), but I can't figure out how to do it without PHP notices. I'm still new, don't bite!

tim.plunkett’s picture

Version: 7.x-dev » 8.x-dev

Patch still applies, not sure if this is really still needed.

sun’s picture

Status: Needs review » Needs work
+++ modules/system/system.admin.inc	27 Sep 2010 21:45:05 -0000
@@ -723,7 +723,7 @@ function system_theme_settings_validate(
 function _system_theme_settings_validate_path($path) {
-  if (drupal_realpath($path)) {
+  if (drupal_realpath($path) && file_exists(drupal_realpath($path))) {

I think we need to omit the drupal_realpath() in the second condition, because $path may be a stream wrapper URI (e.g., flickr://logo.png)

5 days to next Drupal core point release.

sun’s picture

tim.plunkett’s picture

Version: 8.x-dev » 7.x-dev
Status: Needs work » Needs review
StatusFileSize
new562 bytes

That other issue resolved this for D8, and it doesn't support 5.2 anyway.

Status: Needs review » Needs work

The last submitted patch, drupal-924396-5.patch, failed testing.

Status: Needs work » Closed (outdated)

Automatically closed because Drupal 7 security and bugfix support has ended as of 5 January 2025. If the issue verifiably applies to later versions, please reopen with details and update the version.