Merge pull request #19 from notalex/OTWO-3206

Peter Degen-Portnoy · Peter Degen-Portnoy · commit 3d8046dc5721 · 2014-10-08T08:01:01.000-04:00
OTWO-3206: Fix utf-8 encoding issues for svn and git
diff --git a/lib/scm/adapters/git/commits.rb b/lib/scm/adapters/git/commits.rb
@@ -50,7 +50,7 @@ def each_commit(opts={})
 
 		# Returns a single commit, including its diffs
 		def verbose_commit(token)
-			c = Scm::Parsers::GitStyledParser.parse(run("cd '#{url}' && #{Scm::Parsers::GitStyledParser.whatchanged} #{token}")).first
+			c = Scm::Parsers::GitStyledParser.parse(run("cd '#{url}' && #{Scm::Parsers::GitStyledParser.whatchanged} #{token} | #{ string_encoder }")).first
       fixup_null_merge(c)
 		end
 
diff --git a/lib/scm/adapters/svn/commits.rb b/lib/scm/adapters/svn/commits.rb
@@ -144,7 +144,7 @@ def verbose_commit(rev)
 
 		def log(opts={})
 			after = (opts[:after] || 0).to_i
-			run "svn log --trust-server-cert --non-interactive --xml --stop-on-copy -r #{after.to_i + 1}:#{final_token || 'HEAD'} '#{SvnAdapter.uri_encode(File.join(self.root, self.branch_name.to_s))}@#{final_token || 'HEAD'}' #{opt_auth}"
+			run "svn log --trust-server-cert --non-interactive --xml --stop-on-copy -r #{after.to_i + 1}:#{final_token || 'HEAD'} '#{SvnAdapter.uri_encode(File.join(self.root, self.branch_name.to_s))}@#{final_token || 'HEAD'}' #{opt_auth} | #{ string_encoder }"
 		end
 
 		def open_log_file(opts={})
@@ -154,7 +154,7 @@ def open_log_file(opts={})
 					# As a time optimization, just create an empty file rather than fetch a log we know will be empty.
 					File.open(log_filename, 'w') { |f| f.puts '<?xml version="1.0"?>' }
 				else
-					run "svn log --trust-server-cert --non-interactive --xml --stop-on-copy -r #{after + 1}:#{final_token || 'HEAD'} '#{SvnAdapter.uri_encode(File.join(self.root, self.branch_name))}@#{final_token || 'HEAD'}' #{opt_auth} > #{log_filename}"
+					run "svn log --trust-server-cert --non-interactive --xml --stop-on-copy -r #{after + 1}:#{final_token || 'HEAD'} '#{SvnAdapter.uri_encode(File.join(self.root, self.branch_name))}@#{final_token || 'HEAD'}' #{opt_auth} | #{ string_encoder } > #{log_filename}"
 				end
 				File.open(log_filename, 'r') { |io| yield io }
 			ensure
@@ -168,7 +168,7 @@ def log_filename
 
 		# Returns one commit with the exact revision number provided
 		def single_revision_xml(revision)
-			run "svn log --trust-server-cert --non-interactive --verbose --xml --stop-on-copy -r #{revision} --limit 1 #{opt_auth} '#{SvnAdapter.uri_encode(File.join(self.root, self.branch_name))}@#{revision}'"
+			run "svn log --trust-server-cert --non-interactive --verbose --xml --stop-on-copy -r #{revision} --limit 1 #{opt_auth} '#{SvnAdapter.uri_encode(File.join(self.root, self.branch_name))}@#{revision}' | #{ string_encoder }"
 		end
 
 		# Recurses the entire repository and returns an array of file names.
diff --git a/lib/scm/adapters/svn/head.rb b/lib/scm/adapters/svn/head.rb
@@ -11,7 +11,7 @@ def head
 		def parents(commit)
 			# Subversion doesn't have an actual "parent" command, so get
 			# a log for this commit and the one preceding it, and keep only the preceding.
