Merge pull request #407 from mfazekas/utf8

Utf8

11 files changed, 85 insertions, 10 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index e71cf9a..a88a745 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,3 +1,5 @@
+* Fix UTF-8 encoding issues [Ethan J. Brown]
+
 === 4.0.0.alpha4
 
 * Experimental event loop abstraction [Miklos Fazekas]
diff --git a/lib/net/ssh/authentication/ed25519.rb b/lib/net/ssh/authentication/ed25519.rb
index 021c2ad..ec6b56c 100644
--- a/lib/net/ssh/authentication/ed25519.rb
+++ b/lib/net/ssh/authentication/ed25519.rb
@@ -33,7 +33,7 @@ module ED25519
     end
 
     def to_blob
-      Net::SSH::Buffer.from(:string,"ssh-ed25519",:string,@verify_key.to_bytes).to_s
+      Net::SSH::Buffer.from(:mstring,"ssh-ed25519",:string,@verify_key.to_bytes).to_s
     end
 
     def ssh_type
diff --git a/lib/net/ssh/authentication/key_manager.rb b/lib/net/ssh/authentication/key_manager.rb
index b5ab8c4..9899a70 100644
--- a/lib/net/ssh/authentication/key_manager.rb
+++ b/lib/net/ssh/authentication/key_manager.rb
@@ -147,7 +147,7 @@ module Net
 
           if info[:key]
             return Net::SSH::Buffer.from(:string, identity.ssh_type,
-              :string, info[:key].ssh_do_sign(data.to_s)).to_s
+              :mstring, info[:key].ssh_do_sign(data.to_s)).to_s
           end
 
           if info[:from] == :agent
diff --git a/lib/net/ssh/buffer.rb b/lib/net/ssh/buffer.rb
index e1ee886..93e54e1 100644
--- a/lib/net/ssh/buffer.rb
+++ b/lib/net/ssh/buffer.rb
@@ -36,6 +36,7 @@ module Net; module SSH
     # * :long => write a 4-byte integer (#write_long)
     # * :byte => write a single byte (#write_byte)
     # * :string => write a 4-byte length followed by character data (#write_string)
+    # * :mstring => same as string, but caller cannot resuse the string, avoids potential duplication (#write_moved)
     # * :bool => write a single byte, interpreted as a boolean (#write_bool)
     # * :bignum => write an SSH-encoded bignum (#write_bignum)
     # * :key => write an SSH-encoded key value (#write_key)
@@ -184,7 +185,7 @@ module Net; module SSH
       consume!
       data
     end
-      
+
     # Return the next 8 bytes as a 64-bit integer (in network byte order).
     # Returns nil if there are less than 8 bytes remaining to be read in the
     # buffer.
@@ -286,7 +287,14 @@ module Net; module SSH
     # Writes the given data literally into the string. Does not alter the
     # read position. Returns the buffer object.
     def write(*data)
-      data.each { |datum| @content << datum }
+      data.each { |datum| @content << datum.dup.force_encoding('BINARY') }
+      self
+    end
+
+    # Optimized version of write where the caller gives up ownership of string
+    # to the method. This way we can mutate the string.
+    def write_moved(string)
+      @content << string.force_encoding('BINARY')
       self
     end
 
@@ -329,6 +337,19 @@ module Net; module SSH
       self
     end
 
+    # Writes each argument to the buffer as an SSH2-encoded string. Each
+    # string is prefixed by its length, encoded as a 4-byte long integer.
+    # Does not alter the read position. Returns the buffer object.
+    # Might alter arguments see write_moved
+    def write_mstring(*text)
+      text.each do |string|
+        s = string.to_s
+        write_long(s.bytesize)
+        write_moved(s)
+      end
+      self
+    end
+
     # Writes each argument to the buffer as a (C-style) boolean, with 1
     # meaning true, and 0 meaning false. Does not alter the read position.
     # Returns the buffer object.
diff --git a/lib/net/ssh/connection/session.rb b/lib/net/ssh/connection/session.rb
index 4f1bf7c..a5f9815 100644
--- a/lib/net/ssh/connection/session.rb
+++ b/lib/net/ssh/connection/session.rb
@@ -390,6 +390,7 @@ module Net; module SSH; module Connection
       channel.wait
 
       channel[:result] ||= "" unless block
+      channel[:result] &&= channel[:result].force_encoding("UTF-8") unless block
 
       return channel[:result]
     end
diff --git a/lib/net/ssh/transport/algorithms.rb b/lib/net/ssh/transport/algorithms.rb
index 5443c0d..604c891 100644
--- a/lib/net/ssh/transport/algorithms.rb
+++ b/lib/net/ssh/transport/algorithms.rb
@@ -287,8 +287,8 @@ module Net; module SSH; module Transport
 
         Net::SSH::Buffer.from(:byte, KEXINIT,
           :long, [rand(0xFFFFFFFF), rand(0xFFFFFFFF), rand(0xFFFFFFFF), rand(0xFFFFFFFF)],
-          :string, [kex, host_key, encryption, encryption, hmac, hmac],
-          :string, [compression, compression, language, language],
+          :mstring, [kex, host_key, encryption, encryption, hmac, hmac],
+          :mstring, [compression, compression, language, language],
           :bool, false, :long, 0)
       end
 
diff --git a/lib/net/ssh/transport/kex/ecdh_sha2_nistp256.rb b/lib/net/ssh/transport/kex/ecdh_sha2_nistp256.rb
index f860016..953b2da 100644
--- a/lib/net/ssh/transport/kex/ecdh_sha2_nistp256.rb
+++ b/lib/net/ssh/transport/kex/ecdh_sha2_nistp256.rb
@@ -57,7 +57,7 @@ module Net; module SSH; module Transport; module Kex
       # send the KEXECDH_INIT message
       ## byte     SSH_MSG_KEX_ECDH_INIT
       ## string   Q_C, client's ephemeral public key octet string
