Skip to content

Commit

Permalink
Adopt upb change for timestamp and duration json to php (#5106)
Browse files Browse the repository at this point in the history 
* Adopt upb change for timestamp and duration json to php

* Remove unused code

* Re-sync upb

* Fix php implementation timestamp json parsing

* Fix strptime use local timezone on mac.

* Remove succeeding tests

* Resync

* Add tests for values

* Fix php tests

* Fix encoder handlers change default value

Previously, oneofsubmsg_handler and submsg_handler change zval's default value directly.
The fix use REPLACE_ZVAL_VALUE which create a copy of parsed value and assign it to zval.
  • Loading branch information
@TeBoring
TeBoring committed Sep 23, 2018
1 parent 47d33e7 commit 9bda1f1
Show file tree
Hide file tree
Showing 7 changed files with 3,407 additions and 3,797 deletions.
39 changes: 2 additions & 37 deletions conformance/failure_list_php_c.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ Recommended.Proto3.JsonInput.BytesFieldBase64Url.JsonOutput
Recommended.Proto3.JsonInput.BytesFieldBase64Url.ProtobufOutput
Recommended.Proto3.JsonInput.DurationHas3FractionalDigits.Validator
Recommended.Proto3.JsonInput.DurationHas6FractionalDigits.Validator
Recommended.Proto3.JsonInput.DurationHas9FractionalDigits.Validator
Recommended.Proto3.JsonInput.DurationHasZeroFractionalDigit.Validator
Recommended.Proto3.JsonInput.Int64FieldBeString.Validator
Recommended.Proto3.JsonInput.MapFieldValueIsNull
Recommended.Proto3.JsonInput.OneofZeroBytes.JsonOutput
Expand All @@ -18,13 +16,12 @@ Recommended.Proto3.JsonInput.StringFieldUnpairedHighSurrogate
Recommended.Proto3.JsonInput.StringFieldUnpairedLowSurrogate
Recommended.Proto3.JsonInput.TimestampHas3FractionalDigits.Validator
Recommended.Proto3.JsonInput.TimestampHas6FractionalDigits.Validator
Recommended.Proto3.JsonInput.TimestampHas9FractionalDigits.Validator
Recommended.Proto3.JsonInput.TimestampHasZeroFractionalDigit.Validator
Recommended.Proto3.JsonInput.TimestampZeroNormalized.Validator
Recommended.Proto3.JsonInput.Uint64FieldBeString.Validator
Recommended.Proto3.ProtobufInput.OneofZeroBytes.JsonOutput
Required.DurationProtoInputTooLarge.JsonOutput
Required.DurationProtoInputTooSmall.JsonOutput
Required.TimestampProtoInputTooLarge.JsonOutput
Required.TimestampProtoInputTooSmall.JsonOutput
Required.Proto3.JsonInput.Any.JsonOutput
Required.Proto3.JsonInput.Any.ProtobufOutput
Required.Proto3.JsonInput.AnyNested.JsonOutput
Expand All @@ -51,12 +48,8 @@ Required.Proto3.JsonInput.DoubleFieldMaxNegativeValue.ProtobufOutput
Required.Proto3.JsonInput.DoubleFieldMinPositiveValue.JsonOutput
Required.Proto3.JsonInput.DoubleFieldMinPositiveValue.ProtobufOutput
Required.Proto3.JsonInput.DoubleFieldNan.JsonOutput
Required.Proto3.JsonInput.DurationMaxValue.JsonOutput
Required.Proto3.JsonInput.DurationMaxValue.ProtobufOutput
Required.Proto3.JsonInput.DurationMinValue.JsonOutput
Required.Proto3.JsonInput.DurationMinValue.ProtobufOutput
Required.Proto3.JsonInput.DurationRepeatedValue.JsonOutput
Required.Proto3.JsonInput.DurationRepeatedValue.ProtobufOutput
Required.Proto3.JsonInput.FieldMask.JsonOutput
Required.Proto3.JsonInput.FieldMask.ProtobufOutput
Required.Proto3.JsonInput.FloatFieldInfinity.JsonOutput
Expand All @@ -73,36 +66,8 @@ Required.Proto3.JsonInput.StringFieldUnicodeEscape.JsonOutput
Required.Proto3.JsonInput.StringFieldUnicodeEscape.ProtobufOutput
Required.Proto3.JsonInput.StringFieldUnicodeEscapeWithLowercaseHexLetters.JsonOutput
Required.Proto3.JsonInput.StringFieldUnicodeEscapeWithLowercaseHexLetters.ProtobufOutput
Required.Proto3.JsonInput.Struct.JsonOutput
Required.Proto3.JsonInput.Struct.ProtobufOutput
Required.Proto3.JsonInput.TimestampMaxValue.JsonOutput
Required.Proto3.JsonInput.TimestampMaxValue.ProtobufOutput
Required.Proto3.JsonInput.TimestampMinValue.JsonOutput
Required.Proto3.JsonInput.TimestampMinValue.ProtobufOutput
Required.Proto3.JsonInput.TimestampRepeatedValue.JsonOutput
Required.Proto3.JsonInput.TimestampRepeatedValue.ProtobufOutput
Required.Proto3.JsonInput.TimestampWithNegativeOffset.JsonOutput
Required.Proto3.JsonInput.TimestampWithNegativeOffset.ProtobufOutput
Required.Proto3.JsonInput.TimestampWithPositiveOffset.JsonOutput
Required.Proto3.JsonInput.TimestampWithPositiveOffset.ProtobufOutput
Required.Proto3.JsonInput.ValueAcceptBool.JsonOutput
Required.Proto3.JsonInput.ValueAcceptBool.ProtobufOutput
Required.Proto3.JsonInput.ValueAcceptFloat.JsonOutput
Required.Proto3.JsonInput.ValueAcceptFloat.ProtobufOutput
Required.Proto3.JsonInput.ValueAcceptInteger.JsonOutput
Required.Proto3.JsonInput.ValueAcceptInteger.ProtobufOutput
Required.Proto3.JsonInput.ValueAcceptList.JsonOutput
Required.Proto3.JsonInput.ValueAcceptList.ProtobufOutput
Required.Proto3.JsonInput.ValueAcceptNull.JsonOutput
Required.Proto3.JsonInput.ValueAcceptNull.ProtobufOutput
Required.Proto3.JsonInput.ValueAcceptObject.JsonOutput
Required.Proto3.JsonInput.ValueAcceptObject.ProtobufOutput
Required.Proto3.JsonInput.ValueAcceptString.JsonOutput
Required.Proto3.JsonInput.ValueAcceptString.ProtobufOutput
Required.Proto3.ProtobufInput.DoubleFieldNormalizeQuietNan.JsonOutput
Required.Proto3.ProtobufInput.DoubleFieldNormalizeSignalingNan.JsonOutput
Required.Proto3.ProtobufInput.FloatFieldNormalizeQuietNan.JsonOutput
Required.Proto3.ProtobufInput.FloatFieldNormalizeSignalingNan.JsonOutput
Required.Proto3.ProtobufInput.ValidDataRepeated.FLOAT.JsonOutput
Required.TimestampProtoInputTooLarge.JsonOutput
Required.TimestampProtoInputTooSmall.JsonOutput
115 changes: 69 additions & 46 deletions php/ext/google/protobuf/encode_decode.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,12 @@ static void stackenv_uninit(stackenv* se) {
// Parsing.
// -----------------------------------------------------------------------------

// TODO(teboring): This shoud be a bit in upb_msgdef
static bool is_wrapper_msg(const upb_msgdef *msg) {
return !strcmp(upb_filedef_name(upb_msgdef_upcast(msg)->file),
"google/protobuf/wrappers.proto");
}

#define DEREF(msg, ofs, type) *(type*)(((uint8_t *)msg) + ofs)

// Creates a handlerdata that simply contains the offset for this field.
Expand Down Expand Up @@ -420,13 +426,13 @@ static void *submsg_handler(void *closure, const void *hd) {
if (Z_TYPE_P(CACHED_PTR_TO_ZVAL_PTR(DEREF(message_data(msg), submsgdata->ofs,
CACHED_VALUE*))) == IS_NULL) {
#if PHP_MAJOR_VERSION < 7
zval* val = NULL;
MAKE_STD_ZVAL(val);
ZVAL_OBJ(val, subklass->create_object(subklass TSRMLS_CC));
MessageHeader* intern = UNBOX(MessageHeader, val);
zval val;
ZVAL_OBJ(&val, subklass->create_object(subklass TSRMLS_CC));
MessageHeader* intern = UNBOX(MessageHeader, &val);
custom_data_init(subklass, intern PHP_PROTO_TSRMLS_CC);
php_proto_zval_ptr_dtor(*DEREF(message_data(msg), submsgdata->ofs, zval**));
*DEREF(message_data(msg), submsgdata->ofs, zval**) = val;
REPLACE_ZVAL_VALUE(DEREF(message_data(msg), submsgdata->ofs, zval**),
&val, 1);
zval_dtor(&val);
#else
zend_object* obj = subklass->create_object(subklass TSRMLS_CC);
ZVAL_OBJ(DEREF(message_data(msg), submsgdata->ofs, zval*), obj);
Expand Down Expand Up @@ -765,9 +771,16 @@ static void* oneofsubmsg_handler(void* closure, const void* hd) {
// Create new message.
DEREF(message_data(msg), oneofdata->ofs, CACHED_VALUE*) =
OBJ_PROP(&msg->std, oneofdata->property_ofs);
ZVAL_OBJ(CACHED_PTR_TO_ZVAL_PTR(
DEREF(message_data(msg), oneofdata->ofs, CACHED_VALUE*)),
subklass->create_object(subklass TSRMLS_CC));
#if PHP_MAJOR_VERSION < 7
zval val;
ZVAL_OBJ(&val, subklass->create_object(subklass TSRMLS_CC));
REPLACE_ZVAL_VALUE(DEREF(message_data(msg), oneofdata->ofs, zval**),
&val, 1);
zval_dtor(&val);
#else
zend_object* obj = subklass->create_object(subklass TSRMLS_CC);
ZVAL_OBJ(DEREF(message_data(msg), oneofdata->ofs, zval*), obj);
#endif
}

DEREF(message_data(msg), oneofdata->case_ofs, uint32_t) =
Expand Down Expand Up @@ -1080,24 +1093,25 @@ static const upb_json_parsermethod *msgdef_jsonparsermethod(Descriptor* desc) {
// -----------------------------------------------------------------------------

static void putmsg(zval* msg, const Descriptor* desc, upb_sink* sink,
int depth TSRMLS_DC);
int depth, bool is_json TSRMLS_DC);
static void putrawmsg(MessageHeader* msg, const Descriptor* desc,
upb_sink* sink, int depth TSRMLS_DC);
upb_sink* sink, int depth, bool is_json TSRMLS_DC);

static void putstr(zval* str, const upb_fielddef* f, upb_sink* sink);
static void putstr(zval* str, const upb_fielddef* f, upb_sink* sink,
bool force_default);

static void putrawstr(const char* str, int len, const upb_fielddef* f,
upb_sink* sink);
upb_sink* sink, bool force_default);

static void putsubmsg(zval* submsg, const upb_fielddef* f, upb_sink* sink,
int depth TSRMLS_DC);
int depth, bool is_json TSRMLS_DC);
static void putrawsubmsg(MessageHeader* submsg, const upb_fielddef* f,
upb_sink* sink, int depth TSRMLS_DC);
upb_sink* sink, int depth, bool is_json TSRMLS_DC);

static void putarray(zval* array, const upb_fielddef* f, upb_sink* sink,
int depth TSRMLS_DC);
int depth, bool is_json TSRMLS_DC);
static void putmap(zval* map, const upb_fielddef* f, upb_sink* sink,
int depth TSRMLS_DC);
int depth, bool is_json TSRMLS_DC);

static upb_selector_t getsel(const upb_fielddef* f, upb_handlertype_t type) {
upb_selector_t ret;
Expand All @@ -1106,8 +1120,10 @@ static upb_selector_t getsel(const upb_fielddef* f, upb_handlertype_t type) {
return ret;
}

static void put_optional_value(const void* memory, int len, const upb_fielddef* f,
int depth, upb_sink* sink TSRMLS_DC) {
static void put_optional_value(const void* memory, int len,
const upb_fielddef* f,
int depth, upb_sink* sink,
bool is_json TSRMLS_DC) {
assert(upb_fielddef_label(f) == UPB_LABEL_OPTIONAL);

switch (upb_fielddef_type(f)) {
Expand All @@ -1132,7 +1148,8 @@ static void put_optional_value(const void* memory, int len, const upb_fielddef*
#undef T
case UPB_TYPE_STRING:
case UPB_TYPE_BYTES:
putrawstr(memory, len, f, sink);
putrawstr(memory, len, f, sink,
is_json && is_wrapper_msg(upb_fielddef_containingtype(f)));
break;
case UPB_TYPE_MESSAGE: {
#if PHP_MAJOR_VERSION < 7
Expand All @@ -1142,7 +1159,7 @@ static void put_optional_value(const void* memory, int len, const upb_fielddef*
(MessageHeader*)((char*)(*(zend_object**)memory) -
XtOffsetOf(MessageHeader, std));
#endif
putrawsubmsg(submsg, f, sink, depth TSRMLS_CC);
putrawsubmsg(submsg, f, sink, depth, is_json TSRMLS_CC);
break;
}
default:
Expand Down Expand Up @@ -1181,7 +1198,7 @@ static int raw_value_len(void* memory, int len, const upb_fielddef* f) {
}

static void putmap(zval* map, const upb_fielddef* f, upb_sink* sink,
int depth TSRMLS_DC) {
int depth, bool is_json TSRMLS_DC) {
upb_sink subsink;
const upb_fielddef* key_field;
const upb_fielddef* value_field;
Expand Down Expand Up @@ -1209,13 +1226,14 @@ static void putmap(zval* map, const upb_fielddef* f, upb_sink* sink,

// Serialize key.
const char *key = map_iter_key(&it, &len);
put_optional_value(key, len, key_field, depth + 1, &entry_sink TSRMLS_CC);
put_optional_value(key, len, key_field, depth + 1,
&entry_sink, is_json TSRMLS_CC);

// Serialize value.
upb_value value = map_iter_value(&it, &len);
put_optional_value(raw_value(upb_value_memory(&value), value_field),
raw_value_len(upb_value_memory(&value), len, value_field),
value_field, depth + 1, &entry_sink TSRMLS_CC);
value_field, depth + 1, &entry_sink, is_json TSRMLS_CC);

upb_sink_endmsg(&entry_sink, &status);
upb_sink_endsubmsg(&subsink, getsel(f, UPB_HANDLER_ENDSUBMSG));
Expand All @@ -1225,13 +1243,13 @@ static void putmap(zval* map, const upb_fielddef* f, upb_sink* sink,
}

static void putmsg(zval* msg_php, const Descriptor* desc, upb_sink* sink,
int depth TSRMLS_DC) {
int depth, bool is_json TSRMLS_DC) {
MessageHeader* msg = UNBOX(MessageHeader, msg_php);
putrawmsg(msg, desc, sink, depth TSRMLS_CC);
putrawmsg(msg, desc, sink, depth, is_json TSRMLS_CC);
}

static void putrawmsg(MessageHeader* msg, const Descriptor* desc,
upb_sink* sink, int depth TSRMLS_DC) {
upb_sink* sink, int depth, bool is_json TSRMLS_DC) {
upb_msg_field_iter i;
upb_status status;

Expand Down Expand Up @@ -1268,31 +1286,34 @@ static void putrawmsg(MessageHeader* msg, const Descriptor* desc,
zval* map = CACHED_PTR_TO_ZVAL_PTR(
DEREF(message_data(msg), offset, CACHED_VALUE*));
if (map != NULL) {
putmap(map, f, sink, depth TSRMLS_CC);
putmap(map, f, sink, depth, is_json TSRMLS_CC);
}
} else if (upb_fielddef_isseq(f)) {
zval* array = CACHED_PTR_TO_ZVAL_PTR(
DEREF(message_data(msg), offset, CACHED_VALUE*));
if (array != NULL) {
putarray(array, f, sink, depth TSRMLS_CC);
putarray(array, f, sink, depth, is_json TSRMLS_CC);
}
} else if (upb_fielddef_isstring(f)) {
zval* str = CACHED_PTR_TO_ZVAL_PTR(
DEREF(message_data(msg), offset, CACHED_VALUE*));
if (containing_oneof || Z_STRLEN_P(str) > 0) {
putstr(str, f, sink);
if (containing_oneof || (is_json && is_wrapper_msg(desc->msgdef)) ||
Z_STRLEN_P(str) > 0) {
putstr(str, f, sink, is_json && is_wrapper_msg(desc->msgdef));
}
} else if (upb_fielddef_issubmsg(f)) {
putsubmsg(CACHED_PTR_TO_ZVAL_PTR(
DEREF(message_data(msg), offset, CACHED_VALUE*)),
f, sink, depth TSRMLS_CC);
f, sink, depth, is_json TSRMLS_CC);
} else {
upb_selector_t sel = getsel(f, upb_handlers_getprimitivehandlertype(f));

#define T(upbtypeconst, upbtype, ctype, default_value) \
case upbtypeconst: { \
ctype value = DEREF(message_data(msg), offset, ctype); \
if (containing_oneof || value != default_value) { \
if (containing_oneof || \
(is_json && is_wrapper_msg(desc->msgdef)) || \
value != default_value) { \
upb_sink_put##upbtype(sink, sel, value); \
} \
} break;
Expand Down Expand Up @@ -1325,7 +1346,8 @@ static void putrawmsg(MessageHeader* msg, const Descriptor* desc,
upb_sink_endmsg(sink, &status);
}

static void putstr(zval* str, const upb_fielddef *f, upb_sink *sink) {
static void putstr(zval* str, const upb_fielddef *f,
upb_sink *sink, bool force_default) {
upb_sink subsink;

if (ZVAL_IS_NULL(str)) return;
Expand All @@ -1336,7 +1358,7 @@ static void putstr(zval* str, const upb_fielddef *f, upb_sink *sink) {
&subsink);

// For oneof string field, we may get here with string length is zero.
if (Z_STRLEN_P(str) > 0) {
if (Z_STRLEN_P(str) > 0 || force_default) {
// Ensure that the string has the correct encoding. We also check at
// field-set time, but the user may have mutated the string object since
// then.
Expand All @@ -1353,10 +1375,10 @@ static void putstr(zval* str, const upb_fielddef *f, upb_sink *sink) {
}

static void putrawstr(const char* str, int len, const upb_fielddef* f,
upb_sink* sink) {
upb_sink* sink, bool force_default) {
upb_sink subsink;

if (len == 0) return;
if (len == 0 && !force_default) return;

// Ensure that the string has the correct encoding. We also check at field-set
// time, but the user may have mutated the string object since then.
Expand All @@ -1372,27 +1394,27 @@ static void putrawstr(const char* str, int len, const upb_fielddef* f,
}

static void putsubmsg(zval* submsg_php, const upb_fielddef* f, upb_sink* sink,
int depth TSRMLS_DC) {
int depth, bool is_json TSRMLS_DC) {
if (Z_TYPE_P(submsg_php) == IS_NULL) return;

MessageHeader *submsg = UNBOX(MessageHeader, submsg_php);
putrawsubmsg(submsg, f, sink, depth TSRMLS_CC);
putrawsubmsg(submsg, f, sink, depth, is_json TSRMLS_CC);
}

static void putrawsubmsg(MessageHeader* submsg, const upb_fielddef* f,
upb_sink* sink, int depth TSRMLS_DC) {
upb_sink* sink, int depth, bool is_json TSRMLS_DC) {
upb_sink subsink;

Descriptor* subdesc =
UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj(upb_fielddef_msgsubdef(f)));

upb_sink_startsubmsg(sink, getsel(f, UPB_HANDLER_STARTSUBMSG), &subsink);
putrawmsg(submsg, subdesc, &subsink, depth + 1 TSRMLS_CC);
putrawmsg(submsg, subdesc, &subsink, depth + 1, is_json TSRMLS_CC);
upb_sink_endsubmsg(sink, getsel(f, UPB_HANDLER_ENDSUBMSG));
}

static void putarray(zval* array, const upb_fielddef* f, upb_sink* sink,
int depth TSRMLS_DC) {
int depth, bool is_json TSRMLS_DC) {
upb_sink subsink;
upb_fieldtype_t type = upb_fielddef_type(f);
upb_selector_t sel = 0;
Expand Down Expand Up @@ -1436,7 +1458,8 @@ static void putarray(zval* array, const upb_fielddef* f, upb_sink* sink,
const char* rawstr = ZSTR_VAL(*(zend_string**)memory);
int len = ZSTR_LEN(*(zend_string**)memory);
#endif
putrawstr(rawstr, len, f, &subsink);
putrawstr(rawstr, len, f, &subsink,
is_json && is_wrapper_msg(upb_fielddef_containingtype(f)));
break;
}
case UPB_TYPE_MESSAGE: {
Expand All @@ -1447,7 +1470,7 @@ static void putarray(zval* array, const upb_fielddef* f, upb_sink* sink,
(MessageHeader*)((char*)(Z_OBJ_P((zval*)memory)) -
XtOffsetOf(MessageHeader, std));
#endif
putrawsubmsg(submsg, f, &subsink, depth TSRMLS_CC);
putrawsubmsg(submsg, f, &subsink, depth, is_json TSRMLS_CC);
break;
}

Expand Down Expand Up @@ -1504,7 +1527,7 @@ void serialize_to_string(zval* val, zval* return_value TSRMLS_DC) {
stackenv_init(&se, "Error occurred during encoding: %s");
encoder = upb_pb_encoder_create(&se.env, serialize_handlers, &sink.sink);

putmsg(val, desc, upb_pb_encoder_input(encoder), 0 TSRMLS_CC);
putmsg(val, desc, upb_pb_encoder_input(encoder), 0, false TSRMLS_CC);

PHP_PROTO_RETVAL_STRINGL(sink.ptr, sink.len, 1);

Expand Down Expand Up @@ -1571,7 +1594,7 @@ PHP_METHOD(Message, serializeToJsonString) {
stackenv_init(&se, "Error occurred during encoding: %s");
printer = upb_json_printer_create(&se.env, serialize_handlers, &sink.sink);

putmsg(getThis(), desc, upb_json_printer_input(printer), 0 TSRMLS_CC);
putmsg(getThis(), desc, upb_json_printer_input(printer), 0, true TSRMLS_CC);

PHP_PROTO_RETVAL_STRINGL(sink.ptr, sink.len, 1);

Expand Down
Loading

0 comments on commit 9bda1f1

Please sign in to comment.