Fix space stripping

Especially from the last attribute value.

2 files changed, 31 insertions, 21 deletions

diff --git a/lib/gitlab/ldap/dn.rb b/lib/gitlab/ldap/dn.rb
index 4ecb5566018..751219b7334 100644
--- a/lib/gitlab/ldap/dn.rb
+++ b/lib/gitlab/ldap/dn.rb
@@ -73,7 +73,7 @@ module Gitlab
         value = StringIO.new
         hex_buffer = ""
 
-        @dn.each_char do |char|
+        @dn.each_char.with_index do |char, dn_index|
           case state
           when :key then
             case char
@@ -108,7 +108,7 @@ module Gitlab
               value << char
             when ',' then
               state = :key
-              yield key.string.strip, value.string.rstrip
+              yield key.string.strip, rstrip_except_escaped(value.string, dn_index)
               key = StringIO.new
               value = StringIO.new
             else
@@ -120,7 +120,7 @@ module Gitlab
             when '\\' then state = :value_normal_escape
             when ',' then
               state = :key
-              yield key.string.strip, value.string.rstrip
+              yield key.string.strip, rstrip_except_escaped(value.string, dn_index)
               key = StringIO.new
               value = StringIO.new
             when '+' then raise(UnsupportedDnFormatError, "Multivalued RDNs are not supported")
@@ -131,9 +131,6 @@ module Gitlab
             when '0'..'9', 'a'..'f', 'A'..'F' then
               state = :value_normal_escape_hex
               hex_buffer = char
-            when /\s/ then
-              state = :value_normal_escape_whitespace
-              value << char
             else
               state = :value_normal
               value << char
@@ -145,17 +142,6 @@ module Gitlab
               value << "#{hex_buffer}#{char}".to_i(16).chr
             else raise(MalformedDnError, "Invalid escaped hex code \"\\#{hex_buffer}#{char}\"")
             end
-          when :value_normal_escape_whitespace then
-            case char
-            when '\\' then state = :value_normal_escape
-            when ',' then
-              state = :key
-              yield key.string.strip, value.string # Don't strip trailing escaped space!
-              key = StringIO.new
-              value = StringIO.new
-            when '+' then raise(UnsupportedDnFormatError, "Multivalued RDNs are not supported")
-            else value << char
-            end
           when :value_quoted then
             case char
             when '\\' then state = :value_quoted_escape
@@ -186,7 +172,7 @@ module Gitlab
             when ' ' then state = :value_end
             when ',' then
               state = :key
-              yield key.string.strip, value.string.rstrip
+              yield key.string.strip, rstrip_except_escaped(value.string, dn_index)
               key = StringIO.new
               value = StringIO.new
             else raise(MalformedDnError, "Expected the first character of a hex pair, but got \"#{char}\"")
@@ -203,7 +189,7 @@ module Gitlab
             when ' ' then state = :value_end
             when ',' then
               state = :key
-              yield key.string.strip, value.string.rstrip
+              yield key.string.strip, rstrip_except_escaped(value.string, dn_index)
               key = StringIO.new
               value = StringIO.new
             else raise(MalformedDnError, "Expected the end of an attribute value, but got \"#{char}\"")
@@ -216,7 +202,25 @@ module Gitlab
         raise(MalformedDnError, 'DN string ended unexpectedly') unless
           [:value, :value_normal, :value_hexstring, :value_end].include? state
 
-        yield key.string.strip, value.string.rstrip
+        yield key.string.strip, rstrip_except_escaped(value.string, @dn.length)
+      end
+
+      def rstrip_except_escaped(str, dn_index)
+        str_ends_with_whitespace = str.match(/\s\z/)
+
+        if str_ends_with_whitespace
+          dn_part_ends_with_escaped_whitespace = @dn[0, dn_index].match(/\\(\s+)\z/)
+
+          if dn_part_ends_with_escaped_whitespace
+            dn_part_rwhitespace = dn_part_ends_with_escaped_whitespace[1]
+            num_chars_to_remove = dn_part_rwhitespace.length - 1
+            str = str[0, str.length - num_chars_to_remove]
+          else
+            str.rstrip!
+          end
+        end
+
+        str
       end
 
       ##
