Fix the bug that message without namespace is not found in the descri…

…ptor pool. (protocolbuffers#2240)
terry-zhong · Oct 11, 2016 · 60d95f3 · 60d95f3
1 parent 0321baf
commit 60d95f3
Show file tree

Hide file tree

Showing 6 changed files with 83 additions and 23 deletions.
diff --git a/Makefile.am b/Makefile.am
@@ -600,6 +600,8 @@ php_EXTRA_DIST=                                              \
   php/tests/test_include.pb.php                              \
   php/tests/map_field_test.php                               \
   php/tests/test_base.php                                    \
+  php/tests/test_no_namespace.proto                          \
+  php/tests/test_no_namespace.pb.php                         \
   php/tests/test_util.php                                    \
   php/tests/test.proto                                       \
   php/tests/test.pb.php                                      \

diff --git a/php/ext/google/protobuf/def.c b/php/ext/google/protobuf/def.c
@@ -250,28 +250,36 @@ PHP_METHOD(DescriptorPool, getGeneratedPool) {
   RETURN_ZVAL(generated_pool_php, 1, 0);
 }
 
-static void convert_to_class_name_inplace(char *proto_name,
-                                          size_t pkg_name_len) {
+static void convert_to_class_name_inplace(char *class_name,
+                                          const char* fullname,
+                                          const char* package_name) {
   size_t i;
   bool first_char = false;
-
-  for (i = 0; i <= pkg_name_len + 1; i++) {
-    // PHP package uses camel case.
-    if (!first_char && proto_name[i] != '.') {
-      first_char = true;
-      proto_name[i] += 'A' - 'a';
-    }
-    // php packages are divided by '\'.
-    if (proto_name[i] == '.') {
-      first_char = false;
-      proto_name[i] = '\\';
+  size_t pkg_name_len = package_name == NULL ? 0 : strlen(package_name);
+
+  if (pkg_name_len == 0) {
+    strcpy(class_name, fullname);
+  } else {
+    class_name[0] = '.';
+    strcpy(&class_name[1], fullname);
+    for (i = 0; i <= pkg_name_len + 1; i++) {
+      // PHP package uses camel case.
+      if (!first_char && class_name[i] != '.') {
+        first_char = true;
+        class_name[i] += 'A' - 'a';
+      }
+      // php packages are divided by '\'.
+      if (class_name[i] == '.') {
+        first_char = false;
+        class_name[i] = '\\';
+      }
     }
   }
 
   // Submessage is concatenated with its containing messages by '_'.
-  for (i = pkg_name_len; i < strlen(proto_name); i++) {
-    if (proto_name[i] == '.') {
-      proto_name[i] = '_';
+  for (i = pkg_name_len; i < strlen(class_name); i++) {
+    if (class_name[i] == '.') {
+      class_name[i] = '_';
     }
   }
 }
@@ -325,10 +333,8 @@ PHP_METHOD(DescriptorPool, internalAddGeneratedFile) {
     /* Prepend '.' to package name to make it absolute. */                     \
     const char *fullname = upb_##def_type_lower##_fullname(def_type_lower);    \
     char *klass_name = ecalloc(sizeof(char), 2 + strlen(fullname));            \
-    klass_name[0] = '.';                                                       \
-    strcpy(&klass_name[1], fullname);                                          \
-    size_t pkg_name_len = strlen(upb_filedef_package(files[0]));               \
-    convert_to_class_name_inplace(klass_name, pkg_name_len);                   \
+    convert_to_class_name_inplace(klass_name, fullname,                        \
+                                  upb_filedef_package(files[0]));              \
     zend_class_entry **pce;                                                    \
     if (zend_lookup_class(klass_name, strlen(klass_name), &pce TSRMLS_CC) ==   \
         FAILURE) {                                                             \

diff --git a/php/src/Google/Protobuf/descriptor.php b/php/src/Google/Protobuf/descriptor.php
@@ -240,9 +240,13 @@ function getFullClassName(
     $class_name_without_package =
         implode('_', array_map('ucwords',
                                explode('.', $message_name_without_package)));
-    $classname =
-        implode('\\', array_map('ucwords', explode('.', $package))).
-        "\\".$class_name_without_package;
+    if ($package === "") {
+        $classname = $class_name_without_package;
+    } else {
+        $classname =
+            implode('\\', array_map('ucwords', explode('.', $package))).
+            "\\".$class_name_without_package;
+    }
 }
 
 class OneofDescriptor

diff --git a/php/tests/generated_class_test.php b/php/tests/generated_class_test.php
@@ -1,6 +1,7 @@
 <?php
 
 require_once('test.pb.php');
+require_once('test_no_namespace.pb.php');
 require_once('test_util.php');
 
 use Google\Protobuf\Internal\RepeatedField;
@@ -554,4 +555,12 @@ public function testOneofField() {
         $this->assertSame('', $m->getOneofString());
         $this->assertSame(1, $m->getOneofMessage()->getA());
     }
+
+    #########################################################
+    # Test oneof field.
+    #########################################################
+
+    public function testMessageWithoutNamespace() {
+      $m = new NoNameSpace();
+    }
 }
diff --git a/php/tests/test_no_namespace.pb.php b/php/tests/test_no_namespace.pb.php
diff --git a/php/tests/test_no_namespace.proto b/php/tests/test_no_namespace.proto
@@ -0,0 +1,5 @@
+syntax = "proto3";
+
+message NoNameSpace {
+  int32 a = 1;
+}