From dffeb1f9d5b47ecd2a352afe63ab0931254a324f Mon Sep 17 00:00:00 2001 From: Thomas Keller Date: Thu, 2 Dec 2010 01:50:01 +0100 Subject: [PATCH] * move common file-specific functionality out of IDF_Views_Source into new IDF_FileUtil and change all occurrences accordingly * cache /etc/mime.types (or whatever is configured) per request in a static variable in IDF_FileUtil * always link directly to the download of attached files in the issues view and place an additional "view" link only for those attachments which we recognize as text with our weak criteria (closes issue 575) --- src/IDF/Diff.php | 8 +- src/IDF/FileUtil.php | 165 +++++++++++++++++++++++++ src/IDF/IssueFile.php | 18 ++- src/IDF/Template/Markdown.php | 2 +- src/IDF/Tests/TestFileUtil.php | 47 +++++++ src/IDF/Tests/TestSource.php | 15 +-- src/IDF/Views/Issue.php | 26 ++-- src/IDF/Views/Source.php | 130 +------------------ src/IDF/templates/idf/issues/view.html | 13 +- 9 files changed, 254 insertions(+), 170 deletions(-) create mode 100644 src/IDF/FileUtil.php create mode 100644 src/IDF/Tests/TestFileUtil.php diff --git a/src/IDF/Diff.php b/src/IDF/Diff.php index d6abe56..f097183 100644 --- a/src/IDF/Diff.php +++ b/src/IDF/Diff.php @@ -169,8 +169,8 @@ class IDF_Diff $out = ''; foreach ($this->files as $filename=>$file) { $pretty = ''; - $fileinfo = IDF_Views_Source::getMimeType($filename); - if (IDF_Views_Source::isSupportedExtension($fileinfo[2])) { + $fileinfo = IDF_FileUtil::getMimeType($filename); + if (IDF_FileUtil::isSupportedExtension($fileinfo[2])) { $pretty = ' prettyprint'; } $out .= "\n".''."\n"; @@ -350,9 +350,9 @@ class IDF_Diff public function renderCompared($chunks, $filename) { - $fileinfo = IDF_Views_Source::getMimeType($filename); + $fileinfo = IDF_FileUtil::getMimeType($filename); $pretty = ''; - if (IDF_Views_Source::isSupportedExtension($fileinfo[2])) { + if (IDF_FileUtil::isSupportedExtension($fileinfo[2])) { $pretty = ' prettyprint'; } $out = ''; diff --git a/src/IDF/FileUtil.php b/src/IDF/FileUtil.php new file mode 100644 index 0000000..725d980 --- /dev/null +++ b/src/IDF/FileUtil.php @@ -0,0 +1,165 @@ +' + .''; + $i++; + } + return Pluf_Template::markSafe(implode("\n", $table)); + } + + /** + * Find the mime type of a file. + * + * Use /etc/mime.types to find the type. + * + * @param string Filename/Filepath + * @param array Mime type found or 'application/octet-stream', basename, extension + */ + public static function getMimeType($file) + { + static $mimes = null; + if ($mimes == null) { + $mimes = array(); + $src = Pluf::f('idf_mimetypes_db', '/etc/mime.types'); + $filecontent = @file_get_contents($src); + if ($filecontent !== false) { + $mimes = preg_split("/\015\012|\015|\012/", $filecontent); + } + } + + $info = pathinfo($file); + if (isset($info['extension'])) { + foreach ($mimes as $mime) { + if ('#' != substr($mime, 0, 1)) { + $elts = preg_split('/ |\t/', $mime, -1, PREG_SPLIT_NO_EMPTY); + if (in_array($info['extension'], $elts)) { + return array($elts[0], $info['basename'], $info['extension']); + } + } + } + } else { + // we consider that if no extension and base name is all + // uppercase, then we have a text file. + if ($info['basename'] == strtoupper($info['basename'])) { + return array('text/plain', $info['basename'], 'txt'); + } + $info['extension'] = 'bin'; + } + return array('application/octet-stream', $info['basename'], $info['extension']); + } + + /** + * Find the mime type of a file using the fileinfo class. + * + * @param string Filename/Filepath + * @param string File content + * @return array Mime type found or 'application/octet-stream', basename, extension + */ + public static function getMimeTypeFromContent($file, $filedata) + { + $info = pathinfo($file); + $res = array('application/octet-stream', + $info['basename'], + isset($info['extension']) ? $info['extension'] : 'bin'); + if (function_exists('finfo_open')) { + $finfo = finfo_open(FILEINFO_MIME); + $mime = finfo_buffer($finfo, $filedata); + finfo_close($finfo); + if ($mime) { + $res[0] = $mime; + } + if (!isset($info['extension']) && $mime) { + $res[2] = (0 === strpos($mime, 'text/')) ? 'txt' : 'bin'; + } elseif (!isset($info['extension'])) { + $res[2] = 'bin'; + } + } + return $res; + } + + /** + * Find if a given mime type is a text file. + * This uses the output of the self::getMimeType function. + * + * @param array (Mime type, file name, extension) + * @return bool Is text + */ + public static function isText($fileinfo) + { + if (0 === strpos($fileinfo[0], 'text/')) { + return true; + } + $ext = 'mdtext php-dist h gitignore diff patch'; + $extra_ext = trim(Pluf::f('idf_extra_text_ext', '')); + if (!empty($extra_ext)) + $ext .= ' ' . $extra_ext; + $ext = array_merge(self::$supportedExtenstions, explode(' ' , $ext)); + return (in_array($fileinfo[2], $ext)); + } +} diff --git a/src/IDF/IssueFile.php b/src/IDF/IssueFile.php index 12b6375..9ecf559 100644 --- a/src/IDF/IssueFile.php +++ b/src/IDF/IssueFile.php @@ -39,9 +39,9 @@ class IDF_IssueFile extends Pluf_Model array( 'type' => 'Pluf_DB_Field_Sequence', //It is automatically added. - 'blank' => true, + 'blank' => true, ), - 'comment' => + 'comment' => array( 'type' => 'Pluf_DB_Field_Foreignkey', 'model' => 'IDF_IssueComment', @@ -49,7 +49,7 @@ class IDF_IssueFile extends Pluf_Model 'verbose' => __('comment'), 'relate_name' => 'attachment', ), - 'submitter' => + 'submitter' => array( 'type' => 'Pluf_DB_Field_Foreignkey', 'model' => 'Pluf_User', @@ -63,7 +63,7 @@ class IDF_IssueFile extends Pluf_Model 'size' => 100, 'verbose' => __('file name'), ), - 'attachment' => + 'attachment' => array( 'type' => 'Pluf_DB_Field_File', 'blank' => false, @@ -76,7 +76,7 @@ class IDF_IssueFile extends Pluf_Model 'verbose' => __('file size'), 'help_text' => 'Size in bytes.', ), - 'type' => + 'type' => array( 'type' => 'Pluf_DB_Field_Varchar', 'blank' => false, @@ -111,7 +111,7 @@ class IDF_IssueFile extends Pluf_Model $file = Pluf::f('upload_issue_path').'/'.$this->attachment; $this->filesize = filesize($file); // remove .dummy - $this->filename = substr(basename($file), 0, -6); + $this->filename = substr(basename($file), 0, -6); $img_extensions = array('jpeg', 'jpg', 'png', 'gif'); $info = pathinfo($this->filename); if (!isset($info['extension'])) $info['extension'] = ''; @@ -128,4 +128,10 @@ class IDF_IssueFile extends Pluf_Model { @unlink(Pluf::f('upload_issue_path').'/'.$this->attachment); } + + function isText() + { + $info = IDF_FileUtil::getMimeType($this->filename); + return IDF_FileUtil::isText($info); + } } diff --git a/src/IDF/Template/Markdown.php b/src/IDF/Template/Markdown.php index 3054ca8..50c9f6a 100644 --- a/src/IDF/Template/Markdown.php +++ b/src/IDF/Template/Markdown.php @@ -93,7 +93,7 @@ class IDF_Template_Markdown extends Pluf_Template_Tag $info = pathinfo($m[1]); $fileinfo = array($res->headers['Content-Type'], $m[1], isset($info['extension']) ? $info['extension'] : 'bin'); - if (!IDF_Views_Source::isText($fileinfo)) { + if (!IDF_FileUtil::isText($fileinfo)) { return $m[0]; } return $res->content; diff --git a/src/IDF/Tests/TestFileUtil.php b/src/IDF/Tests/TestFileUtil.php new file mode 100644 index 0000000..31d37e2 --- /dev/null +++ b/src/IDF/Tests/TestFileUtil.php @@ -0,0 +1,47 @@ + 'application/x-httpd-php', + 'whatever.pht' => 'application/x-httpd-php', + 'README' => 'text/plain', + ); + foreach ($files as $file => $mime) { + $m = IDF_Views_Source::getMimeType($file); + $this->assertEqual($mime, $m[0]); + } + } +} diff --git a/src/IDF/Tests/TestSource.php b/src/IDF/Tests/TestSource.php index fbe95da..8634cc3 100644 --- a/src/IDF/Tests/TestSource.php +++ b/src/IDF/Tests/TestSource.php @@ -32,19 +32,6 @@ class IDF_Tests_TestSource extends UnitTestCase parent::__construct('Test the source class.'); } - public function testGetMimeType() - { - $files = array( - 'whatever.php' => 'application/x-httpd-php', - 'whatever.pht' => 'application/x-httpd-php', - 'README' => 'text/plain', - ); - foreach ($files as $file => $mime) { - $m = IDF_Views_Source::getMimeType($file); - $this->assertEqual($mime, $m[0]); - } - } - public function testRegexCommit() { $regex = '#^/p/([\-\w]+)/source/tree/([^\/]+)/(.*)$#'; @@ -61,4 +48,4 @@ class IDF_Tests_TestSource extends UnitTestCase $this->assertEqual($res[2], $m[3]); } } -} \ No newline at end of file +} diff --git a/src/IDF/Views/Issue.php b/src/IDF/Views/Issue.php index 211e4b4..e0147fe 100644 --- a/src/IDF/Views/Issue.php +++ b/src/IDF/Views/Issue.php @@ -79,7 +79,7 @@ class IDF_Views_Issue } /** - * View the issues of a given user. + * View the issues of a given user. * * Only open issues are shown. */ @@ -201,7 +201,7 @@ class IDF_Views_Issue { $prj = $request->project; if (!isset($request->REQUEST['q']) or trim($request->REQUEST['q']) == '') { - $url = Pluf_HTTP_URL_urlForView('IDF_Views_Issue::index', + $url = Pluf_HTTP_URL_urlForView('IDF_Views_Issue::index', array($prj->shortname)); return new Pluf_HTTP_Response_Redirect($url); } @@ -263,7 +263,7 @@ class IDF_Views_Issue 'issue' => $issue, ); if ($request->method == 'POST') { - $form = new IDF_Form_IssueUpdate(array_merge($request->POST, + $form = new IDF_Form_IssueUpdate(array_merge($request->POST, $request->FILES), $params); if (!isset($request->POST['preview']) && $form->isValid()) { @@ -306,7 +306,7 @@ class IDF_Views_Issue $prj = $request->project; $attach = Pluf_Shortcuts_GetObjectOr404('IDF_IssueFile', $match[2]); $prj->inOr404($attach->get_comment()->get_issue()); - $info = IDF_Views_Source::getMimeType($attach->filename); + $info = IDF_FileUtil::getMimeType($attach->filename); $mime = 'application/octet-stream'; if (strpos($info[0], 'image/') === 0) { $mime = $info[0]; @@ -330,14 +330,14 @@ class IDF_Views_Issue $prj->inOr404($attach->get_comment()->get_issue()); // If one cannot see the attachement, redirect to the // getAttachment view. - $info = IDF_Views_Source::getMimeType($attach->filename); - if (!IDF_Views_Source::isText($info)) { + $info = IDF_FileUtil::getMimeType($attach->filename); + if (!IDF_FileUtil::isText($info)) { return $this->getAttachment($request, $match); } // Now we want to look at the file but with links back to the // issue. - $file = IDF_Views_Source::highLight($info, - file_get_contents(Pluf::f('upload_issue_path').'/'.$attach->attachment)); + $file = IDF_FileUtil::highLight($info, + file_get_contents(Pluf::f('upload_issue_path').'/'.$attach->attachment)); $title = sprintf(__('View %s'), $attach->filename); return Pluf_Shortcuts_RenderToResponse('idf/issues/attachment.html', array( @@ -406,7 +406,7 @@ class IDF_Views_Issue { $prj = $request->project; $tag = Pluf_Shortcuts_GetObjectOr404('IDF_Tag', $match[2]); - $status = $match[3]; + $status = $match[3]; if ($tag->project != $prj->id or !in_array($status, array('open', 'closed'))) { throw new Pluf_HTTP_Error404(); } @@ -414,7 +414,7 @@ class IDF_Views_Issue $title = sprintf(__('%1$s Issues with Label %2$s'), (string) $prj, (string) $tag); } else { - $title = sprintf(__('%1$s Closed Issues with Label %2$s'), + $title = sprintf(__('%1$s Closed Issues with Label %2$s'), (string) $prj, (string) $tag); } // Get stats about the open/closed issues having this tag. @@ -547,11 +547,11 @@ class IDF_Views_Issue */ function IDF_Views_Issue_SummaryAndLabels($field, $issue, $extra='') { - $edit = Pluf_HTTP_URL_urlForView('IDF_Views_Issue::view', + $edit = Pluf_HTTP_URL_urlForView('IDF_Views_Issue::view', array($issue->shortname, $issue->id)); $tags = array(); foreach ($issue->get_tags_list() as $tag) { - $url = Pluf_HTTP_URL_urlForView('IDF_Views_Issue::listLabel', + $url = Pluf_HTTP_URL_urlForView('IDF_Views_Issue::listLabel', array($issue->shortname, $tag->id, 'open')); $tags[] = sprintf('%s', $url, Pluf_esc((string) $tag)); } @@ -574,4 +574,4 @@ function IDF_Views_Issue_SummaryAndLabels($field, $issue, $extra='') function IDF_Views_Issue_ShowStatus($field, $issue, $extra='') { return Pluf_esc($issue->get_status()->name); -} \ No newline at end of file +} diff --git a/src/IDF/Views/Source.php b/src/IDF/Views/Source.php index 5817b22..ee2d680 100644 --- a/src/IDF/Views/Source.php +++ b/src/IDF/Views/Source.php @@ -31,17 +31,6 @@ Pluf::loadFunction('Pluf_Shortcuts_GetFormForModel'); */ class IDF_Views_Source { - /** - * Extension supported by the syntax highlighter. - */ - public static $supportedExtenstions = array( - 'ascx', 'ashx', 'asmx', 'aspx', 'browser', 'bsh', 'c', 'cl', 'cc', - 'config', 'cpp', 'cs', 'csh', 'csproj', 'css', 'cv', 'cyc', 'el', 'fs', - 'h', 'hh', 'hpp', 'hs', 'html', 'html', 'java', 'js', 'lisp', 'master', - 'pas', 'perl', 'php', 'pl', 'pm', 'py', 'rb', 'scm', 'sh', 'sitemap', - 'skin', 'sln', 'svc', 'vala', 'vb', 'vbproj', 'vbs', 'wsdl', 'xhtml', - 'xml', 'xsd', 'xsl', 'xslt'); - /** * Display help on how to checkout etc. */ @@ -211,7 +200,7 @@ class IDF_Views_Source if ($request_file_info->type != 'tree') { $info = self::getRequestedFileMimeType($request_file_info, $commit, $scm); - if (!self::isText($info)) { + if (!IDF_FileUtil::isText($info)) { $rep = new Pluf_HTTP_Response($scm->getFile($request_file_info), $info[0]); $rep->headers['Content-Disposition'] = 'attachment; filename="'.$info[1].'"'; @@ -373,7 +362,7 @@ class IDF_Views_Source $previous = substr($request_file, 0, -strlen($l.' ')); $scmConf = $request->conf->getVal('scm', 'git'); $props = $scm->getProperties($commit, $request_file); - $content = self::highLight($extra['mime'], $scm->getFile($request_file_info)); + $content = IDF_FileUtil::highLight($extra['mime'], $scm->getFile($request_file_info)); return Pluf_Shortcuts_RenderToResponse('idf/source/'.$scmConf.'/file.html', array( 'page_title' => $page_title, @@ -451,121 +440,12 @@ class IDF_Views_Source */ public static function getRequestedFileMimeType($file_info, $commit, $scm) { - $mime = self::getMimeType($file_info->file); + $mime = IDF_FileUtil::getMimeType($file_info->file); if ('application/octet-stream' != $mime[0]) { return $mime; } - return self::getMimeTypeFromContent($file_info->file, - $scm->getFile($file_info)); - } - - /** - * Find the mime type of a file using the fileinfo class. - * - * @param string Filename/Filepath - * @param string File content - * @return array Mime type found or 'application/octet-stream', basename, extension - */ - public static function getMimeTypeFromContent($file, $filedata) - { - $info = pathinfo($file); - $res = array('application/octet-stream', - $info['basename'], - isset($info['extension']) ? $info['extension'] : 'bin'); - if (function_exists('finfo_open')) { - $finfo = finfo_open(FILEINFO_MIME); - $mime = finfo_buffer($finfo, $filedata); - finfo_close($finfo); - if ($mime) { - $res[0] = $mime; - } - if (!isset($info['extension']) && $mime) { - $res[2] = (0 === strpos($mime, 'text/')) ? 'txt' : 'bin'; - } elseif (!isset($info['extension'])) { - $res[2] = 'bin'; - } - } - return $res; - } - - /** - * Find the mime type of a file. - * - * Use /etc/mime.types to find the type. - * - * @param string Filename/Filepath - * @param array Mime type found or 'application/octet-stream', basename, extension - */ - public static function getMimeType($file) - { - $src= Pluf::f('idf_mimetypes_db', '/etc/mime.types'); - $mimes = preg_split("/\015\012|\015|\012/", file_get_contents($src)); - $info = pathinfo($file); - if (isset($info['extension'])) { - foreach ($mimes as $mime) { - if ('#' != substr($mime, 0, 1)) { - $elts = preg_split('/ |\t/', $mime, -1, PREG_SPLIT_NO_EMPTY); - if (in_array($info['extension'], $elts)) { - return array($elts[0], $info['basename'], $info['extension']); - } - } - } - } else { - // we consider that if no extension and base name is all - // uppercase, then we have a text file. - if ($info['basename'] == strtoupper($info['basename'])) { - return array('text/plain', $info['basename'], 'txt'); - } - $info['extension'] = 'bin'; - } - return array('application/octet-stream', $info['basename'], $info['extension']); - } - - /** - * Find if a given mime type is a text file. - * This uses the output of the self::getMimeType function. - * - * @param array (Mime type, file name, extension) - * @return bool Is text - */ - public static function isText($fileinfo) - { - if (0 === strpos($fileinfo[0], 'text/')) { - return true; - } - $ext = 'mdtext php-dist h gitignore diff patch'; - $extra_ext = trim(Pluf::f('idf_extra_text_ext', '')); - if (!empty($extra_ext)) - $ext .= ' ' . $extra_ext; - $ext = array_merge(self::$supportedExtenstions, explode(' ' , $ext)); - return (in_array($fileinfo[2], $ext)); - } - - public static function highLight($fileinfo, $content) - { - $pretty = ''; - if (self::isSupportedExtension($fileinfo[2])) { - $pretty = ' prettyprint'; - } - $table = array(); - $i = 1; - foreach (preg_split("/\015\012|\015|\012/", $content) as $line) { - $table[] = '' - .''; - $i++; - } - return Pluf_Template::markSafe(implode("\n", $table)); - } - - /** - * Test if an extension is supported by the syntax highlighter. - * - * @param string The extension to test - * @return bool - */ - public static function isSupportedExtension($extension) - { - return in_array($extension, self::$supportedExtenstions); + return IDF_FileUtil::getMimeTypeFromContent($file_info->file, + $scm->getFile($file_info)); } /** diff --git a/src/IDF/templates/idf/issues/view.html b/src/IDF/templates/idf/issues/view.html index 191909a..7c0b16b 100644 --- a/src/IDF/templates/idf/issues/view.html +++ b/src/IDF/templates/idf/issues/view.html @@ -1,8 +1,8 @@ {extends "idf/issues/base.html"} {block titleicon}{if $form} {/if}{/block} {block body} -{assign $i = 0} -{assign $nc = $comments.count()} +{assign $i = 0} +{assign $nc = $comments.count()} {foreach $comments as $c}{ashowuser 'submitter', $c.get_submitter(), $request}{assign $who = $c.get_submitter()}
{if $i == 0} @@ -20,14 +20,13 @@ {if $attachments.count() > 0}
{/if} +{foreach $attachments as $a}
  • {$a.filename} - {$a.filesize|size}{if $a.isText()} - {trans 'view'}{/if}
    • {/foreach}{/if} {if $i> 0 and $c.changedIssue()} -
    +
    {foreach $c.changes as $w => $v} {if $w == 'su'}{trans 'Summary:'}{/if}{if $w == 'st'}{trans 'Status:'}{/if}{if $w == 'ow'}{trans 'Owner:'}{/if}{if $w == 'lb'}{trans 'Labels:'}{/if} {if $w == 'lb'}{assign $l = implode(', ', $v)}{$l}{else}{$v}{/if}
    {/foreach} -
    +
    {/if}
    {assign $i = $i + 1}{if $i == $nc and false == $form}
    @@ -119,7 +118,7 @@
    '.$i.''.IDF_Diff::padLine(Pluf_esc($line)).'
    '.$i.''.IDF_Diff::padLine(Pluf_esc($line)).'
      - | + | {trans 'Cancel'}