From b4a0bff740b7b654bd405f27910c62a60ec58fc7 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Wed, 6 Jul 2016 08:59:16 +0200 Subject: [PATCH 1/7] Some mimetype madness! --- classes/File.php | 11 +++++----- lib/default.php | 3 ++- lib/unknownmimeextensionexception.php | 30 +++++++++++++++++++++++++++ lib/util.php | 2 +- 4 files changed, 39 insertions(+), 7 deletions(-) create mode 100644 lib/unknownmimeextensionexception.php diff --git a/classes/File.php b/classes/File.php index b28f1373d6..6197539d94 100644 --- a/classes/File.php +++ b/classes/File.php @@ -304,13 +304,12 @@ class File extends Managed_DataObject $ext = common_supported_mime_to_ext($mimetype); // we do, so use it! return $ext; - } catch (Exception $e) { // FIXME: Make this exception more specific to "unknown mime=>ext relation" + } catch (UnknownMimeExtensionException $e) { // We don't know the extension for this mimetype, but let's guess. - // If we are very liberal with uploads ($config['attachments']['supported'] === true) - // then we try to do some guessing based on the filename, if it was supplied. - if (!is_null($filename) && common_config('attachments', 'supported')===true - && preg_match('/^.+\.([A-Za-z0-9]+)$/', $filename, $matches)) { + // If we can't recognize the extension from the MIME, we try + // to guess based on filename, if one was supplied. + if (!is_null($filename) && preg_match('/^.+\.([A-Za-z0-9]+)$/', $filename, $matches)) { // we matched on a file extension, so let's see if it means something. $ext = mb_strtolower($matches[1]); @@ -330,6 +329,8 @@ class File extends Managed_DataObject // the attachment extension based on its filename was not blacklisted so it's ok to use it return $ext; } + } catch (Exception $e) { + common_log(LOG_INFO, 'Problem when figuring out extension for mimetype: '._ve($e)); } // If nothing else has given us a result, try to extract it from diff --git a/lib/default.php b/lib/default.php index b3685a284c..5e711bb87c 100644 --- a/lib/default.php +++ b/lib/default.php @@ -249,6 +249,7 @@ $default = 'application/zip' => 'zip', 'application/x-go-sgf' => 'sgf', 'application/xml' => 'xml', + 'application/gpx+xml' => 'gpx', 'image/png' => 'png', 'image/jpeg' => 'jpg', 'image/gif' => 'gif', @@ -273,7 +274,7 @@ $default = 'show_thumbs' => true, // show thumbnails in notice lists for uploaded images, and photos and videos linked remotely that provide oEmbed info 'process_links' => true, // check linked resources for embeddable photos and videos; this will hit referenced external web sites when processing new messages. 'extblacklist' => [ - 'php' => 'phps', + 'php' => 'phps', // this turns .php into .phps 'exe' => false, // this would deny any uploads to keep the "exe" file extension ], ), diff --git a/lib/unknownmimeextensionexception.php b/lib/unknownmimeextensionexception.php new file mode 100644 index 0000000000..0937467d07 --- /dev/null +++ b/lib/unknownmimeextensionexception.php @@ -0,0 +1,30 @@ + + * @license https://www.gnu.org/licenses/agpl-3.0.html + * @link https://gnu.io/social + */ + +class UnknownMimeExtensionException extends ServerException +{ + public function __construct($msg=null) + { + if ($msg === null) { + // TRANS: We accept the file type (we probably just accept all files) + // TRANS: but don't know the file extension for it. + $msg = _('Supported mimetype but unknown extension relation.'); + } + + parent::__construct($msg); + } +} diff --git a/lib/util.php b/lib/util.php index a2415945f1..8d95ec2305 100644 --- a/lib/util.php +++ b/lib/util.php @@ -2016,7 +2016,7 @@ function common_supported_mime_to_ext($mimetype) { $supported = common_config('attachments', 'supported'); if ($supported === true) { - throw new ServerException('Supported mimetype but unknown extension relation.'); + throw new UnknownMimeExtensionException(); } foreach($supported as $type => $ext) { if ($mimetype === $type) { From 4117118e230e1fd27922600337a3910bebb3791d Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Wed, 6 Jul 2016 09:14:59 +0200 Subject: [PATCH 2/7] More specific exceptions for mimetype/extension issues. --- lib/unknownextensionmimeexception.php | 26 ++++++++++++++++++++++++++ lib/unknownmimeextensionexception.php | 11 ++++------- lib/util.php | 4 ++-- 3 files changed, 32 insertions(+), 9 deletions(-) create mode 100644 lib/unknownextensionmimeexception.php diff --git a/lib/unknownextensionmimeexception.php b/lib/unknownextensionmimeexception.php new file mode 100644 index 0000000000..faa6e9cf25 --- /dev/null +++ b/lib/unknownextensionmimeexception.php @@ -0,0 +1,26 @@ + + * @license https://www.gnu.org/licenses/agpl-3.0.html + * @link https://gnu.io/social + */ + +class UnknownExtensionMimeException extends ServerException +{ + public function __construct($ext) + { + // TRANS: We accept the file type (we probably just accept all files) + // TRANS: but don't know the file extension for it. %1$s is the extension. + $msg = sprintf(_('Unknown MIME type for file extension: %1$s'), _ve($ext)); + + parent::__construct($msg); + } +} diff --git a/lib/unknownmimeextensionexception.php b/lib/unknownmimeextensionexception.php index 0937467d07..fbc3a67742 100644 --- a/lib/unknownmimeextensionexception.php +++ b/lib/unknownmimeextensionexception.php @@ -17,14 +17,11 @@ if (!defined('GNUSOCIAL')) { exit(1); } class UnknownMimeExtensionException extends ServerException { - public function __construct($msg=null) + public function __construct($mimetype) { - if ($msg === null) { - // TRANS: We accept the file type (we probably just accept all files) - // TRANS: but don't know the file extension for it. - $msg = _('Supported mimetype but unknown extension relation.'); - } - + // TRANS: We accept the file type (we probably just accept all files) + // TRANS: but don't know the file extension for it. + $msg = sprintf(_('Supported mimetype but unknown extension relation: %1$s'), _ve($mimetype)); parent::__construct($msg); } } diff --git a/lib/util.php b/lib/util.php index 8d95ec2305..846605bc7f 100644 --- a/lib/util.php +++ b/lib/util.php @@ -2000,7 +2000,7 @@ function common_supported_ext_to_mime($fileext) $supported = common_config('attachments', 'supported'); if ($supported === true) { - throw new ServerException('Supported extension but unknown mimetype relation.'); + throw new UnknownExtensionMimeException($fileext); } foreach($supported as $type => $ext) { if ($ext === $fileext) { @@ -2016,7 +2016,7 @@ function common_supported_mime_to_ext($mimetype) { $supported = common_config('attachments', 'supported'); if ($supported === true) { - throw new UnknownMimeExtensionException(); + throw new UnknownMimeExtensionException($mimetype); } foreach($supported as $type => $ext) { if ($mimetype === $type) { From 71afb5be75decf19b5fd7bb294d2b0f9ed92272e Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Wed, 6 Jul 2016 09:34:09 +0200 Subject: [PATCH 3/7] If the file is text/plain, see if we accept the extension --- lib/mediafile.php | 7 +++++-- lib/util.php | 26 ++++++++++++++++---------- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/lib/mediafile.php b/lib/mediafile.php index 54d00b4acf..803cbe0a4c 100644 --- a/lib/mediafile.php +++ b/lib/mediafile.php @@ -355,6 +355,7 @@ class MediaFile $unclearTypes = array('application/octet-stream', 'application/vnd.ms-office', 'application/zip', + 'text/plain', 'text/html', // Ironically, Wikimedia Commons' SVG_logo.svg is identified as text/html // TODO: for XML we could do better content-based sniffing too 'text/xml'); @@ -364,10 +365,12 @@ class MediaFile // If we didn't match, or it is an unclear match if ($originalFilename && (!$mimetype || in_array($mimetype, $unclearTypes))) { try { - $type = common_supported_ext_to_mime($originalFilename); + $type = common_supported_filename_to_mime($originalFilename); return $type; + } catch (UnknownExtensionMimeException $e) { + // FIXME: I think we should keep the file extension here (supported should be === true here) } catch (Exception $e) { - // Extension not found, so $mimetype is our best guess + // Extension parsed but no connected mimetype, so $mimetype is our best guess } } diff --git a/lib/util.php b/lib/util.php index 846605bc7f..985b3773df 100644 --- a/lib/util.php +++ b/lib/util.php @@ -1991,15 +1991,22 @@ function common_accept_to_prefs($accept, $def = '*/*') } // Match by our supported file extensions -function common_supported_ext_to_mime($fileext) +function common_supported_filename_to_mime($filename) { // Accept a filename and take out the extension - if (strpos($fileext, '.') !== false) { - $fileext = substr(strrchr($fileext, '.'), 1); + if (strpos($filename, '.') === false) { + throw new ServerException(sprintf('No extension on filename: %1$s', _ve($filename))); } + $fileext = substr(strrchr($filename, '.'), 1); + return common_supported_ext_to_mime($fileext); +} + +function common_supported_ext_to_mime($fileext) +{ $supported = common_config('attachments', 'supported'); if ($supported === true) { + // FIXME: Should we just accept the extension straight off when supported === true? throw new UnknownExtensionMimeException($fileext); } foreach($supported as $type => $ext) { @@ -2015,16 +2022,15 @@ function common_supported_ext_to_mime($fileext) function common_supported_mime_to_ext($mimetype) { $supported = common_config('attachments', 'supported'); - if ($supported === true) { - throw new UnknownMimeExtensionException($mimetype); - } - foreach($supported as $type => $ext) { - if ($mimetype === $type) { - return $ext; + if (is_array($supported)) { + foreach($supported as $type => $ext) { + if ($mimetype === $type) { + return $ext; + } } } - throw new ServerException('Unsupported MIME type'); + throw new UnknownMimeExtensionException($mimetype); } // The MIME "media" is the part before the slash (video in video/webm) From 4a3ed7d0aec07c8b94dded9edf8d3d859ffc6488 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Thu, 7 Jul 2016 00:43:51 +0200 Subject: [PATCH 4/7] I don't know why we would set the mimetype as title here --- plugins/StoreRemoteMedia/StoreRemoteMediaPlugin.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/StoreRemoteMedia/StoreRemoteMediaPlugin.php b/plugins/StoreRemoteMedia/StoreRemoteMediaPlugin.php index 650f27c21f..2e42802724 100644 --- a/plugins/StoreRemoteMedia/StoreRemoteMediaPlugin.php +++ b/plugins/StoreRemoteMedia/StoreRemoteMediaPlugin.php @@ -55,7 +55,7 @@ class StoreRemoteMediaPlugin extends Plugin switch (common_get_mime_media($file->mimetype)) { case 'image': // Just to set something for now at least... - $file->title = $file->mimetype; + //$file->title = $file->mimetype; break; } From f02d32b718de732181d0f73beed2b7fdb77b1cbd Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Thu, 7 Jul 2016 00:44:50 +0200 Subject: [PATCH 5/7] Reworked File->getUrl to throw exception In case you require a local URL and one can't be generated, throw FileNotStoredLocallyException(File $file) --- classes/File.php | 18 ++++++++++++++---- lib/filenotstoredlocallyexception.php | 15 +++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) create mode 100644 lib/filenotstoredlocallyexception.php diff --git a/classes/File.php b/classes/File.php index 6197539d94..7bd7da27ba 100644 --- a/classes/File.php +++ b/classes/File.php @@ -529,13 +529,23 @@ class File extends Managed_DataObject return common_local_url('attachment', array('attachment'=>$this->getID())); } - public function getUrl($prefer_local=true) + /** + * @param mixed $use_local true means require local, null means prefer local, false means use whatever is stored + */ + public function getUrl($use_local=null) { - if ($prefer_local && !empty($this->filename)) { - // A locally stored file, so let's generate a URL for our instance. - return self::url($this->getFilename()); + if ($use_local !== false) { + if (is_string($this->filename) || !empty($this->filename)) { + // A locally stored file, so let's generate a URL for our instance. + return self::url($this->getFilename()); + } + if ($use_local) { + // if the file wasn't stored locally (has filename) and we require a local URL + throw new FileNotStoredLocallyException($this); + } } + // No local filename available, return the URL we have stored return $this->url; } diff --git a/lib/filenotstoredlocallyexception.php b/lib/filenotstoredlocallyexception.php new file mode 100644 index 0000000000..2d2ad8fc37 --- /dev/null +++ b/lib/filenotstoredlocallyexception.php @@ -0,0 +1,15 @@ +file = $file; + common_debug('Requested local URL for a file that is not stored locally with id=='._ve($this->file->getID())); + parent::__construct(_('Requested local URL for a file that is not stored locally.')); + } +} From 6332a4d800c7cb63f5da6eae25d82135cfc0d244 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Thu, 7 Jul 2016 00:45:31 +0200 Subject: [PATCH 6/7] Handle FileNotStoredLocallyException in attachmentlistitem --- lib/attachmentlistitem.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/attachmentlistitem.php b/lib/attachmentlistitem.php index dc22c8af29..133756147c 100644 --- a/lib/attachmentlistitem.php +++ b/lib/attachmentlistitem.php @@ -63,7 +63,7 @@ class AttachmentListItem extends Widget } function title() { - return $this->attachment->getTitle(); + return $this->attachment->getTitle() ?: _('Untitled attachment'); } function linkTitle() { @@ -141,7 +141,13 @@ class AttachmentListItem extends Widget if ($thumb instanceof File_thumbnail) { $this->out->element('img', $thumb->getHtmlAttrs(['class'=>'u-photo', 'alt' => ''])); } else { - $this->out->element('img', array('class'=>'u-photo', 'src' => $this->attachment->getUrl(), 'alt' => $this->attachment->getTitle())); + try { + // getUrl(true) because we don't want to hotlink, could be made configurable + $this->out->element('img', ['class'=>'u-photo', 'src'=>$this->attachment->getUrl(true), 'alt' => $this->attachment->getTitle()]); + } catch (FileNotStoredLocallyException $e) { + $url = $e->file->getUrl(false); + $this->out->element('a', ['href'=>$url, 'rel'=>'external'], $url); + } } unset($thumb); // there's no need carrying this along after this break; From 1d53e7060ada2f075955d1c3e32357279abab880 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Thu, 7 Jul 2016 11:11:20 +0200 Subject: [PATCH 7/7] Changed ShowfavoritesAction to use Action functions for profiles --- plugins/Favorite/actions/showfavorites.php | 23 +++++++++++----------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/plugins/Favorite/actions/showfavorites.php b/plugins/Favorite/actions/showfavorites.php index ea1169957c..08366e4c16 100644 --- a/plugins/Favorite/actions/showfavorites.php +++ b/plugins/Favorite/actions/showfavorites.php @@ -65,53 +65,52 @@ class ShowfavoritesAction extends ShowstreamAction return array(new Feed(Feed::JSON, common_local_url('ApiTimelineFavorites', array( - 'id' => $this->user->nickname, + 'id' => $this->getTarget()->getNickname(), 'format' => 'as')), // TRANS: Feed link text. %s is a username. sprintf(_('Feed for favorites of %s (Activity Streams JSON)'), - $this->user->nickname)), + $this->getTarget()->getNickname())), new Feed(Feed::RSS1, common_local_url('favoritesrss', - array('nickname' => $this->user->nickname)), + array('nickname' => $this->getTarget()->getNickname())), // TRANS: Feed link text. %s is a username. sprintf(_('Feed for favorites of %s (RSS 1.0)'), - $this->user->nickname)), + $this->getTarget()->getNickname())), new Feed(Feed::RSS2, common_local_url('ApiTimelineFavorites', array( - 'id' => $this->user->nickname, + 'id' => $this->getTarget()->getNickname(), 'format' => 'rss')), // TRANS: Feed link text. %s is a username. sprintf(_('Feed for favorites of %s (RSS 2.0)'), - $this->user->nickname)), + $this->getTarget()->getNickname())), new Feed(Feed::ATOM, common_local_url('ApiTimelineFavorites', array( - 'id' => $this->user->nickname, + 'id' => $this->getTarget()->getNickname(), 'format' => 'atom')), // TRANS: Feed link text. %s is a username. sprintf(_('Feed for favorites of %s (Atom)'), - $this->user->nickname))); + $this->getTarget()->getNickname()))); } function showEmptyListMessage() { if (common_logged_in()) { - $current_user = common_current_user(); - if ($this->user->id === $current_user->id) { + if ($this->getTarget()->sameAs($this->getScoped())) { // TRANS: Text displayed instead of favourite notices for the current logged in user that has no favourites. $message = _('You haven\'t chosen any favorite notices yet. Click the fave button on notices you like to bookmark them for later or shed a spotlight on them.'); } else { // TRANS: Text displayed instead of favourite notices for a user that has no favourites while logged in. // TRANS: %s is a username. - $message = sprintf(_('%s hasn\'t added any favorite notices yet. Post something interesting they would add to their favorites :)'), $this->user->nickname); + $message = sprintf(_('%s hasn\'t added any favorite notices yet. Post something interesting they would add to their favorites :)'), $this->getTarget()->getNickname()); } } else { // TRANS: Text displayed instead of favourite notices for a user that has no favourites while not logged in. // TRANS: %s is a username, %%%%action.register%%%% is a link to the user registration page. // TRANS: (link text)[link] is a Mark Down link. - $message = sprintf(_('%s hasn\'t added any favorite notices yet. Why not [register an account](%%%%action.register%%%%) and then post something interesting they would add to their favorites :)'), $this->user->nickname); + $message = sprintf(_('%s hasn\'t added any favorite notices yet. Why not [register an account](%%%%action.register%%%%) and then post something interesting they would add to their favorites :)'), $this->getTarget()->getNickname()); } $this->elementStart('div', 'guide');