From 820dd293c998f3c5a15f637467ba6ce2c9793183 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Mon, 24 Jan 2011 12:22:47 -0800 Subject: [PATCH] Fix for ticket #3007: .bmp avatar uploads weren't being properly converted to PNG in all cases Part of the reported issue was previuosly fixed by dc497ed0 (smaller size images being blanked). This commit fixes the remaining bug with original-size avatars being left as BMP (which could include the 96px size for instance, which could cause problems in browsers not supporting BMP natively) Added ImageFile::copyTo() as a convenient alias for resizeTo() when not resizing; this performs the BMP/XPM/XBM->PNG conversion if needed, or copies the original file. Copying instead of using move_uploaded_file() is fine here since: a) the files are cleaned up on script completion anyway (vs moving to remove it) b) we're already performing getimagesize() and possibly load/resize on the file before this point (vs needing to move the file into a usable area to work with open_basedir restrictions that prevent working directly with uploaded files in the temp dir; since this would fail anyway, we lose nothing) ImageFile::preferredType() now works on $this->type instead of asking for one, to make it handier to use from outside. (This is still needed in order for calling code to generate a target filename.) Recommended for future: * additional consolidation between the various ways of uploading avatars (touched avatarsettings, grouplogo, and apiaccountupdateprofileimage with similar minor changes) * consolidate type checks and file naming into Avatar class --- actions/apiaccountupdateprofileimage.php | 5 ++-- actions/avatarsettings.php | 9 ++++---- actions/grouplogo.php | 7 +++--- lib/imagefile.php | 29 +++++++++++++++++------- 4 files changed, 32 insertions(+), 18 deletions(-) diff --git a/actions/apiaccountupdateprofileimage.php b/actions/apiaccountupdateprofileimage.php index f2886509d7..6c53e2cbc1 100644 --- a/actions/apiaccountupdateprofileimage.php +++ b/actions/apiaccountupdateprofileimage.php @@ -112,16 +112,17 @@ class ApiAccountUpdateProfileImageAction extends ApiAuthAction return; } + $type = $imagefile->preferredType(); $filename = Avatar::filename( $user->id, - image_type_to_extension($imagefile->type), + image_type_to_extension($type), null, 'tmp'.common_timestamp() ); $filepath = Avatar::path($filename); - move_uploaded_file($imagefile->filepath, $filepath); + $imagefile->copyTo($filepath); $profile = $this->user->getProfile(); diff --git a/actions/avatarsettings.php b/actions/avatarsettings.php index 375420c5c9..10c82ebfd7 100644 --- a/actions/avatarsettings.php +++ b/actions/avatarsettings.php @@ -320,21 +320,20 @@ class AvatarsettingsAction extends AccountSettingsAction } $cur = common_current_user(); - + $type = $imagefile->preferredType(); $filename = Avatar::filename($cur->id, - image_type_to_extension($imagefile->type), + image_type_to_extension($type), null, 'tmp'.common_timestamp()); $filepath = Avatar::path($filename); - - move_uploaded_file($imagefile->filepath, $filepath); + $imagefile->copyTo($filepath); $filedata = array('filename' => $filename, 'filepath' => $filepath, 'width' => $imagefile->width, 'height' => $imagefile->height, - 'type' => $imagefile->type); + 'type' => $type); $_SESSION['FILEDATA'] = $filedata; diff --git a/actions/grouplogo.php b/actions/grouplogo.php index 7b7c01b23b..d2e8fd0e91 100644 --- a/actions/grouplogo.php +++ b/actions/grouplogo.php @@ -353,20 +353,21 @@ class GrouplogoAction extends GroupDesignAction return; } + $type = $imagefile->preferredType(); $filename = Avatar::filename($this->group->id, - image_type_to_extension($imagefile->type), + image_type_to_extension($type), null, 'group-temp-'.common_timestamp()); $filepath = Avatar::path($filename); - move_uploaded_file($imagefile->filepath, $filepath); + $imagefile->copyTo($filepath); $filedata = array('filename' => $filename, 'filepath' => $filepath, 'width' => $imagefile->width, 'height' => $imagefile->height, - 'type' => $imagefile->type); + 'type' => $type); $_SESSION['FILEDATA'] = $filedata; diff --git a/lib/imagefile.php b/lib/imagefile.php index 159deead61..56a7b7d4a2 100644 --- a/lib/imagefile.php +++ b/lib/imagefile.php @@ -128,7 +128,7 @@ class ImageFile */ function resize($size, $x = 0, $y = 0, $w = null, $h = null) { - $targetType = $this->preferredType($this->type); + $targetType = $this->preferredType(); $outname = Avatar::filename($this->id, image_type_to_extension($targetType), $size, @@ -138,6 +138,19 @@ class ImageFile return $outname; } + /** + * Copy the image file to the given destination. + * For obscure formats, this will automatically convert to PNG; + * otherwise the original file will be copied as-is. + * + * @param string $outpath + * @return string filename + */ + function copyTo($outpath) + { + return $this->resizeTo($outpath, $this->width, $this->height); + } + /** * Create and save a thumbnail image. * @@ -154,7 +167,7 @@ class ImageFile { $w = ($w === null) ? $this->width:$w; $h = ($h === null) ? $this->height:$h; - $targetType = $this->preferredType($this->type); + $targetType = $this->preferredType(); if (!file_exists($this->filepath)) { throw new Exception(_('Lost our file.')); @@ -247,25 +260,25 @@ class ImageFile /** * Several obscure file types should be normalized to PNG on resize. * - * @param int $type + * @fixme consider flattening anything not GIF or JPEG to PNG * @return int */ - function preferredType($type) + function preferredType() { - if($type == IMAGETYPE_BMP) { + if($this->type == IMAGETYPE_BMP) { //we don't want to save BMP... it's an inefficient, rare, antiquated format //save png instead return IMAGETYPE_PNG; - } else if($type == IMAGETYPE_WBMP) { + } else if($this->type == IMAGETYPE_WBMP) { //we don't want to save WBMP... it's a rare format that we can't guarantee clients will support //save png instead return IMAGETYPE_PNG; - } else if($type == IMAGETYPE_XBM) { + } else if($this->type == IMAGETYPE_XBM) { //we don't want to save XBM... it's a rare format that we can't guarantee clients will support //save png instead return IMAGETYPE_PNG; } - return $type; + return $this->type; } function unlink() -- 2.39.5