diff options
-rw-r--r-- | php/ext/google/protobuf/encode_decode.c | 5 | ||||
-rw-r--r-- | php/ext/google/protobuf/map.c | 44 | ||||
-rw-r--r-- | php/ext/google/protobuf/message.c | 59 | ||||
-rw-r--r-- | php/ext/google/protobuf/protobuf.c | 37 | ||||
-rw-r--r-- | php/ext/google/protobuf/protobuf.h | 11 | ||||
-rwxr-xr-x | php/tests/gdb_test.sh | 12 | ||||
-rw-r--r-- | php/tests/memory_leak_test.php | 38 | ||||
-rwxr-xr-x | php/tests/test.sh | 20 | ||||
-rw-r--r-- | protoc-artifacts/pom.xml | 2 | ||||
-rw-r--r-- | ruby/Rakefile | 4 | ||||
-rw-r--r-- | ruby/google-protobuf.gemspec | 2 | ||||
-rw-r--r-- | src/google/protobuf/util/internal/datapiece.cc | 7 | ||||
-rw-r--r-- | src/google/protobuf/util/internal/datapiece.h | 3 | ||||
-rw-r--r-- | src/google/protobuf/util/internal/proto_writer.cc | 8 | ||||
-rw-r--r-- | src/google/protobuf/util/internal/proto_writer.h | 2 | ||||
-rw-r--r-- | src/google/protobuf/util/json_util_test.cc | 58 | ||||
-rwxr-xr-x | tests.sh | 16 |
17 files changed, 261 insertions, 67 deletions
diff --git a/php/ext/google/protobuf/encode_decode.c b/php/ext/google/protobuf/encode_decode.c index b98121bb..a8c47f4d 100644 --- a/php/ext/google/protobuf/encode_decode.c +++ b/php/ext/google/protobuf/encode_decode.c @@ -484,11 +484,11 @@ static void map_slot_init(void* memory, upb_fieldtype_t type, zval* cache) { // Store zval** in memory in order to be consistent with the layout of // singular fields. zval** holder = ALLOC(zval*); + *(zval***)memory = holder; zval* tmp; MAKE_STD_ZVAL(tmp); PHP_PROTO_ZVAL_STRINGL(tmp, "", 0, 1); *holder = tmp; - *(zval***)memory = holder; #else *(zval**)memory = cache; PHP_PROTO_ZVAL_STRINGL(*(zval**)memory, "", 0, 1); @@ -521,7 +521,7 @@ static void map_slot_uninit(void* memory, upb_fieldtype_t type) { case UPB_TYPE_BYTES: { #if PHP_MAJOR_VERSION < 7 zval** holder = *(zval***)memory; - php_proto_zval_ptr_dtor(*holder); + zval_ptr_dtor(holder); FREE(holder); #else php_proto_zval_ptr_dtor(*(zval**)memory); @@ -1621,6 +1621,7 @@ static void discard_unknown_fields(MessageHeader* msg) { stringsink* unknown = DEREF(message_data(msg), 0, stringsink*); if (unknown != NULL) { stringsink_uninit(unknown); + FREE(unknown); DEREF(message_data(msg), 0, stringsink*) = NULL; } diff --git a/php/ext/google/protobuf/map.c b/php/ext/google/protobuf/map.c index 2680b547..ab8a518a 100644 --- a/php/ext/google/protobuf/map.c +++ b/php/ext/google/protobuf/map.c @@ -293,13 +293,46 @@ static bool map_field_read_dimension(zval *object, zval *key, int type, } } +static bool map_index_unset(Map *intern, const char* keyval, int length) { + upb_value old_value; + if (upb_strtable_remove2(&intern->table, keyval, length, &old_value)) { + switch (intern->value_type) { + case UPB_TYPE_MESSAGE: { +#if PHP_MAJOR_VERSION < 7 + zval_ptr_dtor(upb_value_memory(&old_value)); +#else + zend_object* object = *(zend_object**)upb_value_memory(&old_value); + if(--GC_REFCOUNT(object) == 0) { + zend_objects_store_del(object); + } +#endif + break; + } + case UPB_TYPE_STRING: + case UPB_TYPE_BYTES: { +#if PHP_MAJOR_VERSION < 7 + zval_ptr_dtor(upb_value_memory(&old_value)); +#else + zend_string* object = *(zend_string**)upb_value_memory(&old_value); + zend_string_release(object); +#endif + break; + } + default: + break; + } + } +} + bool map_index_set(Map *intern, const char* keyval, int length, upb_value v) { // Replace any existing value by issuing a 'remove' operation first. - upb_strtable_remove2(&intern->table, keyval, length, NULL); + map_index_unset(intern, keyval, length); + if (!upb_strtable_insert2(&intern->table, keyval, length, v)) { zend_error(E_USER_ERROR, "Could not insert into table"); return false; } + return true; } @@ -326,12 +359,7 @@ static void map_field_write_dimension(zval *object, zval *key, v.ctype = UPB_CTYPE_UINT64; #endif - // Replace any existing value by issuing a 'remove' operation first. - upb_strtable_remove2(&intern->table, keyval, length, NULL); - if (!upb_strtable_insert2(&intern->table, keyval, length, v)) { - zend_error(E_USER_ERROR, "Could not insert into table"); - return; - } + map_index_set(intern, keyval, length, v); } static bool map_field_unset_dimension(zval *object, zval *key TSRMLS_DC) { @@ -348,7 +376,7 @@ static bool map_field_unset_dimension(zval *object, zval *key TSRMLS_DC) { v.ctype = UPB_CTYPE_UINT64; #endif - upb_strtable_remove2(&intern->table, keyval, length, &v); + map_index_unset(intern, keyval, length); return true; } diff --git a/php/ext/google/protobuf/message.c b/php/ext/google/protobuf/message.c index 24472682..e801f4a0 100644 --- a/php/ext/google/protobuf/message.c +++ b/php/ext/google/protobuf/message.c @@ -292,7 +292,9 @@ PHP_METHOD(Message, clear) { Descriptor* desc = msg->descriptor; zend_class_entry* ce = desc->klass; + zend_object_std_dtor(&msg->std TSRMLS_CC); object_properties_init(&msg->std, ce); + layout_init(desc->layout, message_data(msg), &msg->std TSRMLS_CC); } @@ -445,8 +447,7 @@ static void init_file_wrappers(TSRMLS_D); // Define file init functions static void init_file_any(TSRMLS_D) { - static bool is_initialized = false; - if (is_initialized) return; + if (is_inited_file_any) return; init_generated_pool_once(TSRMLS_C); const char* generated_file = "0acd010a19676f6f676c652f70726f746f6275662f616e792e70726f746f" @@ -461,12 +462,11 @@ static void init_file_any(TSRMLS_D) { hex_to_binary(generated_file, &binary, &binary_len); internal_add_generated_file(binary, binary_len, generated_pool TSRMLS_CC); FREE(binary); - is_initialized = true; + is_inited_file_any = true; } static void init_file_api(TSRMLS_D) { - static bool is_initialized = false; - if (is_initialized) return; + if (is_inited_file_api) return; init_file_source_context(TSRMLS_C); init_file_type(TSRMLS_C); init_generated_pool_once(TSRMLS_C); @@ -502,12 +502,11 @@ static void init_file_api(TSRMLS_D) { hex_to_binary(generated_file, &binary, &binary_len); internal_add_generated_file(binary, binary_len, generated_pool TSRMLS_CC); FREE(binary); - is_initialized = true; + is_inited_file_api = true; } static void init_file_duration(TSRMLS_D) { - static bool is_initialized = false; - if (is_initialized) return; + if (is_inited_file_duration) return; init_generated_pool_once(TSRMLS_C); const char* generated_file = "0ae3010a1e676f6f676c652f70726f746f6275662f6475726174696f6e2e" @@ -523,12 +522,11 @@ static void init_file_duration(TSRMLS_D) { hex_to_binary(generated_file, &binary, &binary_len); internal_add_generated_file(binary, binary_len, generated_pool TSRMLS_CC); FREE(binary); - is_initialized = true; + is_inited_file_duration = true; } static void init_file_field_mask(TSRMLS_D) { - static bool is_initialized = false; - if (is_initialized) return; + if (is_inited_file_field_mask) return; init_generated_pool_once(TSRMLS_C); const char* generated_file = "0ae3010a20676f6f676c652f70726f746f6275662f6669656c645f6d6173" @@ -544,12 +542,11 @@ static void init_file_field_mask(TSRMLS_D) { hex_to_binary(generated_file, &binary, &binary_len); internal_add_generated_file(binary, binary_len, generated_pool TSRMLS_CC); FREE(binary); - is_initialized = true; + is_inited_file_field_mask = true; } static void init_file_empty(TSRMLS_D) { - static bool is_initialized = false; - if (is_initialized) return; + if (is_inited_file_empty) return; init_generated_pool_once(TSRMLS_C); const char* generated_file = "0ab7010a1b676f6f676c652f70726f746f6275662f656d7074792e70726f" @@ -564,12 +561,11 @@ static void init_file_empty(TSRMLS_D) { hex_to_binary(generated_file, &binary, &binary_len); internal_add_generated_file(binary, binary_len, generated_pool TSRMLS_CC); FREE(binary); - is_initialized = true; + is_inited_file_empty = true; } static void init_file_source_context(TSRMLS_D) { - static bool is_initialized = false; - if (is_initialized) return; + if (is_inited_file_source_context) return; init_generated_pool_once(TSRMLS_C); const char* generated_file = "0afb010a24676f6f676c652f70726f746f6275662f736f757263655f636f" @@ -586,12 +582,11 @@ static void init_file_source_context(TSRMLS_D) { hex_to_binary(generated_file, &binary, &binary_len); internal_add_generated_file(binary, binary_len, generated_pool TSRMLS_CC); FREE(binary); - is_initialized = true; + is_inited_file_source_context = true; } static void init_file_struct(TSRMLS_D) { - static bool is_initialized = false; - if (is_initialized) return; + if (is_inited_file_struct) return; init_generated_pool_once(TSRMLS_C); const char* generated_file = "0a81050a1c676f6f676c652f70726f746f6275662f7374727563742e7072" @@ -621,12 +616,11 @@ static void init_file_struct(TSRMLS_D) { hex_to_binary(generated_file, &binary, &binary_len); internal_add_generated_file(binary, binary_len, generated_pool TSRMLS_CC); FREE(binary); - is_initialized = true; + is_inited_file_struct = true; } static void init_file_timestamp(TSRMLS_D) { - static bool is_initialized = false; - if (is_initialized) return; + if (is_inited_file_timestamp) return; init_generated_pool_once(TSRMLS_C); const char* generated_file = "0ae7010a1f676f6f676c652f70726f746f6275662f74696d657374616d70" @@ -642,12 +636,11 @@ static void init_file_timestamp(TSRMLS_D) { hex_to_binary(generated_file, &binary, &binary_len); internal_add_generated_file(binary, binary_len, generated_pool TSRMLS_CC); FREE(binary); - is_initialized = true; + is_inited_file_timestamp = true; } static void init_file_type(TSRMLS_D) { - static bool is_initialized = false; - if (is_initialized) return; + if (is_inited_file_type) return; init_file_any(TSRMLS_C); init_file_source_context(TSRMLS_C); init_generated_pool_once(TSRMLS_C); @@ -711,12 +704,11 @@ static void init_file_type(TSRMLS_D) { hex_to_binary(generated_file, &binary, &binary_len); internal_add_generated_file(binary, binary_len, generated_pool TSRMLS_CC); FREE(binary); - is_initialized = true; + is_inited_file_type = true; } static void init_file_wrappers(TSRMLS_D) { - static bool is_initialized = false; - if (is_initialized) return; + if (is_inited_file_wrappers) return; init_generated_pool_once(TSRMLS_C); const char* generated_file = "0abf030a1e676f6f676c652f70726f746f6275662f77726170706572732e" @@ -739,7 +731,7 @@ static void init_file_wrappers(TSRMLS_D) { hex_to_binary(generated_file, &binary, &binary_len); internal_add_generated_file(binary, binary_len, generated_pool TSRMLS_CC); FREE(binary); - is_initialized = true; + is_inited_file_wrappers = true; } // ----------------------------------------------------------------------------- @@ -928,6 +920,7 @@ PHP_METHOD(Any, unpack) { PHP_PROTO_FAKE_SCOPE_BEGIN(any_type); zval* type_url_php = php_proto_message_read_property( getThis(), &type_url_member PHP_PROTO_TSRMLS_CC); + zval_dtor(&type_url_member); PHP_PROTO_FAKE_SCOPE_END; // Get fully-qualified name from type url. @@ -963,6 +956,7 @@ PHP_METHOD(Any, unpack) { PHP_PROTO_FAKE_SCOPE_RESTART(any_type); zval* value = php_proto_message_read_property( getThis(), &value_member PHP_PROTO_TSRMLS_CC); + zval_dtor(&value_member); PHP_PROTO_FAKE_SCOPE_END; merge_from_string(Z_STRVAL_P(value), Z_STRLEN_P(value), desc, msg); @@ -991,6 +985,8 @@ PHP_METHOD(Any, pack) { PHP_PROTO_FAKE_SCOPE_BEGIN(any_type); message_handlers->write_property(getThis(), &member, &data, NULL PHP_PROTO_TSRMLS_CC); + zval_dtor(&data); + zval_dtor(&member); PHP_PROTO_FAKE_SCOPE_END; // Set type url. @@ -1008,6 +1004,8 @@ PHP_METHOD(Any, pack) { PHP_PROTO_FAKE_SCOPE_RESTART(any_type); message_handlers->write_property(getThis(), &member, &type_url_php, NULL PHP_PROTO_TSRMLS_CC); + zval_dtor(&type_url_php); + zval_dtor(&member); PHP_PROTO_FAKE_SCOPE_END; FREE(type_url); } @@ -1040,6 +1038,7 @@ PHP_METHOD(Any, is) { PHP_PROTO_FAKE_SCOPE_BEGIN(any_type); zval* value = php_proto_message_read_property(getThis(), &member PHP_PROTO_TSRMLS_CC); + zval_dtor(&member); PHP_PROTO_FAKE_SCOPE_END; // Compare two type url. diff --git a/php/ext/google/protobuf/protobuf.c b/php/ext/google/protobuf/protobuf.c index daebb460..da00302f 100644 --- a/php/ext/google/protobuf/protobuf.c +++ b/php/ext/google/protobuf/protobuf.c @@ -145,6 +145,21 @@ PHP_PROTO_HASHTABLE_VALUE get_proto_obj(const char* proto) { } // ----------------------------------------------------------------------------- +// Well Known Types. +// ----------------------------------------------------------------------------- + +bool is_inited_file_any; +bool is_inited_file_api; +bool is_inited_file_duration; +bool is_inited_file_field_mask; +bool is_inited_file_empty; +bool is_inited_file_source_context; +bool is_inited_file_struct; +bool is_inited_file_timestamp; +bool is_inited_file_type; +bool is_inited_file_wrappers; + +// ----------------------------------------------------------------------------- // Reserved Name. // ----------------------------------------------------------------------------- @@ -250,6 +265,17 @@ static PHP_RINIT_FUNCTION(protobuf) { generated_pool_php = NULL; internal_generated_pool_php = NULL; + is_inited_file_any = false; + is_inited_file_api = false; + is_inited_file_duration = false; + is_inited_file_field_mask = false; + is_inited_file_empty = false; + is_inited_file_source_context = false; + is_inited_file_struct = false; + is_inited_file_timestamp = false; + is_inited_file_type = false; + is_inited_file_wrappers = false; + return 0; } @@ -288,6 +314,17 @@ static PHP_RSHUTDOWN_FUNCTION(protobuf) { } #endif + is_inited_file_any = true; + is_inited_file_api = true; + is_inited_file_duration = true; + is_inited_file_field_mask = true; + is_inited_file_empty = true; + is_inited_file_source_context = true; + is_inited_file_struct = true; + is_inited_file_timestamp = true; + is_inited_file_type = true; + is_inited_file_wrappers = true; + return 0; } diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index 6ab0f134..be1de5cf 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -1172,6 +1172,17 @@ extern zend_class_entry* oneof_descriptor_type; // Well Known Type. // ----------------------------------------------------------------------------- +extern bool is_inited_file_any; +extern bool is_inited_file_api; +extern bool is_inited_file_duration; +extern bool is_inited_file_field_mask; +extern bool is_inited_file_empty; +extern bool is_inited_file_source_context; +extern bool is_inited_file_struct; +extern bool is_inited_file_timestamp; +extern bool is_inited_file_type; +extern bool is_inited_file_wrappers; + PHP_METHOD(GPBMetadata_Any, initOnce); PHP_METHOD(GPBMetadata_Api, initOnce); PHP_METHOD(GPBMetadata_Duration, initOnce); diff --git a/php/tests/gdb_test.sh b/php/tests/gdb_test.sh index 484e2edf..a5f6306e 100755 --- a/php/tests/gdb_test.sh +++ b/php/tests/gdb_test.sh @@ -1,10 +1,18 @@ #!/bin/bash +VERSION=$1 + +export PATH=/usr/local/php-$VERSION/bin:$PATH +export C_INCLUDE_PATH=/usr/local/php-$VERSION/include/php/main:/usr/local/php-$VERSION/include/php:$C_INCLUDE_PATH +export CPLUS_INCLUDE_PATH=/usr/local/php-$VERSION/include/php/main:/usr/local/php-$VERSION/include/php:$CPLUS_INCLUDE_PATH + +php -i | grep "Configuration" + # 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 encode_decode_test.php # -# gdb --args php -dextension=../ext/google/protobuf/modules/protobuf.so memory_leak_test.php +gdb --args php -dextension=../ext/google/protobuf/modules/protobuf.so memory_leak_test.php # # USE_ZEND_ALLOC=0 valgrind --leak-check=yes php -dextension=../ext/google/protobuf/modules/protobuf.so memory_leak_test.php diff --git a/php/tests/memory_leak_test.php b/php/tests/memory_leak_test.php index 507635e7..ad55d578 100644 --- a/php/tests/memory_leak_test.php +++ b/php/tests/memory_leak_test.php @@ -50,6 +50,13 @@ $to->mergeFromString($data); TestUtil::assertTestMessage($to); +$from = new TestMessage(); +TestUtil::setTestMessage2($from); + +$data = $from->serializeToString(); + +$to->mergeFromString($data); + // TODO(teboring): This causes following tests fail in php7. # $from->setRecursive($from); @@ -104,7 +111,7 @@ assert(1 === $n->getOneofMessage()->getA()); $m = new TestMessage(); $m->mergeFromString(hex2bin('F80601')); -assert('F80601', bin2hex($m->serializeToString())); +assert('f80601' === bin2hex($m->serializeToString())); // Test create repeated field via array. $str_arr = array("abc"); @@ -142,13 +149,32 @@ $from = new \Google\Protobuf\Value(); $from->setNumberValue(1); assert(1, $from->getNumberValue()); +// Test discard unknown in message. +$m = new TestMessage(); +$from = hex2bin('F80601'); +$m->mergeFromString($from); +$m->discardUnknownFields(); +$to = $m->serializeToString(); +assert("" === bin2hex($to)); + +// Test clear +$m = new TestMessage(); +TestUtil::setTestMessage($m); +$m->clear(); + +// Test unset map element +$m = new TestMessage(); +$map = $m->getMapStringString(); +$map[1] = 1; +unset($map[1]); + // Test descriptor $pool = \Google\Protobuf\DescriptorPool::getGeneratedPool(); $desc = $pool->getDescriptorByClassName("\Foo\TestMessage"); $field = $desc->getField(1); -# $from = new TestMessage(); -# $to = new TestMessage(); -# TestUtil::setTestMessage($from); -# $to->mergeFrom($from); -# TestUtil::assertTestMessage($to); +$from = new TestMessage(); +$to = new TestMessage(); +TestUtil::setTestMessage($from); +$to->mergeFrom($from); +TestUtil::assertTestMessage($to); diff --git a/php/tests/test.sh b/php/tests/test.sh index c35372d3..2983fe99 100755 --- a/php/tests/test.sh +++ b/php/tests/test.sh @@ -1,5 +1,11 @@ #!/bin/bash +VERSION=$1 + +export PATH=/usr/local/php-$VERSION/bin:$PATH +export C_INCLUDE_PATH=/usr/local/php-$VERSION/include/php/main:/usr/local/php-$VERSION/include/php:$C_INCLUDE_PATH +export CPLUS_INCLUDE_PATH=/usr/local/php-$VERSION/include/php/main:/usr/local/php-$VERSION/include/php:$CPLUS_INCLUDE_PATH + # Compile c extension pushd ../ext/google/protobuf/ make clean || true @@ -15,7 +21,7 @@ do echo "****************************" echo "* $t" echo "****************************" - php -dextension=../ext/google/protobuf/modules/protobuf.so `which phpunit` --bootstrap autoload.php $t + # php -dextension=../ext/google/protobuf/modules/protobuf.so `which phpunit` --bootstrap autoload.php $t echo "" done @@ -25,3 +31,15 @@ done export ZEND_DONT_UNLOAD_MODULES=1 export USE_ZEND_ALLOC=0 valgrind --leak-check=yes php -dextension=../ext/google/protobuf/modules/protobuf.so memory_leak_test.php + +# TODO(teboring): Only for debug (phpunit has memory leak which blocks this beging used by +# regresssion test.) + +# for t in "${tests[@]}" +# do +# echo "****************************" +# echo "* $t (memory leak)" +# echo "****************************" +# valgrind --leak-check=yes php -dextension=../ext/google/protobuf/modules/protobuf.so `which phpunit` --bootstrap autoload.php $t +# echo "" +# done diff --git a/protoc-artifacts/pom.xml b/protoc-artifacts/pom.xml index 0f9dd9f8..90323531 100644 --- a/protoc-artifacts/pom.xml +++ b/protoc-artifacts/pom.xml @@ -10,7 +10,7 @@ </parent> <groupId>com.google.protobuf</groupId> <artifactId>protoc</artifactId> - <version>3.5.1</version> + <version>3.5.1-1</version> <packaging>pom</packaging> <name>Protobuf Compiler</name> <description> diff --git a/ruby/Rakefile b/ruby/Rakefile index e30a75a3..7aecfe85 100644 --- a/ruby/Rakefile +++ b/ruby/Rakefile @@ -64,13 +64,13 @@ else task 'gem:windows' do require 'rake_compiler_dock' - RakeCompilerDock.sh "bundle && IN_DOCKER=true rake cross native gem RUBY_CC_VERSION=2.4.0:2.3.0:2.2.2:2.1.5:2.0.0" + RakeCompilerDock.sh "bundle && IN_DOCKER=true rake cross native gem RUBY_CC_VERSION=2.5.0:2.4.0:2.3.0:2.2.2:2.1.6:2.0.0" end if RUBY_PLATFORM =~ /darwin/ task 'gem:native' do system "rake genproto" - system "rake cross native gem RUBY_CC_VERSION=2.4.0:2.3.0:2.2.2:2.1.5:2.0.0" + system "rake cross native gem RUBY_CC_VERSION=2.5.0:2.4.0:2.3.0:2.2.2:2.1.6:2.0.0" end else task 'gem:native' => [:genproto, 'gem:windows'] diff --git a/ruby/google-protobuf.gemspec b/ruby/google-protobuf.gemspec index 998952d0..8fcf85c7 100644 --- a/ruby/google-protobuf.gemspec +++ b/ruby/google-protobuf.gemspec @@ -1,6 +1,6 @@ Gem::Specification.new do |s| s.name = "google-protobuf" - s.version = "3.5.1" + s.version = "3.5.1.2" s.licenses = ["BSD-3-Clause"] s.summary = "Protocol Buffers" s.description = "Protocol Buffers are Google's data interchange format." diff --git a/src/google/protobuf/util/internal/datapiece.cc b/src/google/protobuf/util/internal/datapiece.cc index 213c2c40..eb54faa4 100644 --- a/src/google/protobuf/util/internal/datapiece.cc +++ b/src/google/protobuf/util/internal/datapiece.cc @@ -272,7 +272,8 @@ StatusOr<string> DataPiece::ToBytes() const { } StatusOr<int> DataPiece::ToEnum(const google::protobuf::Enum* enum_type, - bool use_lower_camel_for_enums) const { + bool use_lower_camel_for_enums, + bool ignore_unknown_enum_values) const { if (type_ == TYPE_NULL) return google::protobuf::NULL_VALUE; if (type_ == TYPE_STRING) { @@ -305,6 +306,10 @@ StatusOr<int> DataPiece::ToEnum(const google::protobuf::Enum* enum_type, value = FindEnumValueByNameWithoutUnderscoreOrNull(enum_type, enum_name); if (value != NULL) return value->number(); } + + // If ignore_unknown_enum_values is true an unknown enum value is treated + // as the default + if (ignore_unknown_enum_values) return enum_type->enumvalue(0).number(); } else { // We don't need to check whether the value is actually declared in the // enum because we preserve unknown enum values as well. diff --git a/src/google/protobuf/util/internal/datapiece.h b/src/google/protobuf/util/internal/datapiece.h index 83516d09..95b133da 100644 --- a/src/google/protobuf/util/internal/datapiece.h +++ b/src/google/protobuf/util/internal/datapiece.h @@ -164,7 +164,8 @@ class LIBPROTOBUF_EXPORT DataPiece { // If the value is not a string, attempts to convert to a 32-bit integer. // If none of these succeeds, returns a conversion error status. util::StatusOr<int> ToEnum(const google::protobuf::Enum* enum_type, - bool use_lower_camel_for_enums) const; + bool use_lower_camel_for_enums, + bool ignore_unknown_enum_values) const; private: // Disallow implicit constructor. diff --git a/src/google/protobuf/util/internal/proto_writer.cc b/src/google/protobuf/util/internal/proto_writer.cc index 8bebf2ab..a61ed2d2 100644 --- a/src/google/protobuf/util/internal/proto_writer.cc +++ b/src/google/protobuf/util/internal/proto_writer.cc @@ -267,8 +267,9 @@ inline Status WriteString(int field_number, const DataPiece& data, inline Status WriteEnum(int field_number, const DataPiece& data, const google::protobuf::Enum* enum_type, CodedOutputStream* stream, - bool use_lower_camel_for_enums) { - StatusOr<int> e = data.ToEnum(enum_type, use_lower_camel_for_enums); + bool use_lower_camel_for_enums, + bool ignore_unknown_values) { + StatusOr<int> e = data.ToEnum(enum_type, use_lower_camel_for_enums, ignore_unknown_values); if (e.ok()) { WireFormatLite::WriteEnum(field_number, e.ValueOrDie(), stream); } @@ -665,7 +666,8 @@ ProtoWriter* ProtoWriter::RenderPrimitiveField( case google::protobuf::Field_Kind_TYPE_ENUM: { status = WriteEnum(field.number(), data, typeinfo_->GetEnumByTypeUrl(field.type_url()), - stream_.get(), use_lower_camel_for_enums_); + stream_.get(), use_lower_camel_for_enums_, + ignore_unknown_fields_); break; } default: // TYPE_GROUP or TYPE_MESSAGE diff --git a/src/google/protobuf/util/internal/proto_writer.h b/src/google/protobuf/util/internal/proto_writer.h index 0db8485c..9e3bbfeb 100644 --- a/src/google/protobuf/util/internal/proto_writer.h +++ b/src/google/protobuf/util/internal/proto_writer.h @@ -309,7 +309,7 @@ class LIBPROTOBUF_EXPORT ProtoWriter : public StructuredObjectWriter { // Indicates whether we finished writing root message completely. bool done_; - // If true, don't report unknown field names to the listener. + // If true, don't report unknown field names and enum values to the listener. bool ignore_unknown_fields_; // If true, check if enum name in camel case or without underscore matches the diff --git a/src/google/protobuf/util/json_util_test.cc b/src/google/protobuf/util/json_util_test.cc index 5b714bb1..3a43ca1c 100644 --- a/src/google/protobuf/util/json_util_test.cc +++ b/src/google/protobuf/util/json_util_test.cc @@ -333,6 +333,64 @@ TEST_F(JsonUtilTest, TestDynamicMessage) { EXPECT_EQ(ToJson(generated, options), ToJson(*message, options)); } +TEST_F(JsonUtilTest, TestParsingUnknownEnumsAs0) { + TestMessage m; + { + JsonParseOptions options; + ASSERT_FALSE(options.ignore_unknown_fields); + string input = + "{\n" + " \"enum_value\":\"UNKNOWN_VALUE\"\n" + "}"; + m.set_enum_value(proto3::BAR); + EXPECT_FALSE(FromJson(input, &m, options)); + ASSERT_EQ(proto3::BAR, m.enum_value()); // Keep previous value + + options.ignore_unknown_fields = true; + EXPECT_TRUE(FromJson(input, &m, options)); + EXPECT_EQ(0, m.enum_value()); // Unknown enum value must be decoded as 0 + } + // Integer values are read as usual + { + JsonParseOptions options; + string input = + "{\n" + " \"enum_value\":12345\n" + "}"; + m.set_enum_value(proto3::BAR); + EXPECT_TRUE(FromJson(input, &m, options)); + ASSERT_EQ(12345, m.enum_value()); + + options.ignore_unknown_fields = true; + EXPECT_TRUE(FromJson(input, &m, options)); + EXPECT_EQ(12345, m.enum_value()); + } + + // Trying to pass an object as an enum field value is always treated as an error + { + JsonParseOptions options; + string input = + "{\n" + " \"enum_value\":{}\n" + "}"; + options.ignore_unknown_fields = true; + EXPECT_FALSE(FromJson(input, &m, options)); + options.ignore_unknown_fields = false; + EXPECT_FALSE(FromJson(input, &m, options)); + } + // Trying to pass an array as an enum field value is always treated as an error + { + JsonParseOptions options; + string input = + "{\n" + " \"enum_value\":[]\n" + "}"; + EXPECT_FALSE(FromJson(input, &m, options)); + options.ignore_unknown_fields = true; + EXPECT_FALSE(FromJson(input, &m, options)); + } +} + typedef std::pair<char*, int> Segment; // A ZeroCopyOutputStream that writes to multiple buffers. class SegmentedZeroCopyOutputStream : public io::ZeroCopyOutputStream { @@ -419,7 +419,7 @@ build_php5.5_c() { use_php 5.5 wget https://phar.phpunit.de/phpunit-4.8.0.phar -O /usr/bin/phpunit pushd php/tests - /bin/bash ./test.sh + /bin/bash ./test.sh 5.5 popd # TODO(teboring): Add it back # pushd conformance @@ -430,7 +430,7 @@ build_php5.5_c() { build_php5.5_zts_c() { use_php_zts 5.5 wget https://phar.phpunit.de/phpunit-4.8.0.phar -O /usr/bin/phpunit - cd php/tests && /bin/bash ./test.sh && cd ../.. + cd php/tests && /bin/bash ./test.sh 5.5-zts && cd ../.. # TODO(teboring): Add it back # pushd conformance # make test_php_zts_c @@ -453,7 +453,7 @@ build_php5.6() { build_php5.6_c() { use_php 5.6 wget https://phar.phpunit.de/phpunit-5.7.0.phar -O /usr/bin/phpunit - cd php/tests && /bin/bash ./test.sh && cd ../.. + cd php/tests && /bin/bash ./test.sh 5.6 && cd ../.. # TODO(teboring): Add it back # pushd conformance # make test_php_c @@ -463,7 +463,7 @@ build_php5.6_c() { build_php5.6_zts_c() { use_php_zts 5.6 wget https://phar.phpunit.de/phpunit-5.7.0.phar -O /usr/bin/phpunit - cd php/tests && /bin/bash ./test.sh && cd ../.. + cd php/tests && /bin/bash ./test.sh 5.6-zts && cd ../.. # TODO(teboring): Add it back # pushd conformance # make test_php_zts_c @@ -511,7 +511,7 @@ build_php7.0() { build_php7.0_c() { use_php 7.0 wget https://phar.phpunit.de/phpunit-5.6.0.phar -O /usr/bin/phpunit - cd php/tests && /bin/bash ./test.sh && cd ../.. + cd php/tests && /bin/bash ./test.sh 7.0 && cd ../.. # TODO(teboring): Add it back # pushd conformance # make test_php_c @@ -521,7 +521,7 @@ build_php7.0_c() { build_php7.0_zts_c() { use_php_zts 7.0 wget https://phar.phpunit.de/phpunit-5.6.0.phar -O /usr/bin/phpunit - cd php/tests && /bin/bash ./test.sh && cd ../.. + cd php/tests && /bin/bash ./test.sh 7.0-zts && cd ../.. # TODO(teboring): Add it back. # pushd conformance # make test_php_zts_c @@ -575,7 +575,7 @@ build_php7.1() { build_php7.1_c() { use_php 7.1 wget https://phar.phpunit.de/phpunit-5.6.0.phar -O /usr/bin/phpunit - cd php/tests && /bin/bash ./test.sh && cd ../.. + cd php/tests && /bin/bash ./test.sh 7.1 && cd ../.. pushd conformance # make test_php_c popd @@ -584,7 +584,7 @@ build_php7.1_c() { build_php7.1_zts_c() { use_php_zts 7.1 wget https://phar.phpunit.de/phpunit-5.6.0.phar -O /usr/bin/phpunit - cd php/tests && /bin/bash ./test.sh && cd ../.. + cd php/tests && /bin/bash ./test.sh 7.1-zts && cd ../.. pushd conformance # make test_php_c popd |