[viewvc-dev] Re: [viewvc-users] Don't display binary files

Here's another patch (includes the previous one) that checks if the file is
binary based on its contents.  It considers the file binary if more than 30%
of its characters are non-text characters, per
http://code.activestate.com/recipes/173220/.

I'd prefer to not open and close the file the additional time to check the
threshold.  I had to though since vclib.svn.svn_repo.FileContentsPipe didn't
have anyway to reset to position 0 after reading the first 512 bytes.
Also, maybe we could make the threshold for non-text characters an option in
the conf file.

What do you think?

JJ
On Wed, Nov 12, 2008 at 9:05 AM, JJ <[email protected]> wrote:

> Here's a smaller patch against HEAD that just adds the ignore_svn_mimetype
> option.
>
> I think the next step is adding the correct logic to markup_or_annotate
> that detects if the file is viewable (meaning it is a viewable image or some
> other text file).  Since the file is already being opened at that time,
> perhaps using something like http://code.activestate.com/recipes/173220/ and
> setting a is_viewable variable would be best.  Then the template can check
> this value and display some appropriate message about the file not being
> viewable, similar to
> http://viewvc.tigris.org/source/browse/viewvc/trunk/viewvc.org/favicon.ico?rev=1946&view=markup
> .
>
>   On Tue, Nov 11, 2008 at 11:21 AM, JJ <[email protected]> wrote:
>
>>  On Tue, Nov 11, 2008 at 10:30 AM, C. Michael Pilato <[email protected]
>> > wrote:
>>
>>> JJ wrote:
>>> > My patch is attached to this email.  I made the following decisions,
>>> > which you can feel free to disagree with.
>>> >
>>> > 1) I do not set prefer_markup to true for any images.  There isn't much
>>> > value in showing the image in the markup view as opposed to the
>>> download
>>> > view.  In addition it causes problems because things such as diff,
>>> > annotate and download as text were enabled for image files.
>>>
>>> Well, there is *some* value, in that (as for text files) you can see the
>>> most recent change metadata associated with the image, and you also have
>>> handy links to other views of that thing (which can don't get with the
>>> download view).
>>>
>>
>>
>> You could make the case that ANY file should be viewable then, since any
>> file can have properties.
>>
>> In any event, the templates need some way of telling if the file is binary
>> or text (i.e. something other than prefer_markup, since prefer_markup could
>> then be set for text files or images) so they don't allow diffs or other
>> things that don't make sense for binary files.
>>
>>
>>>
>>> > The other
>>> > problem is that an image file may have prefer_markup set to true, but
>>> > then when displaying it in the markup view it looks like garbled text
>>> if
>>> > the svn:mime-type is set to something like application/octet-stream
>>> > (something which is quite common).
>>>
>>> Hrm.  I wonder if the change to make ViewVC honor the svn:mime-type
>>> property
>>> is going to cause widespread annoyance because so many people haven't
>>> taken
>>> the time to set that property correctly?  Those folks will, in ViewVC
>>> 1.1,
>>> lose the ability to see their files in the browser altogether.
>>>
>>> Do you think this behavior should be configurable?  ignore_svn_mimetype =
>>> 0?
>>>
>>
>>
>> Yes, I personally would like this option.  I didn't realize this behaviour
>> was new to 1.1.  If someone did enable it they should still be informed that
>> the guessed mime type will not take the property into account.
>>
>>
>>>
>>> > 2) I changed the default template to never link to the markup view if
>>> > prefer_markup is false.
>>> >
>>> > 3) I changed the default template to never link to things that apply
>>> > only to text files (diff, download as text, view) if prefer_markup is
>>> false.
>>> > It seems like the check for whether it is a text file needs to be
>>> > smarter now, to include more than just files with text/* mime types in
>>> > this category.  Opening the files would be a performance hit of course.
>>>
>>> ViewVC can't afford to be any slower. :-)
>>
>>
>> Agreed.
>>
>>
>>>
>>>
>>> One comment about your patch:  I had mentioned that get_file_view_info()
>>> would probably be handy as you built out the somewhat standard set of
>>> links
>>> for each file in view_revision(), but I see that you didn't take
>>> advantage
>>> of that function.  Why?  Did it not meet your needs in some way I didn't
>>> expect?
>>>
>> Probably because I don't know what I'm doing and wasn't listening very
>> well.  :-(
>>
>> Another thought on detecting if a mime type is for a text file...
>>
>> This is ugly, but for experimentation sake I expanded is_text as follows.
>>
>> def is_text(mime_type):
>>   rtn = not mime_type or mime_type[:5] == 'text/'
>>   if not rtn:
>>     from pygments.lexers import ClassNotFound, get_lexer_for_mimetype
>>     try:
>>       lexer = get_lexer_for_mimetype(mime_type)
>>       rtn = True
>>     except ClassNotFound:
>>       pass
>>   return rtn
>> This works pretty well, though it is specific to the use of Pygments.
>> Perhaps there is a more specific module in Python that can detect text files
>> based on the mime type?  It looked like the mimetypes module can't, but
>> perhaps I'm wrong.
>>
>> JJ
>>
>>
>>
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Index: viewvc.conf.dist
===================================================================
--- viewvc.conf.dist    (revision 2047)
+++ viewvc.conf.dist    (working copy)
@@ -269,6 +269,13 @@
 # Possible values: "tar", "annotate", "co", "markup", "roots"
 allowed_views = markup, annotate, roots

+# Don't use the svn:mime-type property to determine how to display a
+# file in the markup view.  This is helpful when, for example, an image
+# has svn:mime-type set to application/octet-stream.  If ViewVC tries
+# to display the image using the mime type application/octet-stream,
+# the image will look unrecognizeable.
+ignore_svn_mimetype = 0
+
 # authorizer: The name of the ViewVC authorizer plugin to use when
 # authorizing access to repository contents.  This value must be the
 # name of a Python module addressable as vcauth.MODULENAME (most
Index: templates/file.ezt
===================================================================
--- templates/file.ezt  (revision 2047)
+++ templates/file.ezt  (working copy)
@@ -102,5 +102,12 @@
 [end]
 [end]

+[if-any is_viewable]
+[else]
+<h3>
+<em>Non-viewable file contents hidden</em>
+</h3>
+[end]
+
 [include "include/props.ezt"]
 [include "include/footer.ezt"]
Index: lib/viewvc.py
===================================================================
--- lib/viewvc.py       (revision 2047)
+++ lib/viewvc.py       (working copy)
@@ -972,6 +972,24 @@
 def is_text(mime_type):
   return not mime_type or mime_type[:5] == 'text/'

+text_characters = "".join(map(chr, range(32, 127)) + list("\n\r\t\b"))
+_null_trans = string.maketrans("", "")
+
+def istext(s):
+  if "\0" in s:
+    return 0
+
+  if not s:  # Empty files are considered text
+    return 1
+
+  # Get the non-text characters (maps a character to itself then
+  # use the 'remove' option to get rid of the text characters.)
+  t = s.translate(_null_trans, text_characters)
+
+  # If more than 30% non-text characters, then
+  # this is considered a binary file
+  return float(len(t))/len(s) <= 0.30
+
 def is_cvsroot_path(roottype, path_parts):
   return roottype == 'cvs' and path_parts and path_parts[0] == 'CVSROOT'

@@ -1465,7 +1483,8 @@
   mime_type = None
   if not path_parts:
     return None
-  if request.roottype == 'svn':
+  if request.roottype == 'svn' and \
+     not request.cfg.options.ignore_svn_mimetype:
     try:
       itemprops = request.repos.itemprops(path_parts, rev)
       mime_type = itemprops.get('svn:mime-type')
@@ -1481,6 +1500,7 @@
   lines = fp = image_src_href = None
   annotation = None
   revision = None
+  is_viewable = None
   mime_type = calculate_mime_type(request, path, rev)

   # Is this a viewable image type?
@@ -1493,6 +1513,7 @@
     annotation = 'binary'
     image_src_href = request.get_url(view_func=view_checkout,
                                      params={'revision': rev}, escape=1)
+    is_viewable = True

   # Not a viewable image.
   else:
@@ -1513,10 +1534,18 @@
     if check_freshness(request, None, revision, weak=1):
       fp.close()
       return
-    lines = markup_stream_pygments(request, cfg, blame_source, fp,
-                                   path[-1], mime_type)
-    fp.close()

+    # Make sure its a text file.
+    if istext(fp.read(512)):
+      is_viewable = True
+      fp.close()
+      fp, revision = request.repos.openfile(path, rev)
+      lines = markup_stream_pygments(request, cfg, blame_source, fp,
+                                     path[-1], mime_type)
+      fp.close()
+    else:
+      is_viewable = False
+
   data = common_template_data(request, revision)
   data.update({
     'mime_type' : mime_type,
@@ -1538,6 +1567,7 @@
     'lines' : lines,
     'properties' : get_itemprops(request, path, rev),
     'annotation' : annotation,
+    'is_viewable' : ezt.boolean(is_viewable),
     })

   if cfg.options.show_log_in_markup: