From eb5749ed392aa375ee6c50e9f6c9a5aabc11820b Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Sat, 26 May 2012 15:20:35 -0300 Subject: [PATCH 1/5] Fixed encoding problems with plain/text blobs being sent without charset. --- app/controllers/refs_controller.rb | 10 +++++++++- lib/gitlabhq/encode.rb | 13 ++++++++++--- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/app/controllers/refs_controller.rb b/app/controllers/refs_controller.rb index e4e5b4edd4..60d6946619 100644 --- a/app/controllers/refs_controller.rb +++ b/app/controllers/refs_controller.rb @@ -1,4 +1,5 @@ class RefsController < ApplicationController + include Gitlabhq::Encode before_filter :project # Authorize @@ -49,9 +50,16 @@ class RefsController < ApplicationController def blob if @tree.is_blob? + if @tree.text? + encoding = detect_encoding(@tree.data) + mime_type = encoding ? "text/plain; charset=#{encoding}" : "text/plain" + else + mime_type = @tree.mime_type + end + send_data( @tree.data, - :type => @tree.text? ? "text/plain" : @tree.mime_type, + :type => mime_type, :disposition => 'inline', :filename => @tree.name ) diff --git a/lib/gitlabhq/encode.rb b/lib/gitlabhq/encode.rb index df40206d5e..6069f4f90a 100644 --- a/lib/gitlabhq/encode.rb +++ b/lib/gitlabhq/encode.rb @@ -5,9 +5,9 @@ module Gitlabhq def utf8 message return nil unless message - hash = CharlockHolmes::EncodingDetector.detect(message) rescue {} - if hash[:encoding] - CharlockHolmes::Converter.convert(message, hash[:encoding], 'UTF-8') + encoding = detect_encoding(message) + if encoding + CharlockHolmes::Converter.convert(message, encoding, 'UTF-8') else message end.force_encoding("utf-8") @@ -16,5 +16,12 @@ module Gitlabhq rescue "" end + + def detect_encoding message + return nil unless message + + hash = CharlockHolmes::EncodingDetector.detect(message) rescue {} + return hash[:encoding] ? hash[:encoding] : nil + end end end From 39def0dcbb17a3f5883d6cb1965f815b537e5890 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Sat, 26 May 2012 15:22:47 -0300 Subject: [PATCH 2/5] Better fix for encoding problems on rendering of inline file visualizations like README files. --- app/views/refs/_tree.html.haml | 4 ++-- app/views/refs/_tree_file.html.haml | 2 +- lib/gitlabhq/encode.rb | 3 +++ 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/app/views/refs/_tree.html.haml b/app/views/refs/_tree.html.haml index 137501eaf1..0d9d2d7541 100644 --- a/app/views/refs/_tree.html.haml +++ b/app/views/refs/_tree.html.haml @@ -42,9 +42,9 @@ .readme - if content.name =~ /\.(md|markdown)$/i = preserve do - = markdown(content.data.force_encoding('UTF-8')) + = markdown(content.data.detect_encoding!) - else - = simple_format(content.data.force_encoding('UTF-8')) + = simple_format(content.data.detect_encoding!) - if params[:path] - history_path = tree_file_project_ref_path(@project, @ref, params[:path]) diff --git a/app/views/refs/_tree_file.html.haml b/app/views/refs/_tree_file.html.haml index 3fcc37331e..608faaa83c 100644 --- a/app/views/refs/_tree_file.html.haml +++ b/app/views/refs/_tree_file.html.haml @@ -13,7 +13,7 @@ #tree-readme-holder .readme = preserve do - = markdown(file.data.force_encoding('UTF-8')) + = markdown(file.data.detect_encoding!) - else .view_file_content - unless file.empty? diff --git a/lib/gitlabhq/encode.rb b/lib/gitlabhq/encode.rb index 6069f4f90a..e0e52f0a2a 100644 --- a/lib/gitlabhq/encode.rb +++ b/lib/gitlabhq/encode.rb @@ -1,3 +1,6 @@ +# Patch Strings to enable detect_encoding! on views +require 'charlock_holmes/string' + module Gitlabhq module Encode extend self From 48a36851e60249565e0869f88a05b36252c7e893 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Sat, 26 May 2012 15:27:29 -0300 Subject: [PATCH 3/5] It's dangerous to rescue errors here as it will hide bugs. define_tree_vars already catch all situations where something may not exist. --- app/controllers/refs_controller.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/controllers/refs_controller.rb b/app/controllers/refs_controller.rb index 60d6946619..2de40b4e1d 100644 --- a/app/controllers/refs_controller.rb +++ b/app/controllers/refs_controller.rb @@ -44,8 +44,6 @@ class RefsController < ApplicationController no_cache_headers end end - rescue - return render_404 end def blob @@ -66,8 +64,6 @@ class RefsController < ApplicationController else head(404) end - rescue - return render_404 end def blame From 50c2c16a4d8ca52c4abcbef638f5105a9b0d1ee0 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Sat, 26 May 2012 20:15:06 -0300 Subject: [PATCH 4/5] Better algorithm to deal with encodings. Moved fallback rescue message from view to encode library. This helps fix cases where UTF-8 is wrongly identified as ISO-8859-1. We will only try to convert strings if we are 100% sure about the charset, otherwise, we will fallback to UTF-8. --- app/views/commits/_commit.html.haml | 2 +- lib/gitlabhq/encode.rb | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/app/views/commits/_commit.html.haml b/app/views/commits/_commit.html.haml index a579cca96b..f52dbcfe72 100644 --- a/app/views/commits/_commit.html.haml +++ b/app/views/commits/_commit.html.haml @@ -8,7 +8,7 @@ %strong.cgray= commit.author_name – = image_tag gravatar_icon(commit.author_email), :class => "avatar", :width => 16 - %span.row_title= truncate(commit.safe_message, :length => 50) rescue "--broken encoding" + %span.row_title= truncate(commit.safe_message, :length => 50) %span.right.cgray = time_ago_in_words(commit.committed_date) diff --git a/lib/gitlabhq/encode.rb b/lib/gitlabhq/encode.rb index e0e52f0a2a..780d839f42 100644 --- a/lib/gitlabhq/encode.rb +++ b/lib/gitlabhq/encode.rb @@ -8,16 +8,19 @@ module Gitlabhq def utf8 message return nil unless message - encoding = detect_encoding(message) - if encoding + detect = CharlockHolmes::EncodingDetector.detect(message) rescue {} + + # It's better to default to UTF-8 as sometimes it's wrongly detected as another charset + if detect[:encoding] && detect[:confidence] == 100 CharlockHolmes::Converter.convert(message, encoding, 'UTF-8') else message end.force_encoding("utf-8") + # Prevent app from crash cause of # encoding errors rescue - "" + "--broken encoding: #{encoding}" end def detect_encoding message From 11f90ae42eeca1c9bcc415d046663efef0941bf0 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Sat, 26 May 2012 23:19:27 -0300 Subject: [PATCH 5/5] Forgot to refactor a line on lib/gitlabhq/encode.rb --- lib/gitlabhq/encode.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gitlabhq/encode.rb b/lib/gitlabhq/encode.rb index 780d839f42..1a81accea6 100644 --- a/lib/gitlabhq/encode.rb +++ b/lib/gitlabhq/encode.rb @@ -12,7 +12,7 @@ module Gitlabhq # It's better to default to UTF-8 as sometimes it's wrongly detected as another charset if detect[:encoding] && detect[:confidence] == 100 - CharlockHolmes::Converter.convert(message, encoding, 'UTF-8') + CharlockHolmes::Converter.convert(message, detect[:encoding], 'UTF-8') else message end.force_encoding("utf-8")