-			log = run "svn log --trust-server-cert --non-interactive --verbose --xml --stop-on-copy -r #{commit.token}:1 --limit 2 '#{SvnAdapter.uri_encode(self.url)}' #{opt_auth}"
+			log = run "svn log --trust-server-cert --non-interactive --verbose --xml --stop-on-copy -r #{commit.token}:1 --limit 2 '#{SvnAdapter.uri_encode(self.url)}' #{opt_auth} | #{ string_encoder }"
 			[deepen_commit(strip_commit_branch(Scm::Parsers::SvnXmlParser.parse(log).last))]
 		end
 	end
diff --git a/lib/scm/adapters/svn_chain/chain.rb b/lib/scm/adapters/svn_chain/chain.rb
@@ -79,7 +79,7 @@ def first_commit(after=0)
 		# Returns the first commit with a revision number greater than the provided revision number
 		def next_revision_xml(after=0)
 			return "<?xml?>" if after.to_i >= head_token
-			run "svn log --trust-server-cert --non-interactive --verbose --xml --stop-on-copy -r #{after.to_i+1}:#{final_token || 'HEAD'} --limit 1 #{opt_auth} '#{SvnAdapter.uri_encode(File.join(self.root, self.branch_name))}@#{final_token || 'HEAD'}'"
+			run "svn log --trust-server-cert --non-interactive --verbose --xml --stop-on-copy -r #{after.to_i+1}:#{final_token || 'HEAD'} --limit 1 #{opt_auth} '#{SvnAdapter.uri_encode(File.join(self.root, self.branch_name))}@#{final_token || 'HEAD'}' | #{ string_encoder }"
 		end
 
 		# If the passed diff represents the wholesale movement of the entire
diff --git a/test/bin/svn b/test/bin/svn
@@ -0,0 +1,7 @@
+#! /usr/bin/env bash
+# This is for use with tests dealing with invalid encodings in svn log.
+# This script returns a sample svn xml log with some non utf-8 characters.
+
+DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+
+cat $DIR/../data/svn_with_invalid_encoding.log
diff --git a/test/data/svn_with_invalid_encoding.log b/test/data/svn_with_invalid_encoding.log
@@ -0,0 +1,33 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<log>
+<logentry
+   revision="4">
+<author>cartman</author>
+<date>2000-08-29T18:45:43.000000Z</date>
+<msg>Ample parking day or night, people spouting: "Howdy, neighbor!"
+</msg>
+</logentry>
+<logentry
+   revision="3">
+<author>primus</author>
+<date>2002-07-11T14:27:53.000000Z</date>
+<msg>I'm goin' down to South Park, gonna leave my woes behind.
+
+Thanks to Kenny M�ller
+</msg>
+</logentry>
+<logentry
+   revision="2">
+<author>stan</author>
+<date>2002-07-11T18:19:49.000000Z</date>
+<msg>Friendly faces everywhere, humble folks without temptation
+</msg>
+</logentry>
+<logentry
+   revision="1">
+<author>primus</author>
+<date>2002-07-12T08:14:23.000000Z</date>
+<msg>I'm goin' down to South Park, gonna have myself a time.
+</msg>
+</logentry>
+</log>
diff --git a/test/repositories/git_with_invalid_encoding.tgz b/test/repositories/git_with_invalid_encoding.tgz
diff --git a/test/test_helper.rb b/test/test_helper.rb
@@ -60,6 +60,22 @@ def with_repository(type, name)
 		end
 	end
 
+  # We are unable to add a commit message with non utf8 characters using svn 1.6 & above.
+  # In order to emulate encoding issues, we use a custom svn executable that returns
+  #   an xml log with invalid characters in it.
+  # We prepend our custom svn's location to $PATH to make it available during our tests.
+  def with_invalid_encoded_svn_repository
+    with_repository(Scm::Adapters::SvnChainAdapter, 'svn_with_invalid_encoding') do |svn|
+      original_env_path = ENV['PATH']
+      custom_svn_path = File.expand_path('../bin/', __FILE__)
+      ENV['PATH'] = custom_svn_path + ':' + ENV['PATH']
+
+      yield svn
+
+      ENV['PATH'] = original_env_path
+    end
+  end
+
 	def with_svn_repository(name, branch_name='')
 		with_repository(Scm::Adapters::SvnAdapter, name) do |svn|
 			svn.branch_name = branch_name
