Parse nested listvalue in json for ruby (#5867)

* Add conformance test for nested listvalue

* Fix upb for parsing repeated Value/ListValue

* Add failed repeated ListValue conformance test into php failure list

conformance/binary_json_conformance_suite.cc

@@ -2194,6 +2194,36 @@ void BinaryAndJsonConformanceSuite::RunSuiteImpl() {
           }
         }
       )");
+  RunValidJsonTest(
+      "RepeatedValue", REQUIRED,
+      R"({
+        "repeatedValue": [["a"]]
+      })",
+      R"(
+        repeated_value: [
+          {
+            list_value: {
+              values: [
+                { string_value: "a"}
+              ]
+            }
+          }
+        ]
+      )");
+  RunValidJsonTest(
+      "RepeatedListValue", REQUIRED,
+      R"({
+        "repeatedListValue": [["a"]]
+      })",
+      R"(
+        repeated_list_value: [
+          {
+            values: [
+              { string_value: "a"}
+            ]
+          }
+        ]
+      )");
 
   // Any
   RunValidJsonTest(

conformance/failure_list_php.txt

@@ -14,6 +14,8 @@ Required.Proto3.JsonInput.DoubleFieldTooSmall
 Required.Proto3.JsonInput.Int32FieldNotInteger
 Required.Proto3.JsonInput.Int64FieldNotInteger
 Required.Proto3.JsonInput.RepeatedFieldWrongElementTypeExpectingStringsGotInt
+Required.Proto3.JsonInput.RepeatedListValue.JsonOutput
+Required.Proto3.JsonInput.RepeatedListValue.ProtobufOutput
 Required.Proto3.JsonInput.StringFieldNotAString
 Required.Proto3.JsonInput.Uint32FieldNotInteger
 Required.Proto3.JsonInput.Uint64FieldNotInteger

php/ext/google/protobuf/upb.c

@@ -12768,6 +12768,11 @@ typedef struct {
   /* The table mapping json name to fielddef for this message. */
   upb_strtable *name_table;
 
+  /* We are in a repeated-field context. We need this flag to decide whether to
+   * handle the array as a normal repeated field or a
+   * google.protobuf.ListValue/google.protobuf.Value. */
+  bool is_repeated;
+
   /* We are in a repeated-field context, ready to emit mapentries as
    * submessages. This flag alters the start-of-object (open-brace) behavior to
    * begin a sequence of mapentry messages rather than a single submessage. */
@@ -13808,6 +13813,7 @@ static bool start_stringval(upb_json_parser *p) {
     inner->m = p->top->m;
     inner->f = p->top->f;
     inner->name_table = NULL;
+    inner->is_repeated = false;
     inner->is_map = false;
     inner->is_mapentry = false;
     inner->is_any = false;
@@ -14284,6 +14290,7 @@ static bool start_fieldmask_path(upb_json_parser *p) {
   inner->m = p->top->m;
   inner->f = p->top->f;
   inner->name_table = NULL;
+  inner->is_repeated = false;
   inner->is_map = false;
   inner->is_mapentry = false;
   inner->is_any = false;
@@ -14431,6 +14438,7 @@ static bool handle_mapentry(upb_json_parser *p) {
   inner->m = mapentrymsg;
   inner->name_table = NULL;
   inner->mapfield = mapfield;
+  inner->is_repeated = false;
   inner->is_map = false;
   inner->is_any = false;
   inner->any_frame = NULL;
@@ -14555,6 +14563,7 @@ static bool start_subobject(upb_json_parser *p) {
     inner = p->top + 1;
     inner->m = NULL;
     inner->f = NULL;
+    inner->is_repeated = false;
     inner->is_map = false;
     inner->is_mapentry = false;
     inner->is_any = false;
@@ -14579,6 +14588,7 @@ static bool start_subobject(upb_json_parser *p) {
     inner->name_table = NULL;
     inner->mapfield = p->top->f;
     inner->f = NULL;
+    inner->is_repeated = false;
     inner->is_map = true;
     inner->is_mapentry = false;
     inner->is_any = false;
@@ -14602,6 +14612,7 @@ static bool start_subobject(upb_json_parser *p) {
     inner->m = upb_fielddef_msgsubdef(p->top->f);
     set_name_table(p, inner);
     inner->f = NULL;
+    inner->is_repeated = false;
     inner->is_map = false;
     inner->is_mapentry = false;
     inner->is_unknown_field = false;
@@ -14704,10 +14715,14 @@ static bool start_array(upb_json_parser *p) {
     } else {
       return false;
     }
-  } else if (is_wellknown_field(p, UPB_WELLKNOWN_LISTVALUE)) {
+  } else if (is_wellknown_field(p, UPB_WELLKNOWN_LISTVALUE) &&
+             (!upb_fielddef_isseq(p->top->f) ||
+              p->top->is_repeated)) {
     if (!start_subobject(p)) return false;
     start_listvalue_object(p);
-  } else if (is_wellknown_field(p, UPB_WELLKNOWN_VALUE)) {
+  } else if (is_wellknown_field(p, UPB_WELLKNOWN_VALUE) &&
+             (!upb_fielddef_isseq(p->top->f) ||
+              p->top->is_repeated)) {
     if (!start_subobject(p)) return false;
     start_value_object(p, VALUE_LISTVALUE);
     if (!start_subobject(p)) return false;
@@ -14719,6 +14734,7 @@ static bool start_array(upb_json_parser *p) {
     inner->m = NULL;
     inner->name_table = NULL;
     inner->f = NULL;
+    inner->is_repeated = false;
     inner->is_map = false;
     inner->is_mapentry = false;
     inner->is_any = false;
@@ -14745,6 +14761,7 @@ static bool start_array(upb_json_parser *p) {
   inner->m = p->top->m;
   inner->name_table = NULL;
   inner->f = p->top->f;
+  inner->is_repeated = true;
   inner->is_map = false;
   inner->is_mapentry = false;
   inner->is_any = false;
@@ -15778,6 +15795,7 @@ static void json_parser_reset(upb_json_parser *p) {
 
   p->top = p->stack;
   p->top->f = NULL;
+  p->top->is_repeated = false;
   p->top->is_map = false;
   p->top->is_mapentry = false;
   p->top->is_any = false;

php/tests/proto/test.proto

@@ -1,6 +1,7 @@
 syntax = "proto3";
 
 import 'google/protobuf/any.proto';
+import 'google/protobuf/struct.proto';
 import 'proto/test_include.proto';
 import 'proto/test_no_namespace.proto';
 import 'proto/test_php_namespace.proto';

ruby/ext/google/protobuf_c/upb.c

@@ -12763,6 +12763,11 @@ typedef struct {
   /* The table mapping json name to fielddef for this message. */
   upb_strtable *name_table;
 
+  /* We are in a repeated-field context. We need this flag to decide whether to
+   * handle the array as a normal repeated field or a
+   * google.protobuf.ListValue/google.protobuf.Value. */
+  bool is_repeated;
+
   /* We are in a repeated-field context, ready to emit mapentries as
    * submessages. This flag alters the start-of-object (open-brace) behavior to
    * begin a sequence of mapentry messages rather than a single submessage. */
@@ -13803,6 +13808,7 @@ static bool start_stringval(upb_json_parser *p) {
     inner->m = p->top->m;
     inner->f = p->top->f;
     inner->name_table = NULL;
+    inner->is_repeated = false;
     inner->is_map = false;
     inner->is_mapentry = false;
     inner->is_any = false;
@@ -14286,6 +14292,7 @@ static bool start_fieldmask_path(upb_json_parser *p) {
   inner->m = p->top->m;
   inner->f = p->top->f;
   inner->name_table = NULL;
+  inner->is_repeated = false;
   inner->is_map = false;
   inner->is_mapentry = false;
   inner->is_any = false;
@@ -14433,6 +14440,7 @@ static bool handle_mapentry(upb_json_parser *p) {
   inner->m = mapentrymsg;
   inner->name_table = NULL;
   inner->mapfield = mapfield;
+  inner->is_repeated = false;
   inner->is_map = false;
   inner->is_any = false;
   inner->any_frame = NULL;
@@ -14557,6 +14565,7 @@ static bool start_subobject(upb_json_parser *p) {
     inner = p->top + 1;
     inner->m = NULL;
     inner->f = NULL;
+    inner->is_repeated = false;
     inner->is_map = false;
     inner->is_mapentry = false;
     inner->is_any = false;
@@ -14581,6 +14590,7 @@ static bool start_subobject(upb_json_parser *p) {
     inner->name_table = NULL;
     inner->mapfield = p->top->f;
     inner->f = NULL;
+    inner->is_repeated = false;
     inner->is_map = true;
     inner->is_mapentry = false;
     inner->is_any = false;
@@ -14604,6 +14614,7 @@ static bool start_subobject(upb_json_parser *p) {
     inner->m = upb_fielddef_msgsubdef(p->top->f);
     set_name_table(p, inner);
     inner->f = NULL;
+    inner->is_repeated = false;
     inner->is_map = false;
     inner->is_mapentry = false;
     inner->is_unknown_field = false;
@@ -14706,10 +14717,14 @@ static bool start_array(upb_json_parser *p) {
     } else {
       return false;
     }
-  } else if (is_wellknown_field(p, UPB_WELLKNOWN_LISTVALUE)) {
+  } else if (is_wellknown_field(p, UPB_WELLKNOWN_LISTVALUE) &&
+             (!upb_fielddef_isseq(p->top->f) ||
+              p->top->is_repeated)) {
     if (!start_subobject(p)) return false;
     start_listvalue_object(p);
-  } else if (is_wellknown_field(p, UPB_WELLKNOWN_VALUE)) {
+  } else if (is_wellknown_field(p, UPB_WELLKNOWN_VALUE) &&
+             (!upb_fielddef_isseq(p->top->f) ||
+              p->top->is_repeated)) {
     if (!start_subobject(p)) return false;
     start_value_object(p, VALUE_LISTVALUE);
     if (!start_subobject(p)) return false;
@@ -14721,6 +14736,7 @@ static bool start_array(upb_json_parser *p) {
     inner->m = NULL;
     inner->name_table = NULL;
     inner->f = NULL;
+    inner->is_repeated = false;
     inner->is_map = false;
     inner->is_mapentry = false;
     inner->is_any = false;
@@ -14747,6 +14763,7 @@ static bool start_array(upb_json_parser *p) {
   inner->m = p->top->m;
   inner->name_table = NULL;
   inner->f = p->top->f;
+  inner->is_repeated = true;
   inner->is_map = false;
   inner->is_mapentry = false;
   inner->is_any = false;
@@ -15780,6 +15797,7 @@ static void json_parser_reset(upb_json_parser *p) {
 
   p->top = p->stack;
   p->top->f = NULL;
+  p->top->is_repeated = false;
   p->top->is_map = false;
   p->top->is_mapentry = false;
   p->top->is_any = false;

src/google/protobuf/test_messages_proto3.proto

@@ -206,6 +206,7 @@ message TestAllTypesProto3 {
   repeated google.protobuf.Struct repeated_struct = 324;
   repeated google.protobuf.Any repeated_any = 315;
   repeated google.protobuf.Value repeated_value = 316;
+  repeated google.protobuf.ListValue repeated_list_value = 317;
 
   // Test field-name-to-JSON-name convention.
   // (protobuf says names can be any valid C/C++ identifier.)