[4278]Spaces + getImageSize - Avatar Upload Bug

[Shotster]

The following function in Subs.php assumes a url for an argument when it might actually be a file system path...

Code Select Expand


function url_image_size($url)
{
    global $sourcedir;

    // Make sure it is a proper URL.
    $url = str_replace(' ', '%20', $url); // (<-- BORK!)

    // rest of function...
}


A space is a perfectly valid UNIX file name character, so that means when this function is invoked passing a filesystem path as the argument (as is done when uploading an avatar image) and that path contains one or more spaces, then the call to getimagesize later in the function returns false which of course means a bogus value for $sizes in function downloadAvatar in Subs-Graphics.php which of course returns false from function profileSaveAvatarData in Profile-Modify.php which of course means a confusing error message of "The avatar you have selected is either too large or not an avatar." in the UI. It's confusing because the setting to automatically resize an uploaded avatar is enabled.

Granted, you're not likely to find spaces in the names of files or directories on a web hosting server, but I did encounter the issue on my local development machine. The logic is flawed, but the fix is simple.

-Steve

[Arantor]

A space is a perfectly valid character on Windows too, and I've never had a problem.

AFAIR this function is for specifically the remote avatar specified by URL - which unless you're behind a firewall and passing it a shared resource that everyone would have access to as a direct 'URL', it's a non-issue.

[karlbenson]

AFAIK the function is necessary because the function getImageSize will NOT accept spaces in the filename/url.

http://uk.php.net/manual/en/function.getimagesize.php#42970
But we can't use a space.

[Shotster]

A space is a perfectly valid character on Windows too, and I've never had a problem.

Ok, I thought I was clear, but obviously I wasn't. Here's the scoop...

When uploading an avatar (at least with the auto-resize option enabled), the function url_image_size is called with a file system path (not a URL) for the $url argument. The path is to the temporary avatar image in the attachments directory. The first thing the function does is replace all spaces in the path name with %20. This means the call to getimagesize is passed a non-existent file path...

/path to/myfile

...becomes...

/path%20to/myfile

The problem is not with getimagesize. It's telling the truth. The problem is that the function argument is assumed to be a URL, and it's not.

AFAIR this function is for specifically the remote avatar specified by URL...

Well I'm here to tell you that function is also called when uploading an avatar (at least with the auto-resize option enabled) - not just referencing a remote one. Here's the function call chain...

My hack was to simply check that the passed argument is in fact a URL like so...

Code Select Expand


function url_image_size($url)
{
    global $sourcedir;

    // If it _is_ a URL...
    if (strpos(trim($url),'http') === 0) {
        // Make sure it is a proper URL.
        $url = str_replace(' ', '%20', $url);
    }
    // rest of function...
}


I hope that clarifies the issue.

-Steve

[Shotster]

AFAIK the function is necessary because the function getImageSize will NOT accept spaces in the filename/url.

Sure it will, if the spaces are part of a valid file path. See my latest response. Better yet, step through the code and see for yourself.

-Steve

[JFlame]

Not sure if this has been addressed yet, but I just ran into this on my local dev server. 

My document root had spaces in it and I got the error about the avatar being too large.  I moved my document root to a path with no spaces and all is well.

[Norv]

It wasn't addressed, or not that I know of, if it was.

Thank you very much for the report, and for sharing the investigation details.

[[SiNaN]]

Fixed with Revision 9865.