Many fixes for integers handling in the binary codec. Int32 and Int64…

… will throw an exception if they contain a negative value.
yayashanzei · Jul 10, 2011 · 610529f · 610529f
1 parent 7863a17
commit 610529f
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 17 deletions.
diff --git a/library/DrSlump/Protobuf/Codec/Binary.php b/library/DrSlump/Protobuf/Codec/Binary.php
@@ -108,16 +108,23 @@ protected function encodeMessage(Protobuf\Message $message)
     protected function encodeSimpleType($writer, $type, $value)
     {
         switch ($type) {
+            case Protobuf::TYPE_INT32:
             case Protobuf::TYPE_INT64:
             case Protobuf::TYPE_UINT64:
-            case Protobuf::TYPE_INT32:
             case Protobuf::TYPE_UINT32:
+                if ($value < 0) {
+                    throw new \OutOfBoundsException("Negative Int32 and Int64 types are currently not supported ($value was given)");
+                }
+
                 $writer->varint($value);
                 break;
 
             case Protobuf::TYPE_SINT32: // ZigZag
-            case Protobuf::TYPE_SINT64: // ZigZag
-                $writer->zigzag($value);
+                $writer->zigzag($value, 32);
+                break;
+
+            case Protobuf::TYPE_SINT64 : // ZigZag
+                $writer->zigzag($value, 64);
                 break;
 
             case Protobuf::TYPE_DOUBLE:
@@ -127,6 +134,9 @@ protected function encodeSimpleType($writer, $type, $value)
                 $writer->fixed64($value);
                 break;
             case Protobuf::TYPE_SFIXED64:
+                if ($value < 0) {
+                    throw new \OutOfBoundsException("SFixed64 can only store positive integers currently ($value was given)");
+                }
                 $writer->sFixed64($value);
                 break;
 
@@ -269,7 +279,7 @@ protected function decodeUnknown($reader, $wire)
             case self::WIRE_FIXED32:
                 return $reader->fixed32();
             case self::WIRE_FIXED64:
-                return $reader->fixed64;
+                return $reader->fixed64();
             case self::WIRE_GROUP_START:
             case self::WIRE_GROUP_END:
                 throw new \RuntimeException('Groups are deprecated in Protocol Buffers and unsupported by this library');
@@ -326,9 +336,9 @@ protected function decodeSimpleType($reader, $type, $wireType)
                 return $reader->varint();
 
             case Protobuf::TYPE_SINT32: // ZigZag
-                return $reader->zigzag(32);
+                return $reader->zigzag();
             case Protobuf::TYPE_SINT64: // ZigZag
-                return $reader->zigzag(64);
+                return $reader->zigzag();
             case Protobuf::TYPE_DOUBLE:
                 return $reader->double();
             case Protobuf::TYPE_FIXED64:

diff --git a/library/DrSlump/Protobuf/Codec/Binary/Reader.php b/library/DrSlump/Protobuf/Codec/Binary/Reader.php
@@ -107,10 +107,10 @@ public function varint()
      *
      * @param int $bits - Either 32 or 64
      */
-    public function zigzag($bits)
+    public function zigzag()
     {
         $number = $this->varint();
-        return ($number << 1) ^ ($number >> ($bits-1));
+        return ($number >> 1) ^ (-($number & 1));
     }
 
     /**

diff --git a/library/DrSlump/Protobuf/Codec/Binary/Writer.php b/library/DrSlump/Protobuf/Codec/Binary/Writer.php
@@ -83,10 +83,12 @@ public function varint($value)
 
         // Build an array of bytes with the encoded values
         $values = array();
-        while ($value !== 0) {
+        while ($value > 0) {
             $values[] = 0x80 | ($value & 0x7f);
             $value = $value >> 7;
         }
+
+        // Remove the MSB flag from the last byte
         $values[count($values)-1] &= 0x7f;
 
         // Convert the byte sized ints to actual bytes in a string
@@ -100,10 +102,11 @@ public function varint($value)
      * Encodes an integer with zigzag
      *
      * @param int $value
+     * @param int $base  Either 32 or 64 bits
      */
-    public function zigzag($value)
+    public function zigzag($value, $base = 32)
     {
-        $value = ($value >> 1) ^ (-($value & 1));
+        $value = ($value << 1) ^ ($value >> $base-1);
         $this->varint($value);
     }
 
@@ -129,29 +132,33 @@ public function sFixed32($value)
      */
     public function fixed32($value)
     {
-        $bytes = pack('N*', $value);
+        $bytes = pack('V*', $value);
         $this->write($bytes, 4);
     }
 
     /**
-     * Encode an integer as a fixed of 62bits with sign
+     * Encode an integer as a fixed of 64bits with sign
      *
      * @param int $value
      */
     public function sFixed64($value)
     {
-        $bytes = pack('V*', $value & 0xffffffff, $value / (0xffffffff+1));
-        $this->write($bytes, 8);
+        if ($value < 0) {
+            throw new \OutOfBoundsException("SFixed64 can only store positive integers currently ($value was given)");
+        }
+
+        $this->fixed64($value);
     }
 
     /**
-     * Encode an integer as a fixed of 62bits without sign
+     * Encode an integer as a fixed of 64bits without sign
      *
      * @param int $value
      */
     public function fixed64($value)
     {
-        return $this->sFixed64($value);
+        $bytes = pack('V*', $value & 0xffffffff, $value / (0xffffffff+1));
+        $this->write($bytes, 8);
     }
 
     /**