Throw exception when parsing invalid data. (#3000)

3 files changed, 96 insertions, 41 deletions

diff --git a/php/src/Google/Protobuf/Internal/GPBDecodeException.php b/php/src/Google/Protobuf/Internal/GPBDecodeException.php
new file mode 100644
index 00000000..402d542f
--- /dev/null
+++ b/php/src/Google/Protobuf/Internal/GPBDecodeException.php
@@ -0,0 +1,47 @@
+<?php
+
+// Protocol Buffers - Google's data interchange format
+// Copyright 2008 Google Inc.  All rights reserved.
+// https://developers.google.com/protocol-buffers/
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+namespace Google\Protobuf\Internal;
+
+class GPBDecodeException extends \Exception
+{
+    public function __construct(
+        $message,
+        $code = 0,
+        \Exception $previous = null)
+    {
+        parent::__construct(
+            "Error occurred during parsing: " . $message,
+            $code,
+            $previous);
+    }
+}
diff --git a/php/src/Google/Protobuf/Internal/InputStream.php b/php/src/Google/Protobuf/Internal/InputStream.php
index de5ca978..8012a225 100644
--- a/php/src/Google/Protobuf/Internal/InputStream.php
+++ b/php/src/Google/Protobuf/Internal/InputStream.php
@@ -330,6 +330,7 @@ class InputStream
      * passed unchanged to the corresponding call to popLimit().
      *
      * @param integer $byte_limit
+     * @throws Exception Fail to push limit.
      */
     public function pushLimit($byte_limit)
     {
@@ -339,19 +340,15 @@ class InputStream
 
         // security: byte_limit is possibly evil, so check for negative values
         // and overflow.
-        if ($byte_limit >= 0 && $byte_limit <= PHP_INT_MAX - $current_position) {
+        if ($byte_limit >= 0 &&
+            $byte_limit <= PHP_INT_MAX - $current_position &&
+            $byte_limit <= $this->current_limit - $current_position) {
             $this->current_limit = $current_position + $byte_limit;
+            $this->recomputeBufferLimits();
         } else {
-            // Negative or overflow.
-            $this->current_limit = PHP_INT_MAX;
+            throw new GPBDecodeException("Fail to push limit.");
         }
 
-        // We need to enforce all limits, not just the new one, so if the previous
-        // limit was before the new requested limit, we continue to enforce the
-        // previous limit.
-        $this->current_limit = min($this->current_limit, $old_limit);
-
-        $this->recomputeBufferLimits();
         return $old_limit;
     }
 
@@ -370,7 +367,7 @@ class InputStream
     }
 
     public function incrementRecursionDepthAndPushLimit(
-    $byte_limit, &$old_limit, &$recursion_budget)
+        $byte_limit, &$old_limit, &$recursion_budget)
     {
         $old_limit = $this->pushLimit($byte_limit);
         $recursion_limit = --$this->recursion_limit;
diff --git a/php/src/Google/Protobuf/Internal/Message.php b/php/src/Google/Protobuf/Internal/Message.php
index 887c86ca..cd15e0f0 100644
--- a/php/src/Google/Protobuf/Internal/Message.php
+++ b/php/src/Google/Protobuf/Internal/Message.php
@@ -224,48 +224,57 @@ class Message
         switch ($field->getType()) {
             case GPBType::DOUBLE:
                 if (!GPBWire::readDouble($input, $value)) {
-                    return false;
+                    throw new GPBDecodeException(
+                        "Unexpected EOF inside double field.");
                 }
                 break;
             case GPBType::FLOAT:
                 if (!GPBWire::readFloat($input, $value)) {
-                    return false;
+                    throw new GPBDecodeException(
+                        "Unexpected EOF inside float field.");
                 }
                 break;
             case GPBType::INT64:
                 if (!GPBWire::readInt64($input, $value)) {
-                    return false;
+                    throw new GPBDecodeException(
+                        "Unexpected EOF inside int64 field.");
                 }
                 break;
             case GPBType::UINT64:
                 if (!GPBWire::readUint64($input, $value)) {
-                    return false;
+                    throw new GPBDecodeException(
+                        "Unexpected EOF inside uint64 field.");
                 }
                 break;
             case GPBType::INT32:
                 if (!GPBWire::readInt32($input, $value)) {
-                    return false;
+                    throw new GPBDecodeException(
+                        "Unexpected EOF inside int32 field.");
                 }
                 break;
             case GPBType::FIXED64:
                 if (!GPBWire::readFixed64($input, $value)) {
-                    return false;
+                    throw new GPBDecodeException(
+                        "Unexpected EOF inside fixed64 field.");
                 }
                 break;
             case GPBType::FIXED32:
                 if (!GPBWire::readFixed32($input, $value)) {
-                    return false;
+                    throw new GPBDecodeException(
+                        "Unexpected EOF inside fixed32 field.");
                 }
                 break;
             case GPBType::BOOL:
                 if (!GPBWire::readBool($input, $value)) {
-                    return false;
+                    throw new GPBDecodeException(
+                        "Unexpected EOF inside bool field.");
                 }
                 break;
             case GPBType::STRING:
                 // TODO(teboring): Add utf-8 check.
                 if (!GPBWire::readString($input, $value)) {
-                    return false;
+                    throw new GPBDecodeException(
+                        "Unexpected EOF inside string field.");
                 }
                 break;
             case GPBType::GROUP:
@@ -280,43 +289,51 @@ class Message
                     $value = new $klass;
                 }
                 if (!GPBWire::readMessage($input, $value)) {
-                    return false;
+                    throw new GPBDecodeException(
+                        "Unexpected EOF inside message.");
                 }
                 break;
             case GPBType::BYTES:
                 if (!GPBWire::readString($input, $value)) {
-                    return false;
+                    throw new GPBDecodeException(
+                        "Unexpected EOF inside bytes field.");
                 }
                 break;
             case GPBType::UINT32:
                 if (!GPBWire::readUint32($input, $value)) {
-                    return false;
+                    throw new GPBDecodeException(
+                        "Unexpected EOF inside uint32 field.");
                 }
                 break;
             case GPBType::ENUM:
                 // TODO(teboring): Check unknown enum value.
                 if (!GPBWire::readInt32($input, $value)) {
-                    return false;
+                    throw new GPBDecodeException(
+                        "Unexpected EOF inside enum field.");
                 }
                 break;
             case GPBType::SFIXED32:
                 if (!GPBWire::readSfixed32($input, $value)) {
-                    return false;
+                    throw new GPBDecodeException(
+                        "Unexpected EOF inside sfixed32 field.");
                 }
                 break;
             case GPBType::SFIXED64:
                 if (!GPBWire::readSfixed64($input, $value)) {
-                    return false;
+                    throw new GPBDecodeException(
+                        "Unexpected EOF inside sfixed64 field.");
                 }
                 break;
             case GPBType::SINT32:
                 if (!GPBWire::readSint32($input, $value)) {
-                    return false;
+                    throw new GPBDecodeException(
+                        "Unexpected EOF inside sint32 field.");
                 }
                 break;
             case GPBType::SINT64:
                 if (!GPBWire::readSint64($input, $value)) {
-                    return false;
+                    throw new GPBDecodeException(
+                        "Unexpected EOF inside sint64 field.");
                 }
                 break;
             default:
@@ -345,24 +362,21 @@ class Message
         }
 
         if ($value_format === GPBWire::NORMAL_FORMAT) {
-            if (!self::parseFieldFromStreamNoTag($input, $field, $value)) {
-                return false;
-            }
+            self::parseFieldFromStreamNoTag($input, $field, $value);
         } elseif ($value_format === GPBWire::PACKED_FORMAT) {
             $length = 0;
             if (!GPBWire::readInt32($input, $length)) {
-                return false;
+                throw new GPBDecodeException(
+                    "Unexpected EOF inside packed length.");
             }
             $limit = $input->pushLimit($length);
             $getter = $field->getGetter();
             while ($input->bytesUntilLimit() > 0) {
-                if (!self::parseFieldFromStreamNoTag($input, $field, $value)) {
-                    return false;
-                }
+                self::parseFieldFromStreamNoTag($input, $field, $value);
                 $this->$getter()[] = $value;
             }
             $input->popLimit($limit);
-            return true;
+            return;
         } else {
             return false;
         }
@@ -377,8 +391,6 @@ class Message
             $setter = $field->getSetter();
             $this->$setter($value);
         }
-
-        return true;
     }
 
     /**
@@ -567,7 +579,8 @@ class Message
      * specified message.
      *
      * @param string $data Binary protobuf data.
-     * @return bool Return true on success.
+     * @return null.
+     * @throws Exception Invalid data.
      */
     public function mergeFromString($data)
     {
@@ -595,9 +608,7 @@ class Message
               continue;
             }
 
-            if (!$this->parseFieldFromStream($tag, $input, $field)) {
-                return false;
-            }
+            $this->parseFieldFromStream($tag, $input, $field);
         }
     }