Merge pull request #173 from ruby-ldap/ber-encoding-integers-refactoring

Refactor BER encoding/decoding of Integers

Author: mtodd

lib/net/ber/ber_parser.rb

@@ -41,9 +41,18 @@ def parse_ber_object(syntax, id, data)
       s.ber_identifier = id
       s
     elsif object_type == :integer
-      j = 0
-      data.each_byte { |b| j = (j << 8) + b }
-      j
+      neg = !(data.unpack("C").first & 0x80).zero?
+      int = 0
+
+      data.each_byte do |b|
+        int = (int << 8) + (neg ? 255 - b : b)
+      end
+
+      if neg
+        (int + 1) * -1
+      else
+        int
+      end
     elsif object_type == :oid
       # See X.690 pgh 8.19 for an explanation of this algorithm.
       # This is potentially not good enough. We may need a

lib/net/ber/core_ext.rb

@@ -33,17 +33,10 @@ class Array
 end
 # :startdoc:
 
-require 'net/ber/core_ext/bignum'
+require 'net/ber/core_ext/integer'
 # :stopdoc:
-class Bignum
-  include Net::BER::Extensions::Bignum
-end
-# :startdoc:
-
-require 'net/ber/core_ext/fixnum'
-# :stopdoc:
-class Fixnum
-  include Net::BER::Extensions::Fixnum
+class Integer
+  include Net::BER::Extensions::Integer
 end
 # :startdoc:
 

lib/net/ber/core_ext/bignum.rb

@@ -1,21 +1,21 @@
 # -*- ruby encoding: utf-8 -*-
 ##
-# Ber extensions to the Fixnum class.
-module Net::BER::Extensions::Fixnum
+# BER extensions to the Integer class, affecting Fixnum and Bignum objects.
+module Net::BER::Extensions::Integer
   ##
-  # Converts the fixnum to BER format.
+  # Converts the Integer to BER format.
   def to_ber
     "\002#{to_ber_internal}"
   end
 
   ##
-  # Converts the fixnum to BER enumerated format.
+  # Converts the Integer to BER enumerated format.
   def to_ber_enumerated
     "\012#{to_ber_internal}"
   end
 
   ##
-  # Converts the fixnum to BER length encodining format.
+  # Converts the Integer to BER length encoding format.
   def to_ber_length_encoding
     if self <= 127
       [self].pack('C')
@@ -33,34 +33,34 @@ def to_ber_application(tag)
   end
 
   ##
-  # Used to BER-encode the length and content bytes of a Fixnum. Callers
+  # Used to BER-encode the length and content bytes of an Integer. Callers
   # must prepend the tag byte for the contained value.
   def to_ber_internal
-    # CAUTION: Bit twiddling ahead. You might want to shield your eyes or
-    # something.
+    # Compute the byte length, accounting for negative values requiring two's
+    # complement.
+    size  = 1
+    size += 1 until (((self < 0) ? ~self : self) >> (size * 8)).zero?
 
-    # Looks for the first byte in the fixnum that is not all zeroes. It does
-    # this by masking one byte after another, checking the result for bits
-    # that are left on.
-    size = Net::BER::MAX_FIXNUM_SIZE
-    while size > 1
-      break if (self & (0xff << (size - 1) * 8)) > 0
-      size -= 1
+    # Padding for positive, negative values. See section 8.5 of ITU-T X.690:
+    # http://www.itu.int/ITU-T/studygroups/com17/languages/X.690-0207.pdf
+
+    # For positive integers, if most significant bit in an octet is set to one,
+    # pad the result (otherwise it's decoded as a negative value).
+    if self > 0 && (self & (0x80 << (size - 1) * 8)) > 0
+      size += 1
     end
 
-    # for positive integers, if most significant bit in an octet is set to one,
-    # pad the result (otherwise it's decoded as a negative value)
-    # See section 8.5 of ITU-T X.690:
-    # http://www.itu.int/ITU-T/studygroups/com17/languages/X.690-0207.pdf
-    if self > 0 && (self & (0b10000000 << (size - 1) * 8)) > 0
+    # And for negative integers, pad if the most significant bit in the octet
+    # is not set to one (othwerise, it's decoded as positive value).
+    if self < 0 && (self & (0x80 << (size - 1) * 8)) == 0
       size += 1
     end
 
-    # Store the size of the fixnum in the result
+    # Store the size of the Integer in the result
     result = [size]
 
     # Appends bytes to result, starting with higher orders first. Extraction
-    # of bytes is done by right shifting the original fixnum by an amount
+    # of bytes is done by right shifting the original Integer by an amount
     # and then masking that with 0xff.
     while size > 0
       # right shift size - 1 bytes, mask with 0xff

lib/net/ber/core_ext/fixnum.rb renamed to lib/net/ber/core_ext/integer.rb

@@ -45,9 +45,17 @@ def test_false
     5_000_000_000 => "\x02\x05\x01\x2a\x05\xF2\x00",
 
     # negatives
-    # -1          => "\x02\x01\xFF",
-    # -127        => "\x02\x01\x81",
-    # -128        => "\x02\x01\x80"
+    -1          => "\x02\x01\xFF",
+    -127        => "\x02\x01\x81",
+    -128        => "\x02\x01\x80",
+    -255        => "\x02\x02\xFF\x01",
+    -256        => "\x02\x02\xFF\x00",
+    -65535      => "\x02\x03\xFF\x00\x01",
+    -65536      => "\x02\x03\xFF\x00\x00",
+    -65537      => "\x02\x03\xFE\xFF\xFF",
+    -8388607    => "\x02\x03\x80\x00\x01",
+    -8388608    => "\x02\x03\x80\x00\x00",
+    -16_777_215 => "\x02\x04\xFF\x00\x00\x01",
   }.each do |number, expected_encoding|
     define_method "test_encode_#{number}" do
       assert_equal expected_encoding.b, number.to_ber