Bug fix: When encoding, negative int32 values should be padded to int64 (#2660)

in order to be wire compatible.
This commit is contained in:
Paul Yang 2017-02-07 21:33:28 -08:00
parent 014a5507fb
commit f23869c615
8 changed files with 49 additions and 23 deletions

View File

@ -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)

View File

@ -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;

View File

@ -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

View File

@ -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:

View File

@ -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) {

View File

@ -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()

View File

@ -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());
}
}

View File

@ -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" .