diff --git a/spec/lib/gitlab/ldap/dn_spec.rb b/spec/lib/gitlab/ldap/dn_spec.rb
index 9a1963e2194..9a8ef7721ef 100644
--- a/spec/lib/gitlab/ldap/dn_spec.rb
+++ b/spec/lib/gitlab/ldap/dn_spec.rb
@@ -15,11 +15,17 @@ describe Gitlab::LDAP::DN do
     where(:test_description, :given, :expected) do
       'strips extraneous whitespace'                                                                 | 'uid     =John Smith ,  ou = People, dc=  example,dc =com'                                            | 'uid=john smith,ou=people,dc=example,dc=com'
       'strips extraneous whitespace for a DN with a single RDN'                                      | 'uid  =  John Smith'                                                                                  | 'uid=john smith'
-      'unescapes non-reserved, non-special Unicode characters'                                       | 'uid   =  Sebasti\\c3\\a1n\\ C.\\20Smith\\   ,   ou=People (aka. \\22humans\\")  ,dc=example, dc=com' | 'uid=sebastián c. smith  \\ ,ou=people (aka. \\"humans\\"),dc=example,dc=com'
+      'unescapes non-reserved, non-special Unicode characters'                                       | 'uid   =  Sebasti\\c3\\a1n\\ C.\\20Smith,   ou=People (aka. \\22humans\\")  ,dc=example, dc=com'      | 'uid=sebastián c. smith,ou=people (aka. \\"humans\\"),dc=example,dc=com'
       'downcases the whole string'                                                                   | 'UID=John Smith,ou=People,dc=example,dc=com'                                                          | 'uid=john smith,ou=people,dc=example,dc=com'
       'for a null DN (empty string), returns empty string and does not error'                        | ''                                                                                                    | ''
       'does not strip an escaped leading space in an attribute value'                                | 'uid=\\ John Smith,ou=People,dc=example,dc=com'                                                       | 'uid=\\ john smith,ou=people,dc=example,dc=com'
+      'does not strip an escaped leading space in the last attribute value'                          | 'uid=\\ John Smith'                                                                                   | 'uid=\\ john smith'
       'does not strip an escaped trailing space in an attribute value'                               | 'uid=John Smith\\ ,ou=People,dc=example,dc=com'                                                       | 'uid=john smith\\ ,ou=people,dc=example,dc=com'
+      'strips extraneous spaces after an escaped trailing space'                                     | 'uid=John Smith\\   ,ou=People,dc=example,dc=com'                                                     | 'uid=john smith\\ ,ou=people,dc=example,dc=com'
+      'strips extraneous spaces after an escaped trailing space at the end of the DN'                | 'uid=John Smith,ou=People,dc=example,dc=com\\   '                                                     | 'uid=john smith,ou=people,dc=example,dc=com\\ '
+      'properly preserves escaped trailing space after unescaped trailing spaces'                    | 'uid=John Smith  \\  ,ou=People,dc=example,dc=com'                                                    | 'uid=john smith  \\ ,ou=people,dc=example,dc=com'
+      'preserves multiple inner spaces in an attribute value'                                        | 'uid=John   Smith,ou=People,dc=example,dc=com'                                                        | 'uid=john   smith,ou=people,dc=example,dc=com'
+      'preserves inner spaces after an escaped space'                                                | 'uid=John\\   Smith,ou=People,dc=example,dc=com'                                                      | 'uid=john   smith,ou=people,dc=example,dc=com'
       'hex-escapes an escaped leading newline in an attribute value'                                 | "uid=\\\nJohn Smith,ou=People,dc=example,dc=com"                                                      | "uid=\\0ajohn smith,ou=people,dc=example,dc=com"
       'hex-escapes and does not strip an escaped trailing newline in an attribute value'             | "uid=John Smith\\\n,ou=People,dc=example,dc=com"                                                      | "uid=john smith\\0a,ou=people,dc=example,dc=com"
       'hex-escapes an unescaped leading newline (actually an invalid DN?)'                           | "uid=\nJohn Smith,ou=People,dc=example,dc=com"                                                        | "uid=\\0ajohn smith,ou=people,dc=example,dc=com"