diff --git a/test/unit/git_commits_test.rb b/test/unit/git_commits_test.rb
@@ -133,6 +133,13 @@ def test_log_encoding
       end
     end
 
+    def test_verbose_commits_valid_encoding
+      with_git_repository('git_with_invalid_encoding') do |git|
+        assert_equal true,
+          git.verbose_commit('8d03f4ea64fcd10966fb3773a212b141ada619e1').message.valid_encoding?
+      end
+    end
+
     def test_open_log_file_encoding
       with_git_repository('git_with_invalid_encoding') do |git|
         git.open_log_file do |io|
diff --git a/test/unit/git_head_test.rb b/test/unit/git_head_test.rb
@@ -15,5 +15,12 @@ def test_head_and_parents
 			end
 		end
 
+    def test_head_token
+      with_git_repository('git_with_invalid_encoding') do |git|
+        assert_nothing_raised do
+          git.head_token
+        end
+      end
+    end
 	end
 end
diff --git a/test/unit/git_token_test.rb b/test/unit/git_token_test.rb
@@ -33,5 +33,13 @@ def test_commit_all_includes_write_token
 				assert_equal c.token, git.read_token
 			end
 		end
+
+    def test_read_token_encoding
+      with_git_repository('git_with_invalid_encoding') do |git|
+        assert_nothing_raised do
+          git.read_token
+        end
+      end
+    end
 	end
 end
diff --git a/test/unit/svn_chain_test.rb b/test/unit/svn_chain_test.rb
@@ -58,5 +58,10 @@ def test_parent_branch_name
 					:path => "/trunk", :from_revision => 1, :from_path => "/branches/b"))
 		end
 
+    def test_next_revision_xml_valid_encoding
+      with_invalid_encoded_svn_repository do |svn|
+        assert_equal true, svn.next_revision_xml(0).valid_encoding?
+      end
+    end
 	end
 end
diff --git a/test/unit/svn_commits_test.rb b/test/unit/svn_commits_test.rb
@@ -244,15 +244,32 @@ def test_each_commit
 			assert_equal '/trunk/COPYING', commits[4].diffs[1].path
 		end
 
-    # Invalid utf-8 characters in commit message or filename cause encoding errors.
-    # Svn converts invalid utf-8 characters into valid format before commit.
-    # So no utf-8 encoding issues are seen in ruby when dealing with Svn.
+    def test_log_valid_encoding
+      with_invalid_encoded_svn_repository do |svn|
+        assert_equal true, svn.log.valid_encoding?
+      end
+    end
+
     def test_commits_encoding
-      with_svn_repository('svn_with_invalid_encoding') do |svn|
+      with_invalid_encoded_svn_repository do |svn|
         assert_nothing_raised do
-          svn.commits
+          svn.commits rescue raise Exception
+        end
+      end
+    end
+
+    def test_open_log_file_encoding
+      with_invalid_encoded_svn_repository do |svn|
+        svn.open_log_file do |io|
+          assert_equal true, io.read.valid_encoding?
         end
       end
     end
+
+    def test_single_revision_xml_valid_encoding
+      with_invalid_encoded_svn_repository do |svn|
+        assert_equal true, svn.single_revision_xml(:anything).valid_encoding?
+      end
+    end
 	end
 end
diff --git a/test/unit/svn_head_test.rb b/test/unit/svn_head_test.rb
@@ -14,6 +14,14 @@ def test_head_and_parents
 			end
 		end
 
+    def test_parents_encoding
+      with_invalid_encoded_svn_repository do |svn|
+        assert_nothing_raised do
+          commit = Struct.new(:token).new(:anything)
+          svn.parents(commit) rescue raise Exception
+        end
+      end
+    end
 	end
 end
 
