Re-fix image rendering for help pages

4 files changed, 99 insertions, 8 deletions

diff --git a/app/controllers/help_controller.rb b/app/controllers/help_controller.rb
index 964f624d6d7..10094d86dfb 100644
--- a/app/controllers/help_controller.rb
+++ b/app/controllers/help_controller.rb
@@ -3,13 +3,36 @@ class HelpController < ApplicationController
   end
 
   def show
-    @category = clean_path_info(params[:category])
-    @file = clean_path_info(params[:file])
+    category = clean_path_info(path_params[:category])
+    file = clean_path_info(path_params[:file])
 
-    if File.exists?(Rails.root.join('doc', @category, @file + '.md'))
-      render 'show'
-    else
-      not_found!
+    respond_to do |format|
+      format.any(:markdown, :md, :html) do
+        path = Rails.root.join('doc', category, "#{file}.md")
+
+        if File.exist?(path)
+          @markdown = File.read(path)
+
+          render 'show.html.haml'
+        else
+          # Force template to Haml
+          render 'errors/not_found.html.haml', layout: 'errors', status: 404
+        end
+      end
+
+      # Allow access to images in the doc folder
+      format.any(:png, :gif, :jpeg) do
+        path = Rails.root.join('doc', category, "#{file}.#{params[:format]}")
+
+        if File.exist?(path)
+          send_file(path, disposition: 'inline')
+        else
+          head :not_found
+        end
+      end
+
+      # Any other format we don't recognize, just respond 404
+      format.any { head :not_found }
     end
   end
 
@@ -21,6 +44,13 @@ class HelpController < ApplicationController
 
   private
 
+  def path_params
+    params.require(:category)
+    params.require(:file)
+
+    params
+  end
+
   PATH_SEPS = Regexp.union(*[::File::SEPARATOR, ::File::ALT_SEPARATOR].compact)
 
   # Taken from ActionDispatch::FileHandler
diff --git a/app/views/help/show.html.haml b/app/views/help/show.html.haml
index eca34dbff06..cc1be6a717a 100644
--- a/app/views/help/show.html.haml
+++ b/app/views/help/show.html.haml
@@ -1,2 +1,2 @@
 .documentation.wiki
-  = markdown File.read(Rails.root.join('doc', @category, @file + '.md')).gsub("$your_email", current_user.email)
+  = markdown @markdown.gsub('$your_email', current_user.email)
diff --git a/spec/controllers/help_controller_spec.rb b/spec/controllers/help_controller_spec.rb
new file mode 100644
index 00000000000..93535ced7ae
--- /dev/null
+++ b/spec/controllers/help_controller_spec.rb
@@ -0,0 +1,61 @@
+require 'spec_helper'
+
+describe HelpController do
+  let(:user) { create(:user) }
+
+  before do
+    sign_in(user)
+  end
+
+  describe 'GET #show' do
+    context 'for Markdown formats' do
+      context 'when requested file exists' do
+        before do
+          get :show, category: 'ssh', file: 'README', format: :md
+        end
+
+        it 'assigns to @markdown' do
+          expect(assigns[:markdown]).not_to be_empty
+        end
+
+        it 'renders HTML' do
+          expect(response).to render_template('show.html.haml')
+          expect(response.content_type).to eq 'text/html'
+        end
+      end
+
+      context 'when requested file is missing' do
+        it 'renders not found' do
+          get :show, category: 'foo', file: 'bar', format: :md
+          expect(response).to be_not_found
+        end
+      end
+    end
+
+    context 'for image formats' do
+      context 'when requested file exists' do
+        it 'renders the raw file' do
+          get :show, category: 'workflow/protected_branches',
+            file: 'protected_branches1', format: :png
+          expect(response).to be_success
+          expect(response.content_type).to eq 'image/png'
+          expect(response.headers['Content-Disposition']).to match(/^inline;/)
+        end
+      end
+
+      context 'when requested file is missing' do
+        it 'renders not found' do
+          get :show, category: 'foo', file: 'bar', format: :png
+          expect(response).to be_not_found
+        end
+      end
+    end
+
+    context 'for other formats' do
+      it 'always renders not found' do
+        get :show, category: 'ssh', file: 'README', format: :foo
+        expect(response).to be_not_found
+      end
+    end
+  end
+end
diff --git a/spec/features/help_pages_spec.rb b/spec/features/help_pages_spec.rb
index 41088ce8271..8c6b669ce78 100644
--- a/spec/features/help_pages_spec.rb
+++ b/spec/features/help_pages_spec.rb
@@ -6,7 +6,7 @@ describe 'Help Pages', feature: true do
       login_as :user
     end
     it 'replace the variable $your_email with the email of the user' do
-      visit help_page_path(category: 'ssh', file: 'README.md')
+      visit help_page_path('ssh', 'README')
       expect(page).to have_content("ssh-keygen -t rsa -C \"#{@user.email}\"")
     end
   end