From 265fa12917132631e44dea63a70d750222867344 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Mon, 7 Mar 2016 22:33:34 +0100 Subject: [PATCH] Relatively experimental change to store thumbnails in 'file/thumb/' (by default) --- actions/attachment.php | 2 +- actions/attachment_thumbnail.php | 2 +- classes/File.php | 2 +- classes/File_thumbnail.php | 81 +++++++++++++++++++++++++++----- lib/attachmentlistitem.php | 6 +-- lib/default.php | 11 +++-- lib/gnusocial.php | 22 ++++++++- lib/imagefile.php | 21 +++++++-- plugins/Oembed/OembedPlugin.php | 2 +- 9 files changed, 118 insertions(+), 31 deletions(-) diff --git a/actions/attachment.php b/actions/attachment.php index 1126759832..3ec837a511 100644 --- a/actions/attachment.php +++ b/actions/attachment.php @@ -96,7 +96,7 @@ class AttachmentAction extends ManagedAction { if (empty($this->attachment->filename)) { // if it's not a local file, gtfo - common_redirect($this->attachment->url, 303); + common_redirect($this->attachment->getUrl(), 303); } parent::showPage(); diff --git a/actions/attachment_thumbnail.php b/actions/attachment_thumbnail.php index 3b8eec3ca6..cc1b0f09c6 100644 --- a/actions/attachment_thumbnail.php +++ b/actions/attachment_thumbnail.php @@ -62,6 +62,6 @@ class Attachment_thumbnailAction extends AttachmentAction common_redirect($e->file->getUrl(), 302); } - common_redirect(File_thumbnail::url($thumbnail->filename), 302); + common_redirect(File_thumbnail::url($thumbnail->getFilename()), 302); } } diff --git a/classes/File.php b/classes/File.php index 20e1bc8344..a1a2f78906 100644 --- a/classes/File.php +++ b/classes/File.php @@ -500,7 +500,7 @@ class File extends Managed_DataObject { if ($prefer_local && !empty($this->filename)) { // A locally stored file, so let's generate a URL for our instance. - return self::url($this->filename); + return self::url($this->getFilename()); } // No local filename available, return the URL we have stored diff --git a/classes/File_thumbnail.php b/classes/File_thumbnail.php index e028409f0f..925a3b2429 100644 --- a/classes/File_thumbnail.php +++ b/classes/File_thumbnail.php @@ -129,23 +129,76 @@ class File_thumbnail extends Managed_DataObject static function path($filename) { - // TODO: Store thumbnails in their own directory and don't use File::path here - return File::path($filename); + if (!File::validFilename($filename)) { + // TRANS: Client exception thrown if a file upload does not have a valid name. + throw new ClientException(_('Invalid filename.')); + } + + $dir = common_config('thumbnail', 'dir') ?: File::path('thumb'); + + if (!in_array($dir[mb_strlen($dir)-1], ['/', '\\'])) { + $dir .= DIRECTORY_SEPARATOR; + } + + return $dir . $filename; } static function url($filename) { - // TODO: Store thumbnails in their own directory and don't use File::url here - return File::url($filename); + if (!File::validFilename($filename)) { + // TRANS: Client exception thrown if a file upload does not have a valid name. + throw new ClientException(_('Invalid filename.')); + } + + // FIXME: private site thumbnails? + + $path = common_config('thumbnail', 'path'); + if (empty($path)) { + return File::url('thumb')."/{$filename}"; + } + + $protocol = (GNUsocial::useHTTPS() ? 'https' : 'http'); + $server = common_config('thumbnail', 'server') ?: common_config('site', 'server'); + + if ($path[mb_strlen($path)-1] != '/') { + $path .= '/'; + } + if ($path[0] != '/') { + $path = '/'.$path; + } + + return $protocol.'://'.$server.$path.$filename; + } + + public function getFilename() + { + if (!File::validFilename($this->filename)) { + // TRANS: Client exception thrown if a file upload does not have a valid name. + throw new ClientException(_("Invalid filename.")); + } + return $this->filename; } public function getPath() { - $filepath = self::path($this->filename); - if (!file_exists($filepath)) { - throw new FileNotFoundException($filepath); + $oldpath = File::path($this->getFilename()); + $thumbpath = self::path($this->getFilename()); + + // If we have a file in our old thumbnail storage path, move it to the new one + if (file_exists($oldpath) && !file_exists($thumbpath)) { + if ($this->getFilename() === $this->getFile()->filename) { + // special case where thumbnail file exactly matches stored File + common_debug('File filename and File_thumbnail filename match on '.$this->file_id); + } elseif (!rename($oldpath, $thumbpath)) { + common_log(LOG_ERR, 'Could not move thumbnail from '._ve($oldpath).' to '._ve($thumbpath)); + throw new ServerException('Could not move thumbnail from old path to new path.'); + } else { + common_log(LOG_DEBUG, 'Moved thumbnail '.$this->file_id.' from '._ve($oldpath).' to '._ve($thumbpath)); + } + } elseif (!file_exists($thumbpath)) { + throw new FileNotFoundException($thumbpath); } - return $filepath; + return $thumbpath; } public function getUrl() @@ -188,10 +241,14 @@ class File_thumbnail extends Managed_DataObject public function delete($useWhere=false) { - if (!empty($this->filename) && file_exists(File_thumbnail::path($this->filename))) { - $deleted = @unlink(self::path($this->filename)); - if (!$deleted) { - common_log(LOG_ERR, sprintf('Could not unlink existing file: "%s"', self::path($this->filename))); + if (!empty($this->filename)) { + try { + $deleted = @unlink($this->getPath()); + if (!$deleted) { + common_log(LOG_ERR, 'Could not unlink existing thumbnail file: '._ve($this->getPath())); + } + } catch (FileNotFoundException $e) { + common_log(LOG_INFO, 'Thumbnail already gone from '._ve($e->path)); } } diff --git a/lib/attachmentlistitem.php b/lib/attachmentlistitem.php index e6163ecc92..6ee3c7087b 100644 --- a/lib/attachmentlistitem.php +++ b/lib/attachmentlistitem.php @@ -204,11 +204,7 @@ class AttachmentListItem extends Widget */ protected function scrubHtmlFile(File $attachment) { - $path = File::path($attachment->filename); - if (!file_exists($path) || !is_readable($path)) { - common_log(LOG_ERR, "Missing local HTML attachment $path"); - return false; - } + $path = $attachment->getPath(); $raw = file_get_contents($path); // Normalize... diff --git a/lib/default.php b/lib/default.php index d8b291b344..bf2d37d773 100644 --- a/lib/default.php +++ b/lib/default.php @@ -271,13 +271,18 @@ $default = 'exe' => false, // this would deny any uploads to keep the "exe" file extension ], ), - 'thumbnail' => - array('crop' => false, // overridden to true if thumb height === null + 'thumbnail' => [ + 'dir' => null, // falls back to File::path('thumb') (equivalent to ['attachments']['dir'] . '/thumb/') + 'path' => null, // falls back to generating a URL with File::url('thumb/$filename') (equivalent to ['attachments']['path'] . '/thumb/') + 'server' => null, // Only used if ['thumbnail']['path'] is NOT empty, and then it falls back to ['site']['server'], schema is decided from GNUsocial::useHTTPS() + + 'crop' => false, // overridden to true if thumb height === null 'maxsize' => 1000, // thumbs with an edge larger than this will not be generated 'width' => 450, 'height' => 600, 'upscale' => false, - 'animated' => false), // null="UseFileAsThumbnail", false="can use still frame". true requires ImageMagickPlugin + 'animated' => false, // null="UseFileAsThumbnail", false="can use still frame". true requires ImageMagickPlugin + ], 'application' => array('desclimit' => null), 'group' => diff --git a/lib/gnusocial.php b/lib/gnusocial.php index aecebe2556..f07d2c2446 100644 --- a/lib/gnusocial.php +++ b/lib/gnusocial.php @@ -429,10 +429,28 @@ class GNUsocial */ static function verifyLoadedConfig() { + $mkdirs = []; + if (common_config('htmlpurifier', 'Cache.DefinitionImpl') === 'Serializer' && !is_dir(common_config('htmlpurifier', 'Cache.SerializerPath'))) { - if (!mkdir(common_config('htmlpurifier', 'Cache.SerializerPath'))) { - throw new ConfigException('Could not create HTMLPurifier cache dir: '._ve(common_config('htmlpurifier', 'Cache.SerializerPath'))); + $mkdirs[common_config('htmlpurifier', 'Cache.SerializerPath')] = 'HTMLPurifier Serializer cache'; + } + + // go through our configurable storage directories + foreach (['attachments', 'thumbnail'] as $dirtype) { + $dir = common_config($dirtype, 'dir'); + if (!empty($dir) && !is_dir($dir)) { + $mkdirs[$dir] = $dirtype; + } + } + + // try to create those that are not directories + foreach ($mkdirs as $dir=>$description) { + if (is_file($dir)) { + throw new ConfigException('Expected directory for '._ve($description).' is a file!'); + } + if (!mkdir($dir)) { + throw new ConfigException('Could not create directory for '._ve($description).': '._ve($dir)); } } diff --git a/lib/imagefile.php b/lib/imagefile.php index 9f870ae290..c707208af6 100644 --- a/lib/imagefile.php +++ b/lib/imagefile.php @@ -153,8 +153,14 @@ class ImageFile $image = new ImageFile($file->getID(), $imgPath); } catch (UnsupportedMediaException $e) { // Avoid deleting the original - if ($imgPath != $file->getPath()) { - unlink($imgPath); + try { + if ($imgPath !== $file->getPath()) { + @unlink($imgPath); + } + } catch (FileNotFoundException $e) { + // File reported (via getPath) that the original file + // doesn't exist anyway, so it's safe to delete $imgPath + @unlink($imgPath); } throw $e; } @@ -607,10 +613,15 @@ class ImageFile // Perform resize and store into file $this->resizeTo($outpath, $box); - // Avoid deleting the original - if ($this->getPath() != File_thumbnail::path($this->filename)) { - $this->unlink(); + try { + // Avoid deleting the original + if (!in_array($this->getPath(), [File::path($this->filename), File_thumbnail::path($this->filename)])) { + $this->unlink(); + } + } catch (FileNotFoundException $e) { + // $this->getPath() says the file doesn't exist anyway, so no point in trying to delete it! } + return File_thumbnail::saveThumbnail($this->fileRecord->getID(), null, // no url since we generated it ourselves and can dynamically generate the url $width, $height, diff --git a/plugins/Oembed/OembedPlugin.php b/plugins/Oembed/OembedPlugin.php index 196b07d75d..cb59bb21e5 100644 --- a/plugins/Oembed/OembedPlugin.php +++ b/plugins/Oembed/OembedPlugin.php @@ -328,7 +328,7 @@ class OembedPlugin extends Plugin $ext = File::guessMimeExtension($info['mime']); // We'll trust sha256 (File::FILEHASH_ALG) not to have collision issues any time soon :) - $filename = hash(File::FILEHASH_ALG, $imgData) . ".{$ext}"; + $filename = 'oembed-'.hash(File::FILEHASH_ALG, $imgData) . ".{$ext}"; $fullpath = File_thumbnail::path($filename); // Write the file to disk. Throw Exception on failure if (!file_exists($fullpath) && file_put_contents($fullpath, $imgData) === false) { -- 2.39.5