From 11c979b591b6904d000b6533be3764615934cbc1 Mon Sep 17 00:00:00 2001 From: Paul Yang Date: Tue, 8 Jan 2019 14:11:33 -0800 Subject: [PATCH] Fix Empty ListValue/Struct json encoding (#5532) * Fix Empty ListValue/Struct json encoding * Add test for empty ListValue --- php/ext/google/protobuf/encode_decode.c | 79 +++++++++++++++++++- php/src/Google/Protobuf/Internal/Message.php | 36 +++++++++ php/tests/encode_decode_test.php | 14 ++++ 3 files changed, 128 insertions(+), 1 deletion(-) diff --git a/php/ext/google/protobuf/encode_decode.c b/php/ext/google/protobuf/encode_decode.c index 1313676620..3d2bb7c203 100644 --- a/php/ext/google/protobuf/encode_decode.c +++ b/php/ext/google/protobuf/encode_decode.c @@ -1099,6 +1099,12 @@ static void putrawmsg(MessageHeader* msg, const Descriptor* desc, bool open_msg TSRMLS_DC); static void putjsonany(MessageHeader* msg, const Descriptor* desc, upb_sink* sink, int depth TSRMLS_DC); +static void putjsonlistvalue( + MessageHeader* msg, const Descriptor* desc, + upb_sink* sink, int depth TSRMLS_DC); +static void putjsonstruct( + MessageHeader* msg, const Descriptor* desc, + upb_sink* sink, int depth TSRMLS_DC); static void putstr(zval* str, const upb_fielddef* f, upb_sink* sink, bool force_default); @@ -1342,17 +1348,88 @@ static void putjsonany(MessageHeader* msg, const Descriptor* desc, upb_sink_endmsg(sink, &status); } +static void putjsonlistvalue( + MessageHeader* msg, const Descriptor* desc, + upb_sink* sink, int depth TSRMLS_DC) { + upb_status status; + upb_sink subsink; + const upb_fielddef* f = upb_msgdef_itof(desc->msgdef, 1); + uint32_t offset = desc->layout->fields[upb_fielddef_index(f)].offset; + zval* array; + RepeatedField* intern; + HashTable *ht; + int size, i; + + upb_sink_startmsg(sink); + + array = CACHED_PTR_TO_ZVAL_PTR( + DEREF(message_data(msg), offset, CACHED_VALUE*)); + intern = UNBOX(RepeatedField, array); + ht = PHP_PROTO_HASH_OF(intern->array); + size = zend_hash_num_elements(ht); + + if (size == 0) { + upb_sink_startseq(sink, getsel(f, UPB_HANDLER_STARTSEQ), &subsink); + upb_sink_endseq(sink, getsel(f, UPB_HANDLER_ENDSEQ)); + } else { + putarray(array, f, sink, depth, true TSRMLS_CC); + } + + upb_sink_endmsg(sink, &status); +} + +static void putjsonstruct( + MessageHeader* msg, const Descriptor* desc, + upb_sink* sink, int depth TSRMLS_DC) { + upb_status status; + upb_sink subsink; + const upb_fielddef* f = upb_msgdef_itof(desc->msgdef, 1); + uint32_t offset = desc->layout->fields[upb_fielddef_index(f)].offset; + zval* map; + Map* intern; + int size; + + upb_sink_startmsg(sink); + + map = CACHED_PTR_TO_ZVAL_PTR( + DEREF(message_data(msg), offset, CACHED_VALUE*)); + intern = UNBOX(Map, map); + size = upb_strtable_count(&intern->table); + + if (size == 0) { + upb_sink_startseq(sink, getsel(f, UPB_HANDLER_STARTSEQ), &subsink); + upb_sink_endseq(sink, getsel(f, UPB_HANDLER_ENDSEQ)); + } else { + putmap(map, f, sink, depth, true TSRMLS_CC); + } + + upb_sink_endmsg(sink, &status); +} + static void putrawmsg(MessageHeader* msg, const Descriptor* desc, upb_sink* sink, int depth, bool is_json, bool open_msg TSRMLS_DC) { upb_msg_field_iter i; upb_status status; - if (is_json && upb_msgdef_wellknowntype(desc->msgdef) == UPB_WELLKNOWN_ANY) { + if (is_json && + upb_msgdef_wellknowntype(desc->msgdef) == UPB_WELLKNOWN_ANY) { putjsonany(msg, desc, sink, depth TSRMLS_CC); return; } + if (is_json && + upb_msgdef_wellknowntype(desc->msgdef) == UPB_WELLKNOWN_LISTVALUE) { + putjsonlistvalue(msg, desc, sink, depth TSRMLS_CC); + return; + } + + if (is_json && + upb_msgdef_wellknowntype(desc->msgdef) == UPB_WELLKNOWN_STRUCT) { + putjsonstruct(msg, desc, sink, depth TSRMLS_CC); + return; + } + if (open_msg) { upb_sink_startmsg(sink); } diff --git a/php/src/Google/Protobuf/Internal/Message.php b/php/src/Google/Protobuf/Internal/Message.php index 016c489e37..92d6df7e49 100644 --- a/php/src/Google/Protobuf/Internal/Message.php +++ b/php/src/Google/Protobuf/Internal/Message.php @@ -1383,6 +1383,24 @@ class Message $timestamp = GPBUtil::formatTimestamp($this); $timestamp = json_encode($timestamp); $output->writeRaw($timestamp, strlen($timestamp)); + } elseif (get_class($this) === 'Google\Protobuf\ListValue') { + $field = $this->desc->getField()[1]; + if (!$this->existField($field)) { + $output->writeRaw("[]", 2); + } else { + if (!$this->serializeFieldToJsonStream($output, $field)) { + return false; + } + } + } elseif (get_class($this) === 'Google\Protobuf\Struct') { + $field = $this->desc->getField()[1]; + if (!$this->existField($field)) { + $output->writeRaw("{}", 2); + } else { + if (!$this->serializeFieldToJsonStream($output, $field)) { + return false; + } + } } else { if (!GPBUtil::hasSpecialJsonMapping($this)) { $output->writeRaw("{", 1); @@ -1844,6 +1862,24 @@ class Message $timestamp = GPBUtil::formatTimestamp($this); $timestamp = json_encode($timestamp); $size += strlen($timestamp); + } elseif (get_class($this) === 'Google\Protobuf\ListValue') { + $field = $this->desc->getField()[1]; + if ($this->existField($field)) { + $field_size = $this->fieldJsonByteSize($field); + $size += $field_size; + } else { + // Size for "[]". + $size += 2; + } + } elseif (get_class($this) === 'Google\Protobuf\Struct') { + $field = $this->desc->getField()[1]; + if ($this->existField($field)) { + $field_size = $this->fieldJsonByteSize($field); + $size += $field_size; + } else { + // Size for "{}". + $size += 2; + } } else { if (!GPBUtil::hasSpecialJsonMapping($this)) { // Size for "{}". diff --git a/php/tests/encode_decode_test.php b/php/tests/encode_decode_test.php index 8018f9798b..6aaca840cc 100644 --- a/php/tests/encode_decode_test.php +++ b/php/tests/encode_decode_test.php @@ -898,6 +898,13 @@ class EncodeDecodeTest extends TestBase $this->assertSame("[1.5]", $m->serializeToJsonString()); } + public function testEncodeEmptyListValue() + { + $m = new Struct(); + $m->setFields(['test' => (new Value())->setListValue(new ListValue())]); + $this->assertSame('{"test":[]}', $m->serializeToJsonString()); + } + public function testDecodeTopLevelStruct() { $m = new Struct(); @@ -917,6 +924,13 @@ class EncodeDecodeTest extends TestBase $this->assertSame("{\"a\":1.5}", $m->serializeToJsonString()); } + public function testEncodeEmptyStruct() + { + $m = new Struct(); + $m->setFields(['test' => (new Value())->setStructValue(new Struct())]); + $this->assertSame('{"test":{}}', $m->serializeToJsonString()); + } + public function testDecodeTopLevelAny() { // Make sure packed message has been created at least once.