From 190b5270c8717ca343db42da489e5e7d6d9efb2c Mon Sep 17 00:00:00 2001 From: Paul Yang Date: Wed, 19 Apr 2017 16:23:51 -0700 Subject: Make PHP c extension work with PHP7 (#2951) --- php/src/Google/Protobuf/Internal/MapField.php | 3 +++ php/src/Google/Protobuf/Internal/RepeatedField.php | 4 ++++ 2 files changed, 7 insertions(+) (limited to 'php/src/Google/Protobuf') diff --git a/php/src/Google/Protobuf/Internal/MapField.php b/php/src/Google/Protobuf/Internal/MapField.php index 68c10c08..f65bd9b8 100644 --- a/php/src/Google/Protobuf/Internal/MapField.php +++ b/php/src/Google/Protobuf/Internal/MapField.php @@ -284,6 +284,9 @@ class MapField implements \ArrayAccess, \IteratorAggregate, \Countable GPBUtil::checkString($value, true); break; case GPBType::MESSAGE: + if (is_null($value)) { + trigger_error("Map element cannot be null.", E_USER_ERROR); + } GPBUtil::checkMessage($value, $this->klass); break; default: diff --git a/php/src/Google/Protobuf/Internal/RepeatedField.php b/php/src/Google/Protobuf/Internal/RepeatedField.php index 0dc5d9d2..2ad4709a 100644 --- a/php/src/Google/Protobuf/Internal/RepeatedField.php +++ b/php/src/Google/Protobuf/Internal/RepeatedField.php @@ -225,6 +225,10 @@ class RepeatedField implements \ArrayAccess, \IteratorAggregate, \Countable GPBUtil::checkString($value, true); break; case GPBType::MESSAGE: + if (is_null($value)) { + trigger_error("RepeatedField element cannot be null.", + E_USER_ERROR); + } GPBUtil::checkMessage($value, $this->klass); break; default: -- cgit v1.2.3 From 4c57e8475f78ccac80407f03c2d23d30014785f9 Mon Sep 17 00:00:00 2001 From: Paul Yang Date: Thu, 20 Apr 2017 01:19:03 -0700 Subject: Prepend "PB" to generated classes whose name are reserved words. (#2990) --- php/ext/google/protobuf/def.c | 123 ++++++++++++++-------- php/src/Google/Protobuf/Internal/Message.php | 1 + php/src/Google/Protobuf/descriptor.php | 36 +++++-- php/tests/generated_class_test.php | 11 ++ php/tests/memory_leak_test.php | 3 + php/tests/proto/test.proto | 10 ++ php/tests/proto/test_prefix.proto | 5 + src/google/protobuf/compiler/php/php_generator.cc | 35 ++++-- 8 files changed, 163 insertions(+), 61 deletions(-) (limited to 'php/src/Google/Protobuf') diff --git a/php/ext/google/protobuf/def.c b/php/ext/google/protobuf/def.c index 50c0350e..8e563a61 100644 --- a/php/ext/google/protobuf/def.c +++ b/php/ext/google/protobuf/def.c @@ -30,6 +30,9 @@ #include "protobuf.h" +const char* const kReservedNames[] = {"Empty"}; +const int kReservedNamesSize = 1; + // Forward declare. static void descriptor_init_c_instance(Descriptor* intern TSRMLS_DC); static void descriptor_free_c(Descriptor* object TSRMLS_DC); @@ -355,58 +358,90 @@ PHP_METHOD(DescriptorPool, getGeneratedPool) { #endif } -static void convert_to_class_name_inplace(char *class_name, - const char* fullname, - const char* prefix, - const char* package_name) { +static void classname_no_prefix(const char *fullname, const char *package_name, + char *class_name) { size_t i = 0, j; - bool first_char = true; + bool first_char = true, is_reserved = false; size_t pkg_name_len = package_name == NULL ? 0 : strlen(package_name); - size_t prefix_len = prefix == NULL ? 0 : strlen(prefix); size_t message_name_start = package_name == NULL ? 0 : pkg_name_len + 1; size_t message_len = (strlen(fullname) - message_name_start); - // In php, class name cannot be Empty. - if (strcmp("google.protobuf.Empty", fullname) == 0) { - strcpy(class_name, "\\Google\\Protobuf\\GPBEmpty"); - return; + // Submessage is concatenated with its containing messages by '_'. + for (j = message_name_start; j < message_name_start + message_len; j++) { + if (fullname[j] == '.') { + class_name[i++] = '_'; + } else { + class_name[i++] = fullname[j]; + } + } +} + +static const char *classname_prefix(const char *classname, + const char *prefix_given, + const char *package_name) { + size_t i; + bool is_reserved = false; + + if (prefix_given != NULL && strcmp(prefix_given, "") != 0) { + return prefix_given; } - if (pkg_name_len != 0) { - class_name[i++] = '\\'; - for (j = 0; j < pkg_name_len; j++) { + for (i = 0; i < kReservedNamesSize; i++) { + if (strcmp(kReservedNames[i], classname) == 0) { + is_reserved = true; + break; + } + } + + if (is_reserved) { + if (package_name != NULL && strcmp("google.protobuf", package_name) == 0) { + return "GPB"; + } else { + return "PB"; + } + } + + return ""; +} + +static void convert_to_class_name_inplace(const char *package, + const char *prefix, char *classname) { + size_t package_len = package == NULL ? 0 : strlen(package); + size_t prefix_len = prefix == NULL ? 0 : strlen(prefix); + size_t classname_len = strlen(classname); + int i = 0, j; + bool first_char = true; + + int offset = package_len != 0 ? 2 : 0; + + for (j = 0; j < classname_len; j++) { + classname[package_len + prefix_len + classname_len + offset - 1 - j] = + classname[classname_len - j - 1]; + } + + if (package_len != 0) { + classname[i++] = '\\'; + for (j = 0; j < package_len; j++) { // php packages are divided by '\'. - if (package_name[j] == '.') { - class_name[i++] = '\\'; + if (package[j] == '.') { + classname[i++] = '\\'; first_char = true; } else if (first_char) { // PHP package uses camel case. - if (package_name[j] < 'A' || package_name[j] > 'Z') { - class_name[i++] = package_name[j] + 'A' - 'a'; + if (package[j] < 'A' || package[j] > 'Z') { + classname[i++] = package[j] + 'A' - 'a'; } else { - class_name[i++] = package_name[j]; + classname[i++] = package[j]; } first_char = false; } else { - class_name[i++] = package_name[j]; + classname[i++] = package[j]; } } - class_name[i++] = '\\'; + classname[i++] = '\\'; } - if (prefix_len > 0) { - strcpy(class_name + i, prefix); - i += prefix_len; - } - - // Submessage is concatenated with its containing messages by '_'. - for (j = message_name_start; j < message_name_start + message_len; j++) { - if (fullname[j] == '.') { - class_name[i++] = '_'; - } else { - class_name[i++] = fullname[j]; - } - } + memcpy(classname + i, prefix, prefix_len); } PHP_METHOD(DescriptorPool, internalAddGeneratedFile) { @@ -455,25 +490,27 @@ PHP_METHOD(DescriptorPool, internalAddGeneratedFile) { * bytes allocated, one for '.', one for trailing 0, and 3 for 'GPB' if \ * given message is google.protobuf.Empty.*/ \ const char *fullname = upb_##def_type_lower##_fullname(def_type_lower); \ - const char *prefix = upb_filedef_phpprefix(files[0]); \ - size_t klass_name_len = strlen(fullname) + 5; \ - if (prefix != NULL) { \ - klass_name_len += strlen(prefix); \ + const char *prefix_given = upb_filedef_phpprefix(files[0]); \ + size_t classname_len = strlen(fullname) + 5; \ + if (prefix_given != NULL) { \ + classname_len += strlen(prefix_given); \ } \ - char *klass_name = ecalloc(sizeof(char), klass_name_len); \ - convert_to_class_name_inplace(klass_name, fullname, prefix, \ - upb_filedef_package(files[0])); \ + char *classname = ecalloc(sizeof(char), classname_len); \ + const char *package = upb_filedef_package(files[0]); \ + classname_no_prefix(fullname, package, classname); \ + const char *prefix = classname_prefix(classname, prefix_given, package); \ + convert_to_class_name_inplace(package, prefix, classname); \ PHP_PROTO_CE_DECLARE pce; \ - if (php_proto_zend_lookup_class(klass_name, strlen(klass_name), &pce) == \ + if (php_proto_zend_lookup_class(classname, strlen(classname), &pce) == \ FAILURE) { \ zend_error(E_ERROR, "Generated message class %s hasn't been defined", \ - klass_name); \ + classname); \ return; \ } else { \ desc->klass = PHP_PROTO_CE_UNREF(pce); \ } \ add_ce_obj(desc->klass, desc_php); \ - efree(klass_name); \ + efree(classname); \ break; \ } diff --git a/php/src/Google/Protobuf/Internal/Message.php b/php/src/Google/Protobuf/Internal/Message.php index 887c86ca..0fb6cdc0 100644 --- a/php/src/Google/Protobuf/Internal/Message.php +++ b/php/src/Google/Protobuf/Internal/Message.php @@ -71,6 +71,7 @@ class Message return; } $pool = DescriptorPool::getGeneratedPool(); + var_dump(get_class($this)); $this->desc = $pool->getDescriptorByClassName(get_class($this)); foreach ($this->desc->getField() as $field) { $setter = $field->getSetter(); diff --git a/php/src/Google/Protobuf/descriptor.php b/php/src/Google/Protobuf/descriptor.php index 2263af6e..9c744a8a 100644 --- a/php/src/Google/Protobuf/descriptor.php +++ b/php/src/Google/Protobuf/descriptor.php @@ -220,20 +220,36 @@ class Descriptor } } +function getClassNamePrefix( + $classname, + $file_proto) +{ + $option = $file_proto->getOptions(); + $prefix = is_null($option) ? "" : $option->getPhpClassPrefix(); + if ($prefix !== "") { + return $prefix; + } + + $reserved_words = array("Empty"); + foreach ($reserved_words as $reserved_word) { + if ($classname === $reserved_word) { + if ($file_proto->getPackage() === "google.protobuf") { + return "GPB"; + } else { + return "PB"; + } + } + } + + return ""; +} + function getClassNameWithoutPackage( $name, $file_proto) { - if ($name === "Empty" && $file_proto->getPackage() === "google.protobuf") { - return "GPBEmpty"; - } else { - $option = $file_proto->getOptions(); - $prefix = is_null($option) ? "" : $option->getPhpClassPrefix(); - // Nested message class names are seperated by '_', and package names - // are seperated by '\'. - return $prefix . implode('_', array_map('ucwords', - explode('.', $name))); - } + $classname = implode('_', array_map('ucwords', explode('.', $name))); + return getClassNamePrefix($classname, $file_proto) . $classname; } function getFullClassName( diff --git a/php/tests/generated_class_test.php b/php/tests/generated_class_test.php index c5dee2d6..554d2bea 100644 --- a/php/tests/generated_class_test.php +++ b/php/tests/generated_class_test.php @@ -865,4 +865,15 @@ class GeneratedClassTest extends TestBase $m->setPrefixMessage($n); $this->assertSame(1, $m->getPrefixMessage()->getA()); } + + ######################################################### + # Test prefix for reserved words. + ######################################################### + + public function testPrefixForReservedWords() + { + $m = new \Foo\TestMessage_Empty(); + $m = new \Foo\PBEmpty(); + $m = new \PrefixEmpty(); + } } diff --git a/php/tests/memory_leak_test.php b/php/tests/memory_leak_test.php index ea7a4c96..361982b5 100644 --- a/php/tests/memory_leak_test.php +++ b/php/tests/memory_leak_test.php @@ -5,11 +5,14 @@ require_once('generated/NoNamespaceEnum.php'); require_once('generated/NoNamespaceMessage.php'); require_once('generated/NoNamespaceMessage_NestedEnum.php'); +require_once('generated/PrefixEmpty.php'); require_once('generated/PrefixTestPrefix.php'); require_once('generated/Bar/TestInclude.php'); +require_once('generated/Foo/PBEmpty.php'); require_once('generated/Foo/TestEnum.php'); require_once('generated/Foo/TestIncludePrefixMessage.php'); require_once('generated/Foo/TestMessage.php'); +require_once('generated/Foo/TestMessage_Empty.php'); require_once('generated/Foo/TestMessage_NestedEnum.php'); require_once('generated/Foo/TestMessage_Sub.php'); require_once('generated/Foo/TestPackedMessage.php'); diff --git a/php/tests/proto/test.proto b/php/tests/proto/test.proto index b6c14866..f0d009c8 100644 --- a/php/tests/proto/test.proto +++ b/php/tests/proto/test.proto @@ -108,6 +108,11 @@ message TestMessage { } NestedEnum optional_nested_enum = 101; + + // Test prefix for reserved words. + message Empty { + int32 a = 1; + } } enum TestEnum { @@ -116,6 +121,11 @@ enum TestEnum { TWO = 2; } +// Test prefix for reserved words. +message Empty { + int32 a = 1; +} + message TestPackedMessage { repeated int32 repeated_int32 = 90 [packed = true]; repeated int64 repeated_int64 = 91 [packed = true]; diff --git a/php/tests/proto/test_prefix.proto b/php/tests/proto/test_prefix.proto index 04582121..9bfbad7f 100644 --- a/php/tests/proto/test_prefix.proto +++ b/php/tests/proto/test_prefix.proto @@ -5,3 +5,8 @@ option php_class_prefix = "Prefix"; message TestPrefix { int32 a = 1; } + +// Test prefix for reserved words. +message Empty { + int32 a = 1; +} diff --git a/src/google/protobuf/compiler/php/php_generator.cc b/src/google/protobuf/compiler/php/php_generator.cc index 32f40b2e..d24e1e5e 100644 --- a/src/google/protobuf/compiler/php/php_generator.cc +++ b/src/google/protobuf/compiler/php/php_generator.cc @@ -49,6 +49,8 @@ const std::string kDescriptorMetadataFile = "GPBMetadata/Google/Protobuf/Internal/Descriptor.php"; const std::string kDescriptorDirName = "Google/Protobuf/Internal"; const std::string kDescriptorPackageName = "Google\\Protobuf\\Internal"; +const char* const kReservedNames[] = {"Empty"}; +const int kReservedNamesSize = 1; namespace google { namespace protobuf { @@ -105,14 +107,31 @@ std::string EnumFullName(const EnumDescriptor* envm, bool is_descriptor) { } template -std::string ClassNamePrefix(const DescriptorType* desc) { - // Empty cannot be php class name. - if (desc->name() == "Empty" && - desc->file()->package() == "google.protobuf") { - return "GPB"; - } else { - return (desc->file()->options()).php_class_prefix(); +std::string ClassNamePrefix(const string& classname, + const DescriptorType* desc) { + const string& prefix = (desc->file()->options()).php_class_prefix(); + if (prefix != "") { + return prefix; + } + + bool is_reserved = false; + + for (int i = 0; i < kReservedNamesSize; i++) { + if (classname == kReservedNames[i]) { + is_reserved = true; + break; + } } + + if (is_reserved) { + if (desc->file()->package() == "google.protobuf") { + return "GPB"; + } else { + return "PB"; + } + } + + return ""; } @@ -124,7 +143,7 @@ std::string FullClassName(const DescriptorType* desc, bool is_descriptor) { classname = containing->name() + '_' + classname; containing = containing->containing_type(); } - classname = ClassNamePrefix(desc) + classname; + classname = ClassNamePrefix(classname, desc) + classname; if (desc->file()->package() == "") { return classname; -- cgit v1.2.3 From 4523c9c233dbb61d03996a5bbe25d1b5aea51f7f Mon Sep 17 00:00:00 2001 From: Paul Yang Date: Thu, 20 Apr 2017 16:55:56 -0700 Subject: Allow proto files to import descriptor.proto (#2995) descriptor.proto uses proto2 syntax, which is not ready for external usage. However, some proto3 files import descriptor.proto and cannot be used. In this PR, all references (We cheated by only removing extensions, which is enough for now. User should avoid using messages defined in descriptor.proto as field type.) to content in descriptor.proto are removed from generated files. Those that import descriptor.proto can be used like other proto files. --- Makefile.am | 1 + php/src/Google/Protobuf/Internal/MapField.php | 1 - php/src/Google/Protobuf/Internal/Message.php | 1 - php/tests/gdb_test.sh | 2 +- php/tests/proto/test_import_descriptor_proto.proto | 14 ++++++++++++ php/tests/well_known_test.php | 10 +++++++-- src/google/protobuf/compiler/php/php_generator.cc | 26 ++++++++++++++++++++++ tests.sh | 1 + 8 files changed, 51 insertions(+), 5 deletions(-) create mode 100644 php/tests/proto/test_import_descriptor_proto.proto (limited to 'php/src/Google/Protobuf') diff --git a/Makefile.am b/Makefile.am index 564bff48..d0f033af 100644 --- a/Makefile.am +++ b/Makefile.am @@ -656,6 +656,7 @@ php_EXTRA_DIST= \ php/tests/map_field_test.php \ php/tests/memory_leak_test.php \ php/tests/php_implementation_test.php \ + php/tests/proto/test_import_descriptor_proto.proto \ php/tests/proto/test_include.proto \ php/tests/proto/test.proto \ php/tests/proto/test_prefix.proto \ diff --git a/php/src/Google/Protobuf/Internal/MapField.php b/php/src/Google/Protobuf/Internal/MapField.php index f65bd9b8..55cc12ce 100644 --- a/php/src/Google/Protobuf/Internal/MapField.php +++ b/php/src/Google/Protobuf/Internal/MapField.php @@ -155,7 +155,6 @@ function checkKey($key_type, &$key) GPBUtil::checkString($key, true); break; default: - var_dump($key_type); trigger_error( "Given type cannot be map key.", E_USER_ERROR); diff --git a/php/src/Google/Protobuf/Internal/Message.php b/php/src/Google/Protobuf/Internal/Message.php index 0fb6cdc0..887c86ca 100644 --- a/php/src/Google/Protobuf/Internal/Message.php +++ b/php/src/Google/Protobuf/Internal/Message.php @@ -71,7 +71,6 @@ class Message return; } $pool = DescriptorPool::getGeneratedPool(); - var_dump(get_class($this)); $this->desc = $pool->getDescriptorByClassName(get_class($this)); foreach ($this->desc->getField() as $field) { $setter = $field->getSetter(); diff --git a/php/tests/gdb_test.sh b/php/tests/gdb_test.sh index 484e2edf..0809bef3 100755 --- a/php/tests/gdb_test.sh +++ b/php/tests/gdb_test.sh @@ -3,7 +3,7 @@ # gdb --args php -dextension=../ext/google/protobuf/modules/protobuf.so `which # phpunit` --bootstrap autoload.php tmp_test.php # -gdb --args php -dextension=../ext/google/protobuf/modules/protobuf.so `which phpunit` --bootstrap autoload.php encode_decode_test.php +gdb --args php -dextension=../ext/google/protobuf/modules/protobuf.so `which phpunit` --bootstrap autoload.php well_known_test.php # # gdb --args php -dextension=../ext/google/protobuf/modules/protobuf.so memory_leak_test.php # diff --git a/php/tests/proto/test_import_descriptor_proto.proto b/php/tests/proto/test_import_descriptor_proto.proto new file mode 100644 index 00000000..2a19940d --- /dev/null +++ b/php/tests/proto/test_import_descriptor_proto.proto @@ -0,0 +1,14 @@ +syntax = "proto3"; + +import "google/protobuf/descriptor.proto"; + +message TestImportDescriptorProto { + extend google.protobuf.MethodOptions { + int32 a = 72295727; + } +} + +extend google.protobuf.MethodOptions { + int32 a = 72295728; +} + diff --git a/php/tests/well_known_test.php b/php/tests/well_known_test.php index 40ff1c8f..0c2aec13 100644 --- a/php/tests/well_known_test.php +++ b/php/tests/well_known_test.php @@ -4,8 +4,14 @@ use Google\Protobuf\GPBEmpty; class WellKnownTest extends PHPUnit_Framework_TestCase { - public function testNone() { - $msg = new GPBEmpty(); + public function testNone() + { + $msg = new GPBEmpty(); + } + + public function testImportDescriptorProto() + { + $msg = new TestImportDescriptorProto(); } } diff --git a/src/google/protobuf/compiler/php/php_generator.cc b/src/google/protobuf/compiler/php/php_generator.cc index d24e1e5e..4d475b1f 100644 --- a/src/google/protobuf/compiler/php/php_generator.cc +++ b/src/google/protobuf/compiler/php/php_generator.cc @@ -674,6 +674,12 @@ void GenerateAddFileToPool(const FileDescriptor* file, bool is_descriptor, } else { for (int i = 0; i < file->dependency_count(); i++) { const std::string& name = file->dependency(i)->name(); + // Currently, descriptor.proto is not ready for external usage. Skip to + // import it for now, so that its dependencies can still work as long as + // they don't use protos defined in descriptor.proto. + if (name == kDescriptorFile) { + continue; + } std::string dependency_filename = GeneratedMetadataFileName(name, is_descriptor); printer->Print( @@ -685,6 +691,26 @@ void GenerateAddFileToPool(const FileDescriptor* file, bool is_descriptor, FileDescriptorSet files; FileDescriptorProto* file_proto = files.add_file(); file->CopyTo(file_proto); + + // Filter out descriptor.proto as it cannot be depended on for now. + RepeatedPtrField* dependency = file_proto->mutable_dependency(); + for (RepeatedPtrField::iterator it = dependency->begin(); + it != dependency->end(); ++it) { + if (*it != kDescriptorFile) { + dependency->erase(it); + break; + } + } + + // Filter out all extensions, since we do not support extension yet. + file_proto->clear_extension(); + RepeatedPtrField* message_type = + file_proto->mutable_message_type(); + for (RepeatedPtrField::iterator it = message_type->begin(); + it != message_type->end(); ++it) { + it->clear_extension(); + } + string files_data; files.SerializeToString(&files_data); diff --git a/tests.sh b/tests.sh index 8c56172d..edb37da7 100755 --- a/tests.sh +++ b/tests.sh @@ -362,6 +362,7 @@ generate_php_test_proto() { ../../src/protoc --php_out=generated proto/test.proto proto/test_include.proto proto/test_no_namespace.proto proto/test_prefix.proto pushd ../../src ./protoc --php_out=../php/tests/generated google/protobuf/empty.proto + ./protoc --php_out=../php/tests/generated -I../php/tests -I. ../php/tests/proto/test_import_descriptor_proto.proto popd popd } -- cgit v1.2.3 From 6fff091c49359ffd8550fc8228c79740b504a4fe Mon Sep 17 00:00:00 2001 From: Paul Yang Date: Fri, 21 Apr 2017 15:00:00 -0700 Subject: Throw exception when parsing invalid data. (#3000) --- Makefile.am | 1 + php/ext/google/protobuf/encode_decode.c | 10 +- .../Protobuf/Internal/GPBDecodeException.php | 47 +++++ php/src/Google/Protobuf/Internal/InputStream.php | 17 +- php/src/Google/Protobuf/Internal/Message.php | 73 ++++---- php/tests/encode_decode_test.php | 198 +++++++++++++++++++++ 6 files changed, 297 insertions(+), 49 deletions(-) create mode 100644 php/src/Google/Protobuf/Internal/GPBDecodeException.php (limited to 'php/src/Google/Protobuf') diff --git a/Makefile.am b/Makefile.am index d0f033af..3b57b585 100644 --- a/Makefile.am +++ b/Makefile.am @@ -646,6 +646,7 @@ php_EXTRA_DIST= \ php/src/Google/Protobuf/Internal/EnumBuilderContext.php \ php/src/Google/Protobuf/Internal/GPBUtil.php \ php/src/Google/Protobuf/Internal/FieldOptions_CType.php \ + php/src/Google/Protobuf/Internal/GPBDecodeException.php \ php/src/Google/Protobuf/descriptor.php \ php/src/GPBMetadata/Google/Protobuf/Internal/Descriptor.php \ php/tests/array_test.php \ diff --git a/php/ext/google/protobuf/encode_decode.c b/php/ext/google/protobuf/encode_decode.c index 06dc1195..28bf18f4 100644 --- a/php/ext/google/protobuf/encode_decode.c +++ b/php/ext/google/protobuf/encode_decode.c @@ -261,13 +261,6 @@ static void* appendbytes_handler(void *closure, #endif } - static bool int32_handler(void* closure, const void* hd, - int32_t val) { - MessageHeader* msg = (MessageHeader*)closure; - const size_t *ofs = hd; - DEREF(message_data(msg), *ofs, int32_t) = val; - return true; - } // Handlers that append primitive values to a repeated field. #define DEFINE_SINGULAR_HANDLER(type, ctype) \ static bool type##_handler(void* closure, const void* hd, \ @@ -279,7 +272,7 @@ static void* appendbytes_handler(void *closure, } DEFINE_SINGULAR_HANDLER(bool, bool) -// DEFINE_SINGULAR_HANDLER(int32, int32_t) +DEFINE_SINGULAR_HANDLER(int32, int32_t) DEFINE_SINGULAR_HANDLER(uint32, uint32_t) DEFINE_SINGULAR_HANDLER(float, float) DEFINE_SINGULAR_HANDLER(int64, int64_t) @@ -1435,6 +1428,7 @@ PHP_METHOD(Message, mergeFromString) { char *data = NULL; PHP_PROTO_SIZE data_len; + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s", &data, &data_len) == FAILURE) { return; 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 @@ += 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); } } diff --git a/php/tests/encode_decode_test.php b/php/tests/encode_decode_test.php index 94adf793..288df569 100644 --- a/php/tests/encode_decode_test.php +++ b/php/tests/encode_decode_test.php @@ -211,6 +211,204 @@ class EncodeDecodeTest extends TestBase $this->assertEquals(-1, $m->getOptionalInt32()); } + /** + * @expectedException Exception + */ + public function testDecodeInvalidInt32() + { + $m = new TestMessage(); + $m->mergeFromString(hex2bin('08')); + } + + /** + * @expectedException Exception + */ + public function testDecodeInvalidSubMessage() + { + $m = new TestMessage(); + $m->mergeFromString(hex2bin('9A010108')); + } + + /** + * @expectedException Exception + */ + public function testDecodeInvalidInt64() + { + $m = new TestMessage(); + $m->mergeFromString(hex2bin('10')); + } + + /** + * @expectedException Exception + */ + public function testDecodeInvalidUInt32() + { + $m = new TestMessage(); + $m->mergeFromString(hex2bin('18')); + } + + /** + * @expectedException Exception + */ + public function testDecodeInvalidUInt64() + { + $m = new TestMessage(); + $m->mergeFromString(hex2bin('20')); + } + + /** + * @expectedException Exception + */ + public function testDecodeInvalidSInt32() + { + $m = new TestMessage(); + $m->mergeFromString(hex2bin('28')); + } + + /** + * @expectedException Exception + */ + public function testDecodeInvalidSInt64() + { + $m = new TestMessage(); + $m->mergeFromString(hex2bin('30')); + } + + /** + * @expectedException Exception + */ + public function testDecodeInvalidFixed32() + { + $m = new TestMessage(); + $m->mergeFromString(hex2bin('3D')); + } + + /** + * @expectedException Exception + */ + public function testDecodeInvalidFixed64() + { + $m = new TestMessage(); + $m->mergeFromString(hex2bin('41')); + } + + /** + * @expectedException Exception + */ + public function testDecodeInvalidSFixed32() + { + $m = new TestMessage(); + $m->mergeFromString(hex2bin('4D')); + } + + /** + * @expectedException Exception + */ + public function testDecodeInvalidSFixed64() + { + $m = new TestMessage(); + $m->mergeFromString(hex2bin('51')); + } + + /** + * @expectedException Exception + */ + public function testDecodeInvalidFloat() + { + $m = new TestMessage(); + $m->mergeFromString(hex2bin('5D')); + } + + /** + * @expectedException Exception + */ + public function testDecodeInvalidDouble() + { + $m = new TestMessage(); + $m->mergeFromString(hex2bin('61')); + } + + /** + * @expectedException Exception + */ + public function testDecodeInvalidBool() + { + $m = new TestMessage(); + $m->mergeFromString(hex2bin('68')); + } + + /** + * @expectedException Exception + */ + public function testDecodeInvalidStringLengthMiss() + { + $m = new TestMessage(); + $m->mergeFromString(hex2bin('72')); + } + + /** + * @expectedException Exception + */ + public function testDecodeInvalidStringDataMiss() + { + $m = new TestMessage(); + $m->mergeFromString(hex2bin('7201')); + } + + /** + * @expectedException Exception + */ + public function testDecodeInvalidBytesLengthMiss() + { + $m = new TestMessage(); + $m->mergeFromString(hex2bin('7A')); + } + + /** + * @expectedException Exception + */ + public function testDecodeInvalidBytesDataMiss() + { + $m = new TestMessage(); + $m->mergeFromString(hex2bin('7A01')); + } + + /** + * @expectedException Exception + */ + public function testDecodeInvalidEnum() + { + $m = new TestMessage(); + $m->mergeFromString(hex2bin('8001')); + } + + /** + * @expectedException Exception + */ + public function testDecodeInvalidMessageLengthMiss() + { + $m = new TestMessage(); + $m->mergeFromString(hex2bin('8A01')); + } + + /** + * @expectedException Exception + */ + public function testDecodeInvalidMessageDataMiss() + { + $m = new TestMessage(); + $m->mergeFromString(hex2bin('8A0101')); + } + + /** + * @expectedException Exception + */ + public function testDecodeInvalidPackedMessageLength() + { + $m = new TestPackedMessage(); + $m->mergeFromString(hex2bin('D205')); + } + # TODO(teboring): Add test back when php implementation is ready for json # encode/decode. # public function testJsonEncode() -- cgit v1.2.3 From fba2acd72e8cbf138912295df227ee2c914158c3 Mon Sep 17 00:00:00 2001 From: Paul Yang Date: Mon, 24 Apr 2017 12:40:37 -0700 Subject: Add nested enum descriptor in php rumtime. (#3009) --- php/src/Google/Protobuf/Internal/DescriptorPool.php | 3 +++ php/src/Google/Protobuf/descriptor.php | 6 ++++++ php/tests/generated_class_test.php | 3 ++- 3 files changed, 11 insertions(+), 1 deletion(-) (limited to 'php/src/Google/Protobuf') diff --git a/php/src/Google/Protobuf/Internal/DescriptorPool.php b/php/src/Google/Protobuf/Internal/DescriptorPool.php index 1ef403cf..2c00dfb6 100644 --- a/php/src/Google/Protobuf/Internal/DescriptorPool.php +++ b/php/src/Google/Protobuf/Internal/DescriptorPool.php @@ -95,6 +95,9 @@ class DescriptorPool foreach ($descriptor->getNestedType() as $nested_type) { $this->addDescriptor($nested_type); } + foreach ($descriptor->getEnumType() as $enum_type) { + $this->addEnumDescriptor($enum_type); + } } public function addEnumDescriptor($descriptor) diff --git a/php/src/Google/Protobuf/descriptor.php b/php/src/Google/Protobuf/descriptor.php index 9c744a8a..fb69eda0 100644 --- a/php/src/Google/Protobuf/descriptor.php +++ b/php/src/Google/Protobuf/descriptor.php @@ -210,6 +210,12 @@ class Descriptor $nested_proto, $file_proto, $message_name_without_package)); } + // Handle nested enum. + foreach ($proto->getEnumType() as $enum_proto) { + $desc->addEnumType(EnumDescriptor::buildFromProto( + $enum_proto, $file_proto, $message_name_without_package)); + } + // Handle oneof fields. foreach ($proto->getOneofDecl() as $oneof_proto) { $desc->addOneofDecl( diff --git a/php/tests/generated_class_test.php b/php/tests/generated_class_test.php index 554d2bea..21ee8490 100644 --- a/php/tests/generated_class_test.php +++ b/php/tests/generated_class_test.php @@ -839,7 +839,8 @@ class GeneratedClassTest extends TestBase public function testMessageWithoutNamespace() { $m = new TestMessage(); - $m->setOptionalNoNamespaceMessage(new NoNameSpaceMessage()); + $sub = new NoNameSpaceMessage(); + $m->setOptionalNoNamespaceMessage($sub); $m->getRepeatedNoNamespaceMessage()[] = new NoNameSpaceMessage(); $n = new NoNamespaceMessage(); -- cgit v1.2.3