Search is broken if there are images in repo

[nimag42]

Hi.

If there's an image in the repository, any research will result in an error "Invalid bytes in UTF-8".

This is caused because the search tries to encode line of files in utf-8 before trying to match a regexp, but as image are binary files, they can not be encoded so it throws an error.

It's line 170 in git_layer_rugged.rb

[gerwitz]

bd2bd8e should be reverted.

[nimag42]

Well, I'm not sure, but if you revert this, won't you break research containing utf-8 stuff ?
For my use I just did the following :

@@ -167,7 +167,7 @@ module Gollum
          blob = @repo.lookup(entry[:oid])
          count = 0
          blob.content.each_line do |line|
-            next unless line.force_encoding("UTF-8").match(/#{Regexp.escape(query)}/i)
+           next unless line.force_encoding("UTF-8").scrub().match(/#{Regexp.escape(query)}/i)
            count += 1
          end
          path = options[:path] ? ::File.join(options[:path], root, entry[:name]) : "#{root}#{entry[:name]}"

I am not an expert in Ruby, and this is clearly an obvious hack since when someone search something, and if the image contain (when bytes have been converted in utf8) the text, it's showed in the results too. It's why I did'nt started a PR :)
Something more nicer would be to check if the file is binary or not before trying searching on it, but I don't know how to do so.

[gerwitz]

Your hack is slight improvement to the prior hack, and of course the real solution is to address encoding across the project.

But at the moment we have a release with a new, broken behavior compared to the prior release, which is preventing users from upgrading. Better to revert to the old behavior until someone can address the issue properly.

(I wish I was qualified to do so. My next goal is to learn implementing proper tests, so maybe I can contribute to the gollum projects in that way.)

[dometto]

Yes, I think we should unfortunately revert, and suffer the problem that UTF8 doesn't work for a little while longer. Also, we should see if we can implement a spec testing search functionality in the presense of binary files/images.

[dometto]

@bartkamphorst do you agree with the revert, or do you want to revert and come up with a different fix for unicode at the same time?

[jvstein]

This seems to have regressed in 066e98d

2019-10-13 20:48:36 - LocalJumpError - no block given (yield):
        /usr/local/bundle/gems/gollum-rugged_adapter-0.4.4/lib/rugged_adapter/git_layer_rugged.rb:178:in `block in grep'
        /usr/local/bundle/gems/rugged-0.28.3.1/lib/rugged/tree.rb:175:in `block in walk_blobs'
        /usr/local/bundle/gems/rugged-0.28.3.1/lib/rugged/tree.rb:175:in `walk'
        /usr/local/bundle/gems/rugged-0.28.3.1/lib/rugged/tree.rb:175:in `walk_blobs'
        /usr/local/bundle/gems/gollum-rugged_adapter-0.4.4/lib/rugged_adapter/git_layer_rugged.rb:174:in `grep'
        /usr/local/bundle/gems/gollum-lib-4.2.10/lib/gollum-lib/wiki.rb:645:in `search'

[dometto]

@jvstein are you sure it's failing on the presence images? The adapter should be skipping binary files. Also the error message LocalJumpError - no block given (yield) does not seem to point in that direction. Also, can you give us the output of gollum --versions?

[dometto]

Hmm, on second thought, it looks like it may not be properly skipping binary files (we would have to do a next if blob.binary? instead). @jvstein if you're positive it's a problem with images, it would be great if you could produce a regression test against this!

[jvstein]

@dometto It could be either in my repo. I've got both images and other binary files like pdfs. I tested against cc33a24 and verified the problem doesn't exist there.

Tests are here? https://github.com/gollum/adapter_specs

[jvstein]

Nope, I misdiagnosed. Not caused by binaries.

$ gollum --version
Gollum 4.1.4

[dometto]

@jvstein there's your problem -- you're using rugged adapter of master with gollum 4.x. See here for info on 5.x