FIX: Hide excerpt of binary files in GitHub onebox (#15639)

Oneboxer did not know if a file is binary or not and always tried to
show an excerpt of the file.

lib/onebox/mixins/git_blob_onebox.rb

@@ -169,6 +169,12 @@ module Onebox
             else
               contents = URI.parse(self.raw_template(m)).open(read_timeout: timeout).read
 
+              if contents.encoding == Encoding::BINARY
+                @raw = nil
+                @binary = true
+                return
+              end
+
               contents_lines = contents.lines           #get contents lines
               contents_lines_size = contents_lines.size #get number of lines
 
@@ -211,6 +217,7 @@ module Onebox
             #     as *side effects* of the `raw` method! They must all appear
             #     AFTER the call to `raw`! Don't get bitten by this like I did!
             content: raw,
+            binary: @binary,
             lang: "lang-#{@lang}",
             lines: @selected_lines_array ,
             has_lines: !@selected_lines_array.nil?,

lib/onebox/templates/githubblob.mustache

@@ -1,53 +1,59 @@
 <h4><a href="{{link}}" target="_blank" rel="noopener">{{title}}</a></h4>
 
-{{^has_lines}}
+{{^binary}}
-  {{#model_file}}
+  {{^has_lines}}
-    <iframe class="render-viewer" width="{{width}}" height="{{height}}" src="{{content}}" sandbox="allow-scripts allow-same-origin allow-top-navigation ">
+    {{#model_file}}
-      Viewer requires iframe.
+      <iframe class="render-viewer" width="{{width}}" height="{{height}}" src="{{content}}" sandbox="allow-scripts allow-same-origin allow-top-navigation ">
-    </iframe>
+        Viewer requires iframe.
-  {{/model_file}}
+      </iframe>
+    {{/model_file}}
 
-  {{^model_file}}
+    {{^model_file}}
-    <pre><code class="{{lang}}">{{content}}</code></pre>
+      <pre><code class="{{lang}}">{{content}}</code></pre>
-  {{/model_file}}
+    {{/model_file}}
-{{/has_lines}}
+  {{/has_lines}}
 
-{{#has_lines}}
+  {{#has_lines}}
-  {{! This is a template comment  | Sample rules for this box
+    {{! This is a template comment  | Sample rules for this box
-  <style>
+    <style>
-  pre.onebox code ol{
+    pre.onebox code ol{
-    margin-left:0px;
+      margin-left:0px;
-  }
+    }
-  pre.onebox code ol .lines{
+    pre.onebox code ol .lines{
-    margin-left:30px;
+      margin-left:30px;
-  }
+    }
-  pre.onebox code ol.lines li {
+    pre.onebox code ol.lines li {
-    list-style-type: decimal;
+      list-style-type: decimal;
-    margin-left:45px;
+      margin-left:45px;
-  }
+    }
-  pre.onebox code li{
+    pre.onebox code li{
-    list-style-type: none;
+      list-style-type: none;
-    background-color:#fff;
+      background-color:#fff;
-    border-bottom:1px solid #F0F0F0;
+      border-bottom:1px solid #F0F0F0;
-    padding-left:5px;
+      padding-left:5px;
 
-  }
+    }
-  pre.onebox code li.selected{
+    pre.onebox code li.selected{
-    background-color:#cfc
+      background-color:#cfc
-  }
+    }
-  </style>
+    </style>
-  }}
+    }}
 
-  <pre class="onebox">
+    <pre class="onebox">
-    <code class="{{lang}}">
+      <code class="{{lang}}">
-      <ol class="start lines" start="{{cr_results.from}}" style="counter-reset: li-counter {{cr_results.from_minus_one}} ;">
+        <ol class="start lines" start="{{cr_results.from}}" style="counter-reset: li-counter {{cr_results.from_minus_one}} ;">
-        {{#lines}}
+          {{#lines}}
-          <li{{#selected}} class="selected"{{/selected}}>{{data}}</li>
+            <li{{#selected}} class="selected"{{/selected}}>{{data}}</li>
-        {{/lines}}
+          {{/lines}}
-      </ol>
+        </ol>
-    </code>
+      </code>
-  </pre>
+    </pre>
-{{/has_lines}}
+  {{/has_lines}}
+{{/binary}}
+
+{{#binary}}
+  This file is binary. <a href="{{link}}" target="_blank" rel="noopener">show original</a>
+{{/binary}}
 
 {{#truncated}}
   This file has been truncated. <a href="{{link}}" target="_blank" rel="noopener">show original</a>

spec/lib/onebox/engine/github_blob_onebox_spec.rb

@@ -5,6 +5,7 @@ require "rails_helper"
 describe Onebox::Engine::GithubBlobOnebox do
   before do
     @link = "https://github.com/discourse/onebox/blob/master/lib/onebox/engine/github_blob_onebox.rb"
+    @uri = URI.parse(@link)
     stub_request(:get, "https://raw.githubusercontent.com/discourse/onebox/master/lib/onebox/engine/github_blob_onebox.rb")
       .to_return(status: 200, body: onebox_response(described_class.onebox_name))
   end
@@ -20,5 +21,16 @@ describe Onebox::Engine::GithubBlobOnebox do
     it "includes blob contents" do
       expect(html).to include("module Oneboxer")
     end
+
+    it "does not include blob contents if it is binary" do
+      # stub_request if the response must be binary (ASCII-8BIT)
+      uri = mock('object')
+      uri.stubs(:open).returns(File.open("#{Rails.root}/spec/fixtures/pdf/small.pdf", "rb"))
+      URI.stubs(:parse).with(@link).returns(@uri)
+      URI.stubs(:parse).with('https://raw.githubusercontent.com/discourse/onebox/master/lib/onebox/engine/github_blob_onebox.rb').returns(uri)
+
+      expect(html).not_to include('/Pages')
+      expect(html).to include('This file is binary.')
+    end
   end
 end