Merge pull request #252 from haberman/jsondecode

JSON decoder and bugfixes to encoder

BUILD

@@ -162,9 +162,11 @@ cc_library(
 cc_library(
     name = "json",
     srcs = [
+        "upb/json_decode.c",
         "upb/json_encode.c",
     ],
     hdrs = [
+        "upb/json_decode.h",
         "upb/json_encode.h",
     ],
     deps = [

CMakeLists.txt

@@ -97,7 +97,9 @@ target_link_libraries(textformat
   port
   reflection)
 add_library(json
+  upb/json_decode.c
   upb/json_encode.c
+  upb/json_decode.h
   upb/json_encode.h)
 target_link_libraries(json
   port

generated_for_cmake/upb/json/parser.c

@@ -3311,15 +3311,13 @@ static upb_json_parsermethod *parsermethod_new(upb_json_codecache *c,
       upb_msg_field_next(&i)) {
     const upb_fielddef *f = upb_msg_iter_field(&i);
     upb_value v = upb_value_constptr(f);
-    char *buf;
+    const char *name;
 
     /* Add an entry for the JSON name. */
-    size_t len = upb_fielddef_getjsonname(f, NULL, 0);
+    name = upb_fielddef_jsonname(f);
-    buf = upb_malloc(alloc, len);
+    upb_strtable_insert3(&m->name_table, name, strlen(name), v, alloc);
-    upb_fielddef_getjsonname(f, buf, len);
-    upb_strtable_insert3(&m->name_table, buf, strlen(buf), v, alloc);
 
-    if (strcmp(buf, upb_fielddef_name(f)) != 0) {
+    if (strcmp(name, upb_fielddef_name(f)) != 0) {
       /* Since the JSON name is different from the regular field name, add an
        * entry for the raw name (compliant proto3 JSON parsers must accept
        * both). */

tests/conformance_upb.c

@@ -15,6 +15,7 @@
 #include "upb/decode.h"
 #include "upb/encode.h"
 #include "upb/reflection.h"
+#include "upb/json_decode.h"
 #include "upb/json_encode.h"
 #include "upb/text_encode.h"
 
@@ -85,9 +86,11 @@ void serialize_text(const upb_msg *msg, const upb_msgdef *m, const ctx *c) {
   size_t len2;
   int opts = 0;
   char *data;
+
   if (!conformance_ConformanceRequest_print_unknown_fields(c->request)) {
     opts |= UPB_TXTENC_SKIPUNKNOWN;
   }
+
   len = upb_text_encode(msg, m, c->symtab, opts, NULL, 0);
   data = upb_arena_malloc(c->arena, len + 1);
   len2 = upb_text_encode(msg, m, c->symtab, opts, data, len + 1);
@@ -96,6 +99,33 @@ void serialize_text(const upb_msg *msg, const upb_msgdef *m, const ctx *c) {
       c->response, upb_strview_make(data, len));
 }
 
+bool parse_json(upb_msg *msg, const upb_msgdef *m, const ctx* c) {
+  upb_strview json =
+      conformance_ConformanceRequest_json_payload(c->request);
+  upb_status status;
+  int opts = 0;
+
+  if (conformance_ConformanceRequest_test_category(c->request) ==
+      conformance_JSON_IGNORE_UNKNOWN_PARSING_TEST) {
+    opts |= UPB_JSONDEC_IGNOREUNKNOWN;
+  }
+
+  upb_status_clear(&status);
+  if (upb_json_decode(json.data, json.size, msg, m, c->symtab, opts, c->arena,
+                      &status)) {
+    return true;
+  } else {
+    const char *inerr = upb_status_errmsg(&status);
+    size_t len = strlen(inerr);
+    char *err = upb_arena_malloc(c->arena, len + 1);
+    memcpy(err, inerr, strlen(inerr));
+    err[len] = '\0';
+    conformance_ConformanceResponse_set_parse_error(c->response,
+                                                    upb_strview_makez(err));
+    return false;
+  }
+}
+
 void serialize_json(const upb_msg *msg, const upb_msgdef *m, const ctx *c) {
   size_t len;
   size_t len2;
@@ -104,16 +134,16 @@ void serialize_json(const upb_msg *msg, const upb_msgdef *m, const ctx *c) {
   upb_status status;
 
   upb_status_clear(&status);
-  if (!conformance_ConformanceRequest_print_unknown_fields(c->request)) {
-    opts |= UPB_TXTENC_SKIPUNKNOWN;
-  }
-
   len = upb_json_encode(msg, m, c->symtab, opts, NULL, 0, &status);
 
   if (len == -1) {
-    static const char msg[] = "Error serializing.";
+    const char *inerr = upb_status_errmsg(&status);
-    conformance_ConformanceResponse_set_serialize_error(
+    size_t len = strlen(inerr);
-        c->response, upb_strview_make(msg, strlen(msg)));
+    char *err = upb_arena_malloc(c->arena, len + 1);
+    memcpy(err, inerr, strlen(inerr));
+    err[len] = '\0';
+    conformance_ConformanceResponse_set_serialize_error(c->response,
+                                                        upb_strview_makez(err));
     return;
   }
 
@@ -128,6 +158,8 @@ bool parse_input(upb_msg *msg, const upb_msgdef *m, const ctx* c) {
   switch (conformance_ConformanceRequest_payload_case(c->request)) {
     case conformance_ConformanceRequest_payload_protobuf_payload:
       return parse_proto(msg, m, c);
+    case conformance_ConformanceRequest_payload_json_payload:
+      return parse_json(msg, m, c);
     case conformance_ConformanceRequest_payload_NOT_SET:
       fprintf(stderr, "conformance_upb: Request didn't have payload.\n");
       return false;

upb/def.c

@@ -27,6 +27,7 @@ struct upb_fielddef {
   const upb_filedef *file;
   const upb_msgdef *msgdef;
   const char *full_name;
+  const char *json_name;
   union {
     int64_t sint;
     uint64_t uint;
@@ -117,10 +118,15 @@ struct upb_symtab {
 
 /* Inside a symtab we store tagged pointers to specific def types. */
 typedef enum {
-  UPB_DEFTYPE_MSG = 0,
+  UPB_DEFTYPE_FIELD = 0,
-  UPB_DEFTYPE_ENUM = 1,
+
-  UPB_DEFTYPE_FIELD = 2,
+  /* Only inside symtab table. */
-  UPB_DEFTYPE_ONEOF = 3
+  UPB_DEFTYPE_MSG = 1,
+  UPB_DEFTYPE_ENUM = 2,
+
+  /* Only inside message table. */
+  UPB_DEFTYPE_ONEOF = 1,
+  UPB_DEFTYPE_FIELD_JSONNAME = 2
 } upb_deftype_t;
 
 static const void *unpack_def(upb_value v, upb_deftype_t type) {
@@ -462,47 +468,12 @@ const char *upb_fielddef_name(const upb_fielddef *f) {
   return shortdefname(f->full_name);
 }
 
-uint32_t upb_fielddef_selectorbase(const upb_fielddef *f) {
+const char *upb_fielddef_jsonname(const upb_fielddef *f) {
-  return f->selector_base;
+  return f->json_name;
-}
-
-size_t upb_fielddef_getjsonname(const upb_fielddef *f, char *buf, size_t len) {
-  const char *name = upb_fielddef_name(f);
-  size_t src, dst = 0;
-  bool ucase_next = false;
-
-#define WRITE(byte) \
-  ++dst; \
-  if (dst < len) buf[dst - 1] = byte; \
-  else if (dst == len) buf[dst - 1] = '\0'
-
-  if (!name) {
-    WRITE('\0');
-    return 0;
-  }
-
-  /* Implement the transformation as described in the spec:
-   *   1. upper case all letters after an underscore.
-   *   2. remove all underscores.
-   */
-  for (src = 0; name[src]; src++) {
-    if (name[src] == '_') {
-      ucase_next = true;
-      continue;
-    }
-
-    if (ucase_next) {
-      WRITE(toupper(name[src]));
-      ucase_next = false;
-    } else {
-      WRITE(name[src]);
-    }
 }
 
-  WRITE('\0');
+uint32_t upb_fielddef_selectorbase(const upb_fielddef *f) {
-  return dst;
+  return f->selector_base;
-
-#undef WRITE
 }
 
 const upb_msgdef *upb_fielddef_containingtype(const upb_fielddef *f) {
@@ -690,18 +661,30 @@ bool upb_msgdef_lookupname(const upb_msgdef *m, const char *name, size_t len,
 
   *o = unpack_def(val, UPB_DEFTYPE_ONEOF);
   *f = unpack_def(val, UPB_DEFTYPE_FIELD);
-  UPB_ASSERT((*o != NULL) ^ (*f != NULL));  /* Exactly one of the two should be set. */
+  return *o || *f;  /* False if this was a JSON name. */
-  return true;
+}
+
+const upb_fielddef *upb_msgdef_lookupjsonname(const upb_msgdef *m,
+                                              const char *name, size_t len) {
+  upb_value val;
+  const upb_fielddef* f;
+
+  if (!upb_strtable_lookup2(&m->ntof, name, len, &val)) {
+    return NULL;
+  }
+
+  f = unpack_def(val, UPB_DEFTYPE_FIELD);
+  if (!f) f = unpack_def(val, UPB_DEFTYPE_FIELD_JSONNAME);
+
+  return f;
 }
 
 int upb_msgdef_numfields(const upb_msgdef *m) {
-  /* The number table contains only fields. */
+  return m->field_count;
-  return (int)upb_inttable_count(&m->itof);
 }
 
 int upb_msgdef_numoneofs(const upb_msgdef *m) {
-  /* The name table includes oneofs, and the number table does not. */
+  return m->oneof_count;
-  return (int)(upb_strtable_count(&m->ntof) - upb_inttable_count(&m->itof));
 }
 
 const upb_msglayout *upb_msgdef_layout(const upb_msgdef *m) {
@@ -1098,6 +1081,51 @@ static const char *makefullname(const symtab_addctx *ctx, const char *prefix,
   }
 }
 
+size_t getjsonname(const char *name, char *buf, size_t len) {
+  size_t src, dst = 0;
+  bool ucase_next = false;
+
+#define WRITE(byte) \
+  ++dst; \
+  if (dst < len) buf[dst - 1] = byte; \
+  else if (dst == len) buf[dst - 1] = '\0'
+
+  if (!name) {
+    WRITE('\0');
+    return 0;
+  }
+
+  /* Implement the transformation as described in the spec:
+   *   1. upper case all letters after an underscore.
+   *   2. remove all underscores.
+   */
+  for (src = 0; name[src]; src++) {
+    if (name[src] == '_') {
+      ucase_next = true;
+      continue;
+    }
+
+    if (ucase_next) {
+      WRITE(toupper(name[src]));
+      ucase_next = false;
+    } else {
+      WRITE(name[src]);
+    }
+  }
+
+  WRITE('\0');
+  return dst;
+
+#undef WRITE
+}
+
+static char* makejsonname(const char* name, upb_alloc *alloc) {
+  size_t size = getjsonname(name, NULL, 0);
+  char* json_name = upb_malloc(alloc, size);
+  getjsonname(name, json_name, size);
+  return json_name;
+}
+
 static bool symtab_add(const symtab_addctx *ctx, const char *name,
                        upb_value v) {
   upb_value tmp;
@@ -1311,6 +1339,7 @@ static bool create_fielddef(
   const google_protobuf_FieldOptions *options;
   upb_strview name;
   const char *full_name;
+  const char *json_name;
   const char *shortname;
   uint32_t field_number;
 
@@ -1324,6 +1353,13 @@ static bool create_fielddef(
   full_name = makefullname(ctx, prefix, name);
   shortname = shortdefname(full_name);
 
+  if (google_protobuf_FieldDescriptorProto_has_json_name(field_proto)) {
+    json_name = strviewdup(
+        ctx, google_protobuf_FieldDescriptorProto_json_name(field_proto));
+  } else {
+    json_name = makejsonname(shortname, ctx->alloc);
+  }
+
   field_number = google_protobuf_FieldDescriptorProto_number(field_proto);
 
   if (field_number == 0 || field_number > UPB_MAX_FIELDNUMBER) {
@@ -1333,26 +1369,42 @@ static bool create_fielddef(
 
   if (m) {
     /* direct message field. */
-    upb_value v, packed_v;
+    upb_value v, field_v, json_v;
+    size_t json_size;
 
     f = (upb_fielddef*)&m->fields[m->field_count++];
     f->msgdef = m;
     f->is_extension_ = false;
 
-    packed_v = pack_def(f, UPB_DEFTYPE_FIELD);
+    if (upb_strtable_lookup(&m->ntof, shortname, NULL)) {
-    v = upb_value_constptr(f);
-
-    if (!upb_strtable_insert3(&m->ntof, name.data, name.size, packed_v, alloc)) {
       upb_status_seterrf(ctx->status, "duplicate field name (%s)", shortname);
       return false;
     }
 
-    if (!upb_inttable_insert2(&m->itof, field_number, v, alloc)) {
+    if (upb_strtable_lookup(&m->ntof, json_name, NULL)) {
+      upb_status_seterrf(ctx->status, "duplicate json_name (%s)", json_name);
+      return false;
+    }
+
+    if (upb_inttable_lookup(&m->itof, field_number, NULL)) {
       upb_status_seterrf(ctx->status, "duplicate field number (%u)",
                          field_number);
       return false;
     }
 
+    field_v = pack_def(f, UPB_DEFTYPE_FIELD);
+    json_v = pack_def(f, UPB_DEFTYPE_FIELD_JSONNAME);
+    v = upb_value_constptr(f);
+    json_size = strlen(json_name);
+
+    CHK_OOM(
+        upb_strtable_insert3(&m->ntof, name.data, name.size, field_v, alloc));
+    CHK_OOM(upb_inttable_insert2(&m->itof, field_number, v, alloc));
+
+    if (strcmp(shortname, json_name) != 0) {
+      upb_strtable_insert3(&m->ntof, json_name, json_size, json_v, alloc);
+    }
+
     if (ctx->layouts) {
       const upb_msglayout_field *fields = m->layout->fields;
       int count = m->layout->field_count;
@@ -1369,12 +1421,13 @@ static bool create_fielddef(
     }
   } else {
     /* extension field. */
-    f = (upb_fielddef*)&ctx->file->exts[ctx->file->ext_count];
+    f = (upb_fielddef*)&ctx->file->exts[ctx->file->ext_count++];
     f->is_extension_ = true;
     CHK_OOM(symtab_add(ctx, full_name, pack_def(f, UPB_DEFTYPE_FIELD)));
   }
 
   f->full_name = full_name;
+  f->json_name = json_name;
   f->file = ctx->file;
   f->type_ = (int)google_protobuf_FieldDescriptorProto_type(field_proto);
   f->label_ = (int)google_protobuf_FieldDescriptorProto_label(field_proto);
@@ -1741,7 +1794,8 @@ static bool build_filedef(
     } else if (streql_view(syntax, "proto3")) {
       file->syntax = UPB_SYNTAX_PROTO3;
     } else {
-      upb_status_seterrf(ctx->status, "Invalid syntax '%s'", syntax);
+      upb_status_seterrf(ctx->status, "Invalid syntax '" UPB_STRVIEW_FORMAT "'",
+                         UPB_STRVIEW_ARGS(syntax));
       return false;
     }
   } else {

upb/def.h

@@ -99,10 +99,10 @@ upb_descriptortype_t upb_fielddef_descriptortype(const upb_fielddef *f);
 upb_label_t upb_fielddef_label(const upb_fielddef *f);
 uint32_t upb_fielddef_number(const upb_fielddef *f);
 const char *upb_fielddef_name(const upb_fielddef *f);
+const char *upb_fielddef_jsonname(const upb_fielddef *f);
 bool upb_fielddef_isextension(const upb_fielddef *f);
 bool upb_fielddef_lazy(const upb_fielddef *f);
 bool upb_fielddef_packed(const upb_fielddef *f);
-size_t upb_fielddef_getjsonname(const upb_fielddef *f, char *buf, size_t len);
 const upb_msgdef *upb_fielddef_containingtype(const upb_fielddef *f);
 const upb_oneofdef *upb_fielddef_containingoneof(const upb_fielddef *f);
 uint32_t upb_fielddef_index(const upb_fielddef *f);
@@ -151,32 +151,10 @@ class upb::FieldDefPtr {
   Type type() const { return upb_fielddef_type(ptr_); }
   Label label() const { return upb_fielddef_label(ptr_); }
   const char* name() const { return upb_fielddef_name(ptr_); }
+  const char* json_name() const { return upb_fielddef_jsonname(ptr_); }
   uint32_t number() const { return upb_fielddef_number(ptr_); }
   bool is_extension() const { return upb_fielddef_isextension(ptr_); }
 
-  /* Copies the JSON name for this field into the given buffer.  Returns the
-   * actual size of the JSON name, including the NULL terminator.  If the
-   * return value is 0, the JSON name is unset.  If the return value is
-   * greater than len, the JSON name was truncated.  The buffer is always
-   * NULL-terminated if len > 0.
-   *
-   * The JSON name always defaults to a camelCased version of the regular
-   * name.  However if the regular name is unset, the JSON name will be unset
-   * also.
-   */
-  size_t GetJsonName(char *buf, size_t len) const {
-    return upb_fielddef_getjsonname(ptr_, buf, len);
-  }
-
-  /* Convenience version of the above function which copies the JSON name
-   * into the given string, returning false if the name is not set. */
-  template <class T>
-  bool GetJsonName(T* str) {
-    str->resize(GetJsonName(NULL, 0));
-    GetJsonName(&(*str)[0], str->size());
-    return str->size() > 0;
-  }
-
   /* For UPB_TYPE_MESSAGE fields only where is_tag_delimited() == false,
    * indicates whether this field should have lazy parsing handlers that yield
    * the unparsed string for the submessage.
@@ -455,6 +433,10 @@ UPB_INLINE bool upb_msgdef_lookupnamez(const upb_msgdef *m, const char *name,
   return upb_msgdef_lookupname(m, name, strlen(name), f, o);
 }
 
+/* Returns a field by either JSON name or regular proto name. */
+const upb_fielddef *upb_msgdef_lookupjsonname(const upb_msgdef *m,
+                                              const char *name, size_t len);
+
 /* Iteration over fields and oneofs.  For example:
  *
  * upb_msg_field_iter i;

upb/json/parser.rl

@@ -2874,15 +2874,13 @@ static upb_json_parsermethod *parsermethod_new(upb_json_codecache *c,
       upb_msg_field_next(&i)) {
     const upb_fielddef *f = upb_msg_iter_field(&i);
     upb_value v = upb_value_constptr(f);
-    char *buf;
+    const char *name;
 
     /* Add an entry for the JSON name. */
-    size_t len = upb_fielddef_getjsonname(f, NULL, 0);
+    name = upb_fielddef_jsonname(f);
-    buf = upb_malloc(alloc, len);
+    upb_strtable_insert3(&m->name_table, name, strlen(name), v, alloc);
-    upb_fielddef_getjsonname(f, buf, len);
-    upb_strtable_insert3(&m->name_table, buf, strlen(buf), v, alloc);
 
-    if (strcmp(buf, upb_fielddef_name(f)) != 0) {
+    if (strcmp(name, upb_fielddef_name(f)) != 0) {
       /* Since the JSON name is different from the regular field name, add an
        * entry for the raw name (compliant proto3 JSON parsers must accept
        * both). */

upb/json/printer.c

@@ -65,12 +65,8 @@ strpc *newstrpc(upb_handlers *h, const upb_fielddef *f,
     ret->ptr = upb_gstrdup(upb_fielddef_name(f));
     ret->len = strlen(ret->ptr);
   } else {
-    size_t len;
+    ret->ptr = upb_gstrdup(upb_fielddef_jsonname(f));
-    ret->len = upb_fielddef_getjsonname(f, NULL, 0);
+    ret->len = strlen(ret->ptr);
-    ret->ptr = upb_gmalloc(ret->len);
-    len = upb_fielddef_getjsonname(f, ret->ptr, ret->len);
-    UPB_ASSERT(len == ret->len);
-    ret->len--;  /* NULL */
   }
 
   upb_handlers_addcleanup(h, ret, freestrpc);

upb/json_decode.h

@@ -0,0 +1,24 @@
+
+#ifndef UPB_JSONDECODE_H_
+#define UPB_JSONDECODE_H_
+
+#include "upb/def.h"
+#include "upb/msg.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+enum {
+  UPB_JSONDEC_IGNOREUNKNOWN = 1
+};
+
+bool upb_json_decode(const char *buf, size_t size, upb_msg *msg,
+                     const upb_msgdef *m, const upb_symtab *any_pool,
+                     int options, upb_arena *arena, upb_status *status);
+
+#ifdef __cplusplus
+}  /* extern "C" */
+#endif
+
+#endif  /* UPB_JSONDECODE_H_ */

upb/json_encode.c

@@ -29,13 +29,23 @@ static void jsonenc_msg(jsonenc *e, const upb_msg *msg, const upb_msgdef *m);
 static void jsonenc_scalar(jsonenc *e, upb_msgval val, const upb_fielddef *f);
 static void jsonenc_msgfield(jsonenc *e, const upb_msg *msg,
                              const upb_msgdef *m);
+static void jsonenc_msgfields(jsonenc *e, const upb_msg *msg,
+                              const upb_msgdef *m);
 static void jsonenc_value(jsonenc *e, const upb_msg *msg, const upb_msgdef *m);
 
-static void jsonenc_err(jsonenc *e, const char *msg) {
+UPB_NORETURN static void jsonenc_err(jsonenc *e, const char *msg) {
   upb_status_seterrmsg(e->status, msg);
   longjmp(e->err, 1);
 }
 
+static upb_arena *jsonenc_arena(jsonenc *e) {
+  /* Create lazily, since it's only needed for Any */
+  if (!e->arena) {
+    e->arena = upb_arena_new();
+  }
+  return e->arena;
+}
+
 static void jsonenc_putbytes(jsonenc *e, const void *data, size_t len) {
   size_t have = e->end - e->ptr;
   if (UPB_LIKELY(have >= len)) {
@@ -70,19 +80,19 @@ static void jsonenc_printf(jsonenc *e, const char *fmt, ...) {
 }
 
 static void jsonenc_nanos(jsonenc *e, int32_t nanos) {
-  const char zeros[3] = "000";
+  int digits = 9;
 
   if (nanos == 0) return;
   if (nanos < 0 || nanos >= 1000000000) {
     jsonenc_err(e, "error formatting timestamp as JSON: invalid nanos");
   }
 
-  jsonenc_printf(e, "%09" PRId32, nanos);
+  while (nanos % 1000 == 0) {
-
+    nanos /= 1000;
-  /* Remove trailing zeros, 3 at a time. */
+    digits -= 3;
-  while ((e->ptr - e->buf) >= 3 && memcmp(e->ptr, zeros, 3) == 0) {
-    e->ptr -= 3;
   }
+
+  jsonenc_printf(e, ".%0.*" PRId32, digits, nanos);
 }
 
 static void jsonenc_timestamp(jsonenc *e, const upb_msg *msg,
@@ -107,7 +117,7 @@ static void jsonenc_timestamp(jsonenc *e, const upb_msg *msg,
    * Fliegel, H. F., and Van Flandern, T. C., "A Machine Algorithm for
    *   Processing Calendar Dates," Communications of the Association of
    *   Computing Machines, vol. 11 (1968), p. 657.  */
-  L = (seconds / 86400) + 2440588;
+  L = (seconds / 86400) + 68569 + 2440588;
   N = 4 * L / 146097;
   L = L - (146097 * N + 3) / 4;
   I = 4000 * (L + 1) / 1461001;
@@ -138,6 +148,10 @@ static void jsonenc_duration(jsonenc *e, const upb_msg *msg, const upb_msgdef *m
     jsonenc_err(e, "bad duration");
   }
 
+  if (nanos < 0) {
+    nanos = -nanos;
+  }
+
   jsonenc_printf(e, "\"%" PRId64, seconds);
   jsonenc_nanos(e, nanos);
   jsonenc_putstr(e, "s\"");
@@ -158,8 +172,8 @@ static void jsonenc_bytes(jsonenc *e, upb_strview str) {
   /* This is the regular base64, not the "web-safe" version. */
   static const char base64[] =
       "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
-  const char *ptr = str.data;
+  const unsigned char *ptr = (unsigned char*)str.data;
-  const char *end = ptr + str.size;
+  const unsigned char *end = ptr + str.size;
   char buf[4];
 
   jsonenc_putstr(e, "\"");
@@ -212,10 +226,10 @@ static void jsonenc_stringbody(jsonenc *e, upb_strview str) {
         jsonenc_putstr(e, "\\\"");
         break;
       case '\f':
-        jsonenc_putstr(e, "\f'");
+        jsonenc_putstr(e, "\\f");
         break;
       case '\b':
-        jsonenc_putstr(e, "\b'");
+        jsonenc_putstr(e, "\\b");
         break;
       case '\\':
         jsonenc_putstr(e, "\\\\");
@@ -255,21 +269,22 @@ static void jsonenc_double(jsonenc *e, const char *fmt, double val) {
 static void jsonenc_wrapper(jsonenc *e, const upb_msg *msg,
                             const upb_msgdef *m) {
   const upb_fielddef *val_f = upb_msgdef_itof(m, 1);
-  upb_msgval val = upb_msg_get(m, val_f);
+  upb_msgval val = upb_msg_get(msg, val_f);
   jsonenc_scalar(e, val, val_f);
 }
 
-const upb_msgdef *jsonenc_getanymsg(jsonenc *e, upb_strview type_url) {
+static const upb_msgdef *jsonenc_getanymsg(jsonenc *e, upb_strview type_url) {
   /* Find last '/', if any. */
   const char *end = type_url.data + type_url.size;
   const char *ptr = end;
+  const upb_msgdef *ret;
 
-  if (!e->ext_pool || type_url.size == 0) return NULL;
+  if (!e->ext_pool || type_url.size == 0) goto badurl;
 
   while (true) {
     if (--ptr == type_url.data) {
       /* Type URL must contain at least one '/', with host before. */
-      return NULL;
+      goto badurl;
     }
     if (*ptr == '/') {
       ptr++;
@@ -277,19 +292,29 @@ const upb_msgdef *jsonenc_getanymsg(jsonenc *e, upb_strview type_url) {
     }
   }
 
-  return upb_symtab_lookupmsg2(e->ext_pool, ptr, end - ptr);
+  ret = upb_symtab_lookupmsg2(e->ext_pool, ptr, end - ptr);
+
+  if (!ret) {
+    jsonenc_err(e, "Couldn't find Any type");
+  }
+
+  return ret;
+
+badurl:
+  jsonenc_err(e, "Bad type URL");
 }
 
 static void jsonenc_any(jsonenc *e, const upb_msg *msg, const upb_msgdef *m) {
   const upb_fielddef *type_url_f = upb_msgdef_itof(m, 1);
-  const upb_fielddef *value_f = upb_msgdef_itof(m, 1);
+  const upb_fielddef *value_f = upb_msgdef_itof(m, 2);
   upb_strview type_url = upb_msg_get(msg, type_url_f).str_val;
   upb_strview value = upb_msg_get(msg, value_f).str_val;
   const upb_msgdef *any_m = jsonenc_getanymsg(e, type_url);
   const upb_msglayout *any_layout = upb_msgdef_layout(any_m);
-  upb_msg *any = upb_msg_new(any_m, e->arena);
+  upb_arena *arena = jsonenc_arena(e);
+  upb_msg *any = upb_msg_new(any_m, arena);
 
-  if (!upb_decode(value.data, value.size, any, any_layout, e->arena)) {
+  if (!upb_decode(value.data, value.size, any, any_layout, arena)) {
     jsonenc_err(e, "Error decoding message in Any");
   }
 
@@ -297,9 +322,9 @@ static void jsonenc_any(jsonenc *e, const upb_msg *msg, const upb_msgdef *m) {
   jsonenc_string(e, type_url);
   jsonenc_putstr(e, ", ");
 
-  if (upb_msgdef_wellknowntype(m) == UPB_WELLKNOWN_UNSPECIFIED) {
+  if (upb_msgdef_wellknowntype(any_m) == UPB_WELLKNOWN_UNSPECIFIED) {
     /* Regular messages: {"@type": "...", "foo": 1, "bar": 2} */
-    jsonenc_msg(e, any, any_m);
+    jsonenc_msgfields(e, any, any_m);
   } else {
     /* Well-known type: {"@type": "...", "value": <well-known encoding>} */
     jsonenc_putstr(e, "value: ");
@@ -323,15 +348,17 @@ static void jsonenc_fieldpath(jsonenc *e, upb_strview path) {
 
   while (ptr < end) {
     char ch = *ptr;
+
     if (ch >= 'A' && ch <= 'Z') {
       jsonenc_err(e, "Field mask element may not have upper-case letter.");
     } else if (ch == '_') {
       if (ptr == end - 1 || *(ptr + 1) < 'a' || *(ptr + 1) > 'z') {
         jsonenc_err(e, "Underscore must be followed by a lowercase letter.");
       }
-    } else {
+      ch = *++ptr - 32;
-      jsonenc_putbytes(e, &ch, 1);
     }
+
+    jsonenc_putbytes(e, &ch, 1);
     ptr++;
   }
 }
@@ -468,7 +495,7 @@ static void jsonenc_msgfield(jsonenc *e, const upb_msg *msg,
       jsonenc_listvalue(e, msg, m);
       break;
     case UPB_WELLKNOWN_STRUCT:
-      jsonenc_listvalue(e, msg, m);
+      jsonenc_struct(e, msg, m);
       break;
   }
 }
@@ -532,6 +559,7 @@ static void jsonenc_mapkey(jsonenc *e, upb_msgval val, const upb_fielddef *f) {
       break;
     case UPB_TYPE_STRING:
       jsonenc_stringbody(e, val.str_val);
+      break;
     default:
       UPB_UNREACHABLE();
   }
@@ -575,15 +603,12 @@ static void jsonenc_map(jsonenc *e, const upb_map *map, const upb_fielddef *f) {
 
 static void jsonenc_fieldval(jsonenc *e, const upb_fielddef *f,
                              upb_msgval val, bool *first) {
-  char buf[128];
   const char *name;
 
   if (e->options & UPB_JSONENC_PROTONAMES) {
     name = upb_fielddef_name(f);
   } else {
-    /* TODO(haberman): we need a better JSON name API. */
+    name = upb_fielddef_jsonname(f);
-    upb_fielddef_getjsonname(f, buf, sizeof(buf));
-    name = buf;
   }
 
   jsonenc_putsep(e, ", ", first);
@@ -598,13 +623,12 @@ static void jsonenc_fieldval(jsonenc *e, const upb_fielddef *f,
   }
 }
 
-static void jsonenc_msg(jsonenc *e, const upb_msg *msg, const upb_msgdef *m) {
+static void jsonenc_msgfields(jsonenc *e, const upb_msg *msg,
+                              const upb_msgdef *m) {
   upb_msgval val;
   const upb_fielddef *f;
   bool first = true;
 
-  jsonenc_putstr(e, "{");
-
   if (e->options & UPB_JSONENC_EMITDEFAULTS) {
     /* Iterate over all fields. */
     upb_msg_field_iter i;
@@ -620,11 +644,15 @@ static void jsonenc_msg(jsonenc *e, const upb_msg *msg, const upb_msgdef *m) {
       jsonenc_fieldval(e, f, val, &first);
     }
   }
+}
 
+static void jsonenc_msg(jsonenc *e, const upb_msg *msg, const upb_msgdef *m) {
+  jsonenc_putstr(e, "{");
+  jsonenc_msgfields(e, msg, m);
   jsonenc_putstr(e, "}");
 }
 
-size_t jsonenc_nullz(jsonenc *e, size_t size) {
+static size_t jsonenc_nullz(jsonenc *e, size_t size) {
   size_t ret = e->ptr - e->buf + e->overflow;
 
   if (size > 0) {
@@ -647,9 +675,11 @@ size_t upb_json_encode(const upb_msg *msg, const upb_msgdef *m,
   e.options = options;
   e.ext_pool = ext_pool;
   e.status = status;
+  e.arena = NULL;
 
   if (setjmp(e.err)) return -1;
 
   jsonenc_msg(&e, msg, m);
+  if (e.arena) upb_arena_free(e.arena);
   return jsonenc_nullz(&e, size);
 }

upb/reflection.c

@@ -90,6 +90,18 @@ bool upb_msg_has(const upb_msg *msg, const upb_fielddef *f) {
   }
 }
 
+bool upb_msg_hasoneof(const upb_msg *msg, const upb_oneofdef *o) {
+  upb_oneof_iter i;
+  const upb_fielddef *f;
+  const upb_msglayout_field *field;
+
+  upb_oneof_begin(&i, o);
+  if (upb_oneof_done(&i)) return false;
+  f = upb_oneof_iter_field(&i);
+  field = upb_fielddef_layout(f);
+  return *oneofcase(msg, field) != 0;
+}
+
 upb_msgval upb_msg_get(const upb_msg *msg, const upb_fielddef *f) {
   if (!upb_fielddef_haspresence(f) || upb_msg_has(msg, f)) {
     return _upb_msg_getraw(msg, f);
@@ -136,8 +148,11 @@ upb_mutmsgval upb_msg_mutable(upb_msg *msg, const upb_fielddef *f,
   const upb_msglayout_field *field = upb_fielddef_layout(f);
   upb_mutmsgval ret;
   char *mem = PTR_AT(msg, field->offset, char);
+  bool wrong_oneof = in_oneof(field) && *oneofcase(msg, field) != field->number;
+
   memcpy(&ret, mem, sizeof(void*));
-  if (a && !ret.msg) {
+
+  if (a && (!ret.msg || wrong_oneof)) {
     if (upb_fielddef_ismap(f)) {
       const upb_msgdef *entry = upb_fielddef_msgsubdef(f);
       const upb_fielddef *key = upb_msgdef_itof(entry, UPB_MAPENTRY_KEY);
@@ -149,7 +164,12 @@ upb_mutmsgval upb_msg_mutable(upb_msg *msg, const upb_fielddef *f,
       UPB_ASSERT(upb_fielddef_issubmsg(f));
       ret.msg = upb_msg_new(upb_fielddef_msgsubdef(f), a);
     }
+
     memcpy(mem, &ret, sizeof(void*));
+
+    if (wrong_oneof) {
+      *oneofcase(msg, field) = field->number;
+    }
   }
   return ret;
 }

upb/reflection.h

@@ -44,6 +44,9 @@ upb_mutmsgval upb_msg_mutable(upb_msg *msg, const upb_fielddef *f, upb_arena *a)
 /* May only be called for fields where upb_fielddef_haspresence(f) == true. */
 bool upb_msg_has(const upb_msg *msg, const upb_fielddef *f);
 
+/* Returns whether any field is set in the oneof. */
+bool upb_msg_hasoneof(const upb_msg *msg, const upb_oneofdef *o);
+
 /* Sets the given field to the given value.  For a msg/array/map/string, the
  * value must be in the same arena.  */
 void upb_msg_set(upb_msg *msg, const upb_fielddef *f, upb_msgval val,