Skip to content

Commit

Permalink
Fix conformance test (protocolbuffers#6750)
Browse files Browse the repository at this point in the history
* Fix conformance test

Default value of string/message map is not encoded

* Fix zts build
  • Loading branch information
TeBoring authored Oct 10, 2019
1 parent 7efcc04 commit 2dec82e
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 10 deletions.
2 changes: 0 additions & 2 deletions conformance/failure_list_php_c.txt
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,6 @@ Required.Proto3.ProtobufInput.DoubleFieldNormalizeQuietNan.JsonOutput
Required.Proto3.ProtobufInput.DoubleFieldNormalizeSignalingNan.JsonOutput
Required.Proto3.ProtobufInput.FloatFieldNormalizeQuietNan.JsonOutput
Required.Proto3.ProtobufInput.FloatFieldNormalizeSignalingNan.JsonOutput
Required.Proto3.ProtobufInput.ValidDataMap.STRING.MESSAGE.MissingDefault.JsonOutput
Required.Proto3.ProtobufInput.ValidDataMap.STRING.MESSAGE.MissingDefault.ProtobufOutput
Required.Proto3.ProtobufInput.ValidDataRepeated.BYTES.JsonOutput
Required.Proto3.ProtobufInput.ValidDataRepeated.BYTES.ProtobufOutput
Required.Proto3.ProtobufInput.ValidDataRepeated.FLOAT.PackedInput.JsonOutput
Expand Down
34 changes: 26 additions & 8 deletions php/ext/google/protobuf/encode_decode.c
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,6 @@ static void new_php_string(zval** value_ptr, const char* str, size_t len) {
!IS_INTERNED(Z_STRVAL_PP(value_ptr))) {
FREE(Z_STRVAL_PP(value_ptr));
}
ZVAL_EMPTY_STRING(*value_ptr);
ZVAL_STRINGL(*value_ptr, str, len, 1);
}
#else
Expand Down Expand Up @@ -459,6 +458,7 @@ static void *submsg_handler(void *closure, const void *hd) {
// Handler data for startmap/endmap handlers.
typedef struct {
size_t ofs;
const upb_msgdef* value_md;
upb_fieldtype_t key_field_type;
upb_fieldtype_t value_field_type;
} map_handlerdata_t;
Expand All @@ -485,7 +485,9 @@ PHP_PROTO_WRAP_OBJECT_START(map_parse_frame_t)
PHP_PROTO_WRAP_OBJECT_END
typedef struct map_parse_frame_t map_parse_frame_t;

static void map_slot_init(void* memory, upb_fieldtype_t type, zval* cache) {
static void map_slot_init(
void* memory, upb_fieldtype_t type, zval* cache,
const upb_msgdef* value_msg PHP_PROTO_TSRMLS_DC) {
switch (type) {
case UPB_TYPE_STRING:
case UPB_TYPE_BYTES: {
Expand All @@ -505,16 +507,24 @@ static void map_slot_init(void* memory, upb_fieldtype_t type, zval* cache) {
break;
}
case UPB_TYPE_MESSAGE: {
Descriptor* subdesc =
UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj(value_msg));
zend_class_entry* subklass = subdesc->klass;
MessageHeader* submsg;
#if PHP_MAJOR_VERSION < 7
zval** holder = ALLOC(zval*);
zval* tmp;
MAKE_STD_ZVAL(tmp);
ZVAL_NULL(tmp);
ZVAL_OBJ(tmp, subklass->create_object(subklass TSRMLS_CC));
submsg = UNBOX(MessageHeader, tmp);
custom_data_init(subklass, submsg PHP_PROTO_TSRMLS_CC);
*holder = tmp;
*(zval***)memory = holder;
#else
*(zval**)memory = cache;
ZVAL_NULL(*(zval**)memory);
ZVAL_OBJ(*(zval**)memory, subklass->create_object(subklass TSRMLS_CC));
submsg = UNBOX(MessageHeader, cache);
custom_data_init(subklass, submsg PHP_PROTO_TSRMLS_CC);
#endif
break;
}
Expand Down Expand Up @@ -585,8 +595,10 @@ static void map_slot_value(upb_fieldtype_t type, const void* from,
zend_string_addref(*(zend_string**)to);
break;
case UPB_TYPE_MESSAGE:
*(zend_object**)to = Z_OBJ_P(*(zval**)from);
GC_ADDREF(*(zend_object**)to);
if (!ZVAL_IS_NULL(*(zval**)from)) {
*(zend_object**)to = Z_OBJ_P(*(zval**)from);
GC_ADDREF(*(zend_object**)to);
}
break;
#endif
default:
Expand All @@ -600,6 +612,7 @@ static void map_slot_value(upb_fieldtype_t type, const void* from,
static void *startmapentry_handler(void *closure, const void *hd) {
MessageHeader* msg = closure;
const map_handlerdata_t* mapdata = hd;
TSRMLS_FETCH();
zval* map = CACHED_PTR_TO_ZVAL_PTR(
DEREF(message_data(msg), mapdata->ofs, CACHED_VALUE*));

Expand All @@ -608,9 +621,9 @@ static void *startmapentry_handler(void *closure, const void *hd) {
frame->map = map;

map_slot_init(&frame->data->key_storage, mapdata->key_field_type,
&frame->key_zval);
&frame->key_zval, NULL PHP_PROTO_TSRMLS_CC);
map_slot_init(&frame->data->value_storage, mapdata->value_field_type,
&frame->value_zval);
&frame->value_zval, mapdata->value_md PHP_PROTO_TSRMLS_CC);

return frame;
}
Expand Down Expand Up @@ -665,6 +678,11 @@ static map_handlerdata_t* new_map_handlerdata(
value_field = upb_msgdef_itof(mapentry_def, MAP_VALUE_FIELD);
PHP_PROTO_ASSERT(value_field != NULL);
hd->value_field_type = upb_fielddef_type(value_field);
if (upb_fielddef_type(value_field) == UPB_TYPE_MESSAGE) {
hd->value_md = upb_fielddef_msgsubdef(value_field);
} else {
hd->value_md = NULL;
}

return hd;
}
Expand Down
32 changes: 32 additions & 0 deletions php/tests/encode_decode_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -1198,4 +1198,36 @@ public function testJsonDecodeNumericStringMapKey()
$n->mergeFromJsonString($data);
}

public function testMessageMapNoValue()
{
$m = new TestMessage();
$m->mergeFromString(hex2bin("CA0700"));
$m->serializeToString();
$this->assertTrue(true);
}

public function testAnyMapNoValue()
{
$m = new TestMessage();
$m->mergeFromString(hex2bin("D20700"));
$m->serializeToString();
$this->assertTrue(true);
}

public function testListValueMapNoValue()
{
$m = new TestMessage();
$m->mergeFromString(hex2bin("DA0700"));
$m->serializeToString();
$this->assertTrue(true);
}

public function testStructMapNoValue()
{
$m = new TestMessage();
$m->mergeFromString(hex2bin("E20700"));
$m->serializeToString();
$this->assertTrue(true);
}

}
6 changes: 6 additions & 0 deletions php/tests/proto/test.proto
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,12 @@ message TestMessage {
}

reserved 111;

// Test map with missing message value
map<string, TestMessage> map_string_message = 121;
map<string, google.protobuf.Any> map_string_any = 122;
map<string, google.protobuf.ListValue> map_string_list = 123;
map<string, google.protobuf.Struct> map_string_struct = 124;
}

enum TestEnum {
Expand Down

0 comments on commit 2dec82e

Please sign in to comment.