Merge pull request #8670 from haberman/php-wkt-submsg

Fixed SEGV in sub-message getters for well-known types when message is unset.

php/ext/google/protobuf/message.c

@@ -134,6 +134,10 @@ static void Message_get(Message *intern, const upb_fielddef *f, zval *rv) {
     RepeatedField_GetPhpWrapper(rv, msgval.array, TypeInfo_Get(f),
                                 &intern->arena);
   } else {
+    if (upb_fielddef_issubmsg(f) && !upb_msg_has(intern->msg, f)) {
+      ZVAL_NULL(rv);
+      return;
+    }
     upb_msgval msgval = upb_msg_get(intern->msg, f);
     Convert_UpbToPhp(msgval, rv, TypeInfo_Get(f), &intern->arena);
   }
@@ -280,7 +284,6 @@ static int Message_has_property(PROTO_VAL *obj, PROTO_STR *member,
  * Message_unset_property()
  *
  * Object handler for unsetting a property. Called when PHP code calls:
- * does any of:
  *
  *   unset($message->foobar);
  *

php/src/Google/Protobuf/Api.php

@@ -275,7 +275,7 @@ class Api extends \Google\Protobuf\Internal\Message
      */
     public function getSourceContext()
     {
-        return isset($this->source_context) ? $this->source_context : null;
+        return $this->source_context;
     }
 
     public function hasSourceContext()

php/src/Google/Protobuf/Enum.php

@@ -155,7 +155,7 @@ class Enum extends \Google\Protobuf\Internal\Message
      */
     public function getSourceContext()
     {
-        return isset($this->source_context) ? $this->source_context : null;
+        return $this->source_context;
     }
 
     public function hasSourceContext()

php/src/Google/Protobuf/Internal/DescriptorProto.php

@@ -256,7 +256,7 @@ class DescriptorProto extends \Google\Protobuf\Internal\Message
      */
     public function getOptions()
     {
-        return isset($this->options) ? $this->options : null;
+        return $this->options;
     }
 
     public function hasOptions()

php/src/Google/Protobuf/Internal/DescriptorProto/ExtensionRange.php

@@ -128,7 +128,7 @@ class ExtensionRange extends \Google\Protobuf\Internal\Message
      */
     public function getOptions()
     {
-        return isset($this->options) ? $this->options : null;
+        return $this->options;
     }
 
     public function hasOptions()

php/src/Google/Protobuf/Internal/EnumDescriptorProto.php

@@ -128,7 +128,7 @@ class EnumDescriptorProto extends \Google\Protobuf\Internal\Message
      */
     public function getOptions()
     {
-        return isset($this->options) ? $this->options : null;
+        return $this->options;
     }
 
     public function hasOptions()

php/src/Google/Protobuf/Internal/EnumValueDescriptorProto.php

@@ -116,7 +116,7 @@ class EnumValueDescriptorProto extends \Google\Protobuf\Internal\Message
      */
     public function getOptions()
     {
-        return isset($this->options) ? $this->options : null;
+        return $this->options;
     }
 
     public function hasOptions()

php/src/Google/Protobuf/Internal/FieldDescriptorProto.php

@@ -515,7 +515,7 @@ class FieldDescriptorProto extends \Google\Protobuf\Internal\Message
      */
     public function getOptions()
     {
-        return isset($this->options) ? $this->options : null;
+        return $this->options;
     }
 
     public function hasOptions()

php/src/Google/Protobuf/Internal/FileDescriptorProto.php

@@ -375,7 +375,7 @@ class FileDescriptorProto extends \Google\Protobuf\Internal\Message
      */
     public function getOptions()
     {
-        return isset($this->options) ? $this->options : null;
+        return $this->options;
     }
 
     public function hasOptions()
@@ -412,7 +412,7 @@ class FileDescriptorProto extends \Google\Protobuf\Internal\Message
      */
     public function getSourceCodeInfo()
     {
-        return isset($this->source_code_info) ? $this->source_code_info : null;
+        return $this->source_code_info;
     }
 
     public function hasSourceCodeInfo()

php/src/Google/Protobuf/Internal/MethodDescriptorProto.php

@@ -180,7 +180,7 @@ class MethodDescriptorProto extends \Google\Protobuf\Internal\Message
      */
     public function getOptions()
     {
-        return isset($this->options) ? $this->options : null;
+        return $this->options;
     }
 
     public function hasOptions()

php/src/Google/Protobuf/Internal/OneofDescriptorProto.php

@@ -79,7 +79,7 @@ class OneofDescriptorProto extends \Google\Protobuf\Internal\Message
      */
     public function getOptions()
     {
-        return isset($this->options) ? $this->options : null;
+        return $this->options;
     }
 
     public function hasOptions()

php/src/Google/Protobuf/Internal/ServiceDescriptorProto.php

@@ -106,7 +106,7 @@ class ServiceDescriptorProto extends \Google\Protobuf\Internal\Message
      */
     public function getOptions()
     {
-        return isset($this->options) ? $this->options : null;
+        return $this->options;
     }
 
     public function hasOptions()

php/src/Google/Protobuf/Option.php

@@ -101,7 +101,7 @@ class Option extends \Google\Protobuf\Internal\Message
      */
     public function getValue()
     {
-        return isset($this->value) ? $this->value : null;
+        return $this->value;
     }
 
     public function hasValue()

php/src/Google/Protobuf/Type.php

@@ -189,7 +189,7 @@ class Type extends \Google\Protobuf\Internal\Message
      */
     public function getSourceContext()
     {
-        return isset($this->source_context) ? $this->source_context : null;
+        return $this->source_context;
     }
 
     public function hasSourceContext()

php/tests/GeneratedClassTest.php

@@ -472,6 +472,8 @@ class GeneratedClassTest extends TestBase
     {
         $m = new TestMessage();
 
+        $this->assertNull($m->getOptionalMessage());
+
         $sub_m = new Sub();
         $sub_m->setA(1);
         $m->setOptionalMessage($sub_m);

php/tests/WellKnownTest.php

@@ -270,6 +270,8 @@ class WellKnownTest extends TestBase {
 
         $m = new Value();
 
+        $this->assertNull($m->getStructValue());
+
         $m->setNullValue(NullValue::NULL_VALUE);
         $this->assertSame(NullValue::NULL_VALUE, $m->getNullValue());
         $this->assertSame("null_value", $m->getKind());

src/google/protobuf/compiler/php/php_generator.cc

@@ -648,32 +648,21 @@ void GenerateFieldAccessor(const FieldDescriptor* field, const Options& options,
   std::string deprecation_trigger = (field->options().deprecated()) ? "@trigger_error('" +
       field->name() + " is deprecated.', E_USER_DEPRECATED);\n        " : "";
 
+  // Emit getter.
   if (oneof != NULL) {
     printer->Print(
         "public function get^camel_name^()\n"
         "{\n"
         "    ^deprecation_trigger^return $this->readOneof(^number^);\n"
-        "}\n\n"
-        "public function has^camel_name^()\n"
-        "{\n"
-        "    ^deprecation_trigger^return $this->hasOneof(^number^);\n"
         "}\n\n",
         "camel_name", UnderscoresToCamelCase(field->name(), true),
         "number", IntToString(field->number()),
         "deprecation_trigger", deprecation_trigger);
-  } else if (field->has_presence()) {
+  } else if (field->has_presence() && !field->message_type()) {
     printer->Print(
         "public function get^camel_name^()\n"
         "{\n"
         "    ^deprecation_trigger^return isset($this->^name^) ? $this->^name^ : ^default_value^;\n"
-        "}\n\n"
-        "public function has^camel_name^()\n"
-        "{\n"
-        "    ^deprecation_trigger^return isset($this->^name^);\n"
-        "}\n\n"
-        "public function clear^camel_name^()\n"
-        "{\n"
-        "    ^deprecation_trigger^unset($this->^name^);\n"
         "}\n\n",
         "camel_name", UnderscoresToCamelCase(field->name(), true),
         "name", field->name(),
@@ -690,6 +679,32 @@ void GenerateFieldAccessor(const FieldDescriptor* field, const Options& options,
         "deprecation_trigger", deprecation_trigger);
   }
 
+  // Emit hazzers/clear.
+  if (oneof) {
+    printer->Print(
+        "public function has^camel_name^()\n"
+        "{\n"
+        "    ^deprecation_trigger^return $this->hasOneof(^number^);\n"
+        "}\n\n",
+        "camel_name", UnderscoresToCamelCase(field->name(), true),
+        "number", IntToString(field->number()),
+        "deprecation_trigger", deprecation_trigger);
+  } else if (field->has_presence()) {
+    printer->Print(
+        "public function has^camel_name^()\n"
+        "{\n"
+        "    ^deprecation_trigger^return isset($this->^name^);\n"
+        "}\n\n"
+        "public function clear^camel_name^()\n"
+        "{\n"
+        "    ^deprecation_trigger^unset($this->^name^);\n"
+        "}\n\n",
+        "camel_name", UnderscoresToCamelCase(field->name(), true),
+        "name", field->name(),
+        "default_value", DefaultForField(field),
+        "deprecation_trigger", deprecation_trigger);
+  }
+
   // For wrapper types, generate an additional getXXXUnwrapped getter
   if (!field->is_map() &&
       !field->is_repeated() &&