-      buffer = Net::SSH::Buffer.from(:byte, init, :string, ecdh.public_key.to_bn.to_s(2))
+      buffer = Net::SSH::Buffer.from(:byte, init, :mstring, ecdh.public_key.to_bn.to_s(2))
       connection.send_message(buffer)
 
       # expect the following KEXECDH_REPLY message
diff --git a/lib/net/ssh/transport/openssl.rb b/lib/net/ssh/transport/openssl.rb
index dc2fdb0..2c6ae0c 100644
--- a/lib/net/ssh/transport/openssl.rb
+++ b/lib/net/ssh/transport/openssl.rb
@@ -185,7 +185,7 @@ module OpenSSL
         def to_blob
           @blob ||= Net::SSH::Buffer.from(:string, ssh_type,
                                           :string, CurveNameAliasInv[self.group.curve_name],
-                                          :string, self.public_key.to_bn.to_s(2)).to_s
+                                          :mstring, self.public_key.to_bn.to_s(2)).to_s
           @blob
         end
 
diff --git a/net-ssh.gemspec b/net-ssh.gemspec
index d6b163f..5e621bb 100644
--- a/net-ssh.gemspec
+++ b/net-ssh.gemspec
@@ -34,7 +34,7 @@ Gem::Specification.new do |spec|
     spec.add_development_dependency("bcrypt_pbkdf", "~> 1.0.0.alpha1") unless RUBY_PLATFORM == "java"
   end
 
-  spec.add_development_dependency "bundler", "~> 1.11.2"
+  spec.add_development_dependency "bundler", "~> 1.11"
   spec.add_development_dependency "rake", "~> 11.1"
   spec.add_development_dependency "minitest", "~> 5.0"
   spec.add_development_dependency "rubocop", "~> 0.39.0"
diff --git a/test/integration/test_encoding.rb b/test/integration/test_encoding.rb
new file mode 100644
index 0000000..febdfba
--- /dev/null
+++ b/test/integration/test_encoding.rb
@@ -0,0 +1,23 @@
+require_relative 'common'
+require 'fileutils'
+require 'tmpdir'
+
+require 'net/ssh'
+
+class TestEncoding < NetSSHTest
+  def test_unicode_character
+    ret = Net::SSH.start("localhost", "net_ssh_1", password: 'foopwd') do |ssh|
+      ssh.exec! "echo \"hello from:$USER\" \u2603"
+    end
+    assert_equal ret, "hello from:net_ssh_1 \u2603\n"
+  end
+
+  def test_long_command_with_unicode_in_it
+    string = "eeeeeeeeeeeeeeeeeeeeeeeeeewwowowowìeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee"
+    command = "echo \"#{string}\""
+    ret = Net::SSH.start("localhost", "net_ssh_1", password: 'foopwd') do |ssh|
+      ssh.exec! command
+    end
+    assert_equal ret, "#{string}\n"
+  end
+end
\ No newline at end of file
diff --git a/test/test_buffer.rb b/test/test_buffer.rb
index 8ac4718..399660e 100644
--- a/test/test_buffer.rb
+++ b/test/test_buffer.rb
@@ -26,6 +26,34 @@ class TestBuffer < NetSSHTest
     assert_equal "\1\0\0\0\2\0\0\0\0\0\0\0\3\0\0\0\0014\1\0\000\000\000\004I\226\002\322something", buffer.to_s
   end
 
+  def test_from_should_build_new_buffer_that_includes_utf8_string_128_characters
+    length = 128
+    # letter A has a 1 byte UTF8 representation
+    buffer = Net::SSH::Buffer.from(:long, 2, :string, 'A' * length)
+    # long of 2 + length 128 as network endian + 128 A's
+    expected = "\x00\x00\x00\x02" + [length].pack('N*') + ("\x41" * length)
+    assert_equal expected, buffer.to_s
+  end
+
+  def test_from_should_build_new_buffer_with_frozen_strings
+    foo = 'foo'.freeze
+    buffer = Net::SSH::Buffer.from(:string, foo)
+    assert_equal "\0\0\0\3foo", buffer.to_s
+  end
+
+  def test_from_should_build_new_buffer_with_utf8_frozen_strings
+    foo = "\u2603".freeze
+    buffer = Net::SSH::Buffer.from(:string, foo)
+    assert_equal "\0\0\0\3\u2603".force_encoding('BINARY'), buffer.to_s
+  end
+
+  def test_from_should_not_change_regular_paramaters
+    foo = "\u2603"
+    buffer = Net::SSH::Buffer.from(:string, foo)
+    assert_equal "\0\0\0\3\u2603".force_encoding('BINARY'), buffer.to_s
+    assert_equal foo.encoding.to_s, "UTF-8"
+  end
+
   def test_from_with_array_argument_should_write_multiple_of_the_given_type
     buffer = Net::SSH::Buffer.from(:byte, [1,2,3,4,5])
     assert_equal "\1\2\3\4\5", buffer.to_s
@@ -33,7 +61,7 @@ class TestBuffer < NetSSHTest
 
   def test_from_should_measure_bytesize_of_utf_8_string_correctly
     buffer = Net::SSH::Buffer.from(:string, "\u2603") # Snowman is 3 bytes
-    assert_equal "\0\0\0\3\u2603", buffer.to_s
+    assert_equal "\0\0\0\3\u2603".force_encoding('BINARY'), buffer.to_s
   end
 
   def test_read_without_argument_should_read_to_end