diff --git a/php/ext/google/protobuf/upb.c b/php/ext/google/protobuf/upb.c index e0c56f8ed..aca1eb6e4 100644 --- a/php/ext/google/protobuf/upb.c +++ b/php/ext/google/protobuf/upb.c @@ -10759,7 +10759,7 @@ static size_t encode_strbuf(void *c, const void *hd, const char *buf, T(double, double, dbl2uint64, encode_fixed64) T(float, float, flt2uint32, encode_fixed32) T(int64, int64_t, uint64_t, encode_varint) -T(int32, int32_t, uint32_t, encode_varint) +T(int32, int32_t, int64_t, encode_varint) T(fixed64, uint64_t, uint64_t, encode_fixed64) T(fixed32, uint32_t, uint32_t, encode_fixed32) T(bool, bool, bool, encode_varint) diff --git a/php/src/Google/Protobuf/Internal/GPBWire.php b/php/src/Google/Protobuf/Internal/GPBWire.php index f75e08613..67eb1bee6 100644 --- a/php/src/Google/Protobuf/Internal/GPBWire.php +++ b/php/src/Google/Protobuf/Internal/GPBWire.php @@ -403,10 +403,14 @@ class GPBWire return self::varint32Size($tag); } - public static function varint32Size($value) + public static function varint32Size($value, $sign_extended = false) { if ($value < 0) { - return 5; + if ($sign_extended) { + return 10; + } else { + return 5; + } } if ($value < (1 << 7)) { return 1; diff --git a/php/src/Google/Protobuf/Internal/InputStream.php b/php/src/Google/Protobuf/Internal/InputStream.php index bf052c2f8..de5ca9781 100644 --- a/php/src/Google/Protobuf/Internal/InputStream.php +++ b/php/src/Google/Protobuf/Internal/InputStream.php @@ -70,7 +70,6 @@ class InputStream private $total_bytes_read; const MAX_VARINT_BYTES = 10; - const MAX_VARINT32_BYTES = 5; const DEFAULT_RECURSION_LIMIT = 100; const DEFAULT_TOTAL_BYTES_LIMIT = 33554432; // 32 << 20, 32MB diff --git a/php/src/Google/Protobuf/Internal/Message.php b/php/src/Google/Protobuf/Internal/Message.php index ca4fde028..887c86ca6 100644 --- a/php/src/Google/Protobuf/Internal/Message.php +++ b/php/src/Google/Protobuf/Internal/Message.php @@ -772,9 +772,11 @@ class Message case GPBType::SFIXED64: $size += 8; break; - case GPBType::UINT32: case GPBType::INT32: case GPBType::ENUM: + $size += GPBWire::varint32Size($value, true); + break; + case GPBType::UINT32: $size += GPBWire::varint32Size($value); break; case GPBType::UINT64: diff --git a/php/src/Google/Protobuf/Internal/OutputStream.php b/php/src/Google/Protobuf/Internal/OutputStream.php index 587ac352d..8c6d9b680 100644 --- a/php/src/Google/Protobuf/Internal/OutputStream.php +++ b/php/src/Google/Protobuf/Internal/OutputStream.php @@ -39,7 +39,6 @@ class OutputStream private $buffer_size; private $current; - const MAX_VARINT32_BYTES = 5; const MAX_VARINT64_BYTES = 10; public function __construct($size) @@ -56,8 +55,8 @@ class OutputStream public function writeVarint32($value) { - $bytes = str_repeat(chr(0), self::MAX_VARINT32_BYTES); - $size = self::writeVarintToArray($value, $bytes, true); + $bytes = str_repeat(chr(0), self::MAX_VARINT64_BYTES); + $size = self::writeVarintToArray($value, $bytes); return $this->writeRaw($bytes, $size); } @@ -102,20 +101,16 @@ class OutputStream return true; } - private static function writeVarintToArray($value, &$buffer, $trim = false) + private static function writeVarintToArray($value, &$buffer) { $current = 0; $high = 0; $low = 0; if (PHP_INT_SIZE == 4) { - GPBUtil::divideInt64ToInt32($value, $high, $low, $trim); + GPBUtil::divideInt64ToInt32($value, $high, $low); } else { - if ($trim) { - $low = $value & 0xFFFFFFFF; - } else { - $low = $value; - } + $low = $value; } while ($low >= 0x80 || $low < 0) { diff --git a/php/tests/encode_decode_test.php b/php/tests/encode_decode_test.php index b54b12392..ba4fff697 100644 --- a/php/tests/encode_decode_test.php +++ b/php/tests/encode_decode_test.php @@ -190,6 +190,27 @@ class EncodeDecodeTest extends TestBase $m->mergeFromString($data); } + public function testEncodeNegativeInt32() + { + $m = new TestMessage(); + $m->setOptionalInt32(-1); + $data = $m->encode(); + $this->assertSame("08ffffffffffffffffff01", bin2hex($data)); + } + + public function testDecodeNegativeInt32() + { + $m = new TestMessage(); + $this->assertEquals(0, $m->getOptionalInt32()); + $m->decode(hex2bin("08ffffffffffffffffff01")); + $this->assertEquals(-1, $m->getOptionalInt32()); + + $m = new TestMessage(); + $this->assertEquals(0, $m->getOptionalInt32()); + $m->decode(hex2bin("08ffffffff0f")); + $this->assertEquals(-1, $m->getOptionalInt32()); + } + # TODO(teboring): Add test back when php implementation is ready for json # encode/decode. # public function testJsonEncode() diff --git a/php/tests/php_implementation_test.php b/php/tests/php_implementation_test.php index ec6b8d5a7..e12498082 100644 --- a/php/tests/php_implementation_test.php +++ b/php/tests/php_implementation_test.php @@ -469,6 +469,11 @@ class ImplementationTest extends TestBase $output = new OutputStream(3); $output->writeVarint32(16384); $this->assertSame(hex2bin('808001'), $output->getData()); + + // Negative numbers are padded to be compatible with int64. + $output = new OutputStream(10); + $output->writeVarint32(-43); + $this->assertSame(hex2bin('D5FFFFFFFFFFFFFFFF01'), $output->getData()); } public function testWriteVarint64() @@ -496,13 +501,13 @@ class ImplementationTest extends TestBase { $m = new TestMessage(); TestUtil::setTestMessage($m); - $this->assertSame(481, $m->byteSize()); + $this->assertSame(506, $m->byteSize()); } public function testPackedByteSize() { $m = new TestPackedMessage(); TestUtil::setTestPackedMessage($m); - $this->assertSame(156, $m->byteSize()); + $this->assertSame(166, $m->byteSize()); } } diff --git a/php/tests/test_util.php b/php/tests/test_util.php index 9c8e34d2a..61f94aa10 100644 --- a/php/tests/test_util.php +++ b/php/tests/test_util.php @@ -321,7 +321,7 @@ class TestUtil public static function getGoldenTestMessage() { return hex2bin( - "08D6FFFFFF0F" . + "08D6FFFFFFFFFFFFFFFF01" . "10D5FFFFFFFFFFFFFFFF01" . "182A" . "202B" . @@ -339,8 +339,8 @@ class TestUtil "800101" . "8A01020821" . - "F801D6FFFFFF0F" . - "F801CCFFFFFF0F" . + "F801D6FFFFFFFFFFFFFFFF01" . + "F801CCFFFFFFFFFFFFFFFF01" . "8002D5FFFFFFFFFFFFFFFF01" . "8002CBFFFFFFFFFFFFFFFF01" . "88022A" . @@ -374,7 +374,7 @@ class TestUtil "FA02020822" . "FA02020823" . - "BA040C08C2FFFFFF0F10C2FFFFFF0F" . + "BA041608C2FFFFFFFFFFFFFFFF0110C2FFFFFFFFFFFFFFFF01" . "C2041608C1FFFFFFFFFFFFFFFF0110C1FFFFFFFFFFFFFFFF01" . "CA0404083E103E" . "D20404083F103F" . @@ -489,7 +489,7 @@ class TestUtil public static function getGoldenTestPackedMessage() { return hex2bin( - "D2050AD6FFFFFF0FCCFFFFFF0F" . + "D20514D6FFFFFFFFFFFFFFFF01CCFFFFFFFFFFFFFFFF01" . "DA0514D5FFFFFFFFFFFFFFFF01CBFFFFFFFFFFFFFFFF01" . "E205022A34" . "EA05022B35" . @@ -509,7 +509,7 @@ class TestUtil public static function getGoldenTestUnpackedMessage() { return hex2bin( - "D005D6FFFFFF0FD005CCFFFFFF0F" . + "D005D6FFFFFFFFFFFFFFFF01D005CCFFFFFFFFFFFFFFFF01" . "D805D5FFFFFFFFFFFFFFFF01D805CBFFFFFFFFFFFFFFFF01" . "E0052AE00534" . "E8052BE80535" .