drslump / Protobuf-PHP

PHP implementation of Google's Protocol Buffers with a protoc plugin compiler
http://drslump.github.com/Protobuf-PHP/
MIT License
462 stars 163 forks source link

Int64 wrong parsing. #50

Open Hiller opened 9 years ago

Hiller commented 9 years ago

Hi. I found that Int64 parsing from binary reader is wrong. it should be done like this.

    public function varint()
    {
        $result = $shift = 0;
        $bytes=array();

        do {
            $byte = $this->byte();
            $bytes[]=$byte;
        } while ($byte > 0x7f);
        for ($j = count($bytes)-1; $j >= 0; $j--)
        {
            $result = $result * 128 + ($bytes[$j] & 0x7F);
        }
        return $result;
    }
drslump commented 9 years ago

I think the problem is perhaps you're running on a PHP interpreter with 32bit integers, could you check the value for PHP_INT_SIZE?

The current implementation relies on bit arithmetic to improve the performance which in PHP always result on an integer, in your solution it uses standard arithmetic which for big numbers will end up resulting on a floating point number which can end up loosing precision.

Perhaps the library can be changed to fallback to your decoding implementation if after the 4th byte we're still decoding more ($byte > 0x7f). That way we would still use the in theory faster bit arithmetic for most common cases but support floating point if we go out of the integer range.

Hiller commented 9 years ago

Integer max size for me is 9223372036854775807 but when i'm trying to decode message wich contains timestamp in microseconds, i receive wrong number. Using this function i'm getting it right. Just for comparison: my way: public 'timestamp' => float 1428581286157 old way: public 'timestamp' => float 79193446 Even with 32 bit integer this number should be much greater

Sufir commented 7 years ago

This problem is solved now?