diff --git a/runtime/pb/c/c_runtime.c b/runtime/pb/c/c_runtime.c index 66aba31fd09f46c66e60fd4ebd16ab9e7c930b6c..b8523ac0317c73308c4d32c98ab31e75b2d26122 100644 --- a/runtime/pb/c/c_runtime.c +++ b/runtime/pb/c/c_runtime.c @@ -2109,6 +2109,58 @@ { return (int64_t)((u >> 1) ^ (~(u & 1) + 1)); } +/* RFC 3629 UTF-8 validator. Mirrors the rules enforced by Tarantool's + * ICU-backed utf8.len (overlong, lone surrogate U+D800..U+DFFF, code + * points > U+10FFFF, truncated multi-byte sequences). Returns 1 if the + * byte run is valid UTF-8, 0 otherwise. Proto3 (and proto2 string + * fields, per the recommended profile) requires this on the wire — the + * pure-Lua decoder calls utf8.len() via wire.lua. */ +static int +is_valid_utf8(const uint8_t *s, size_t n) +{ + size_t i = 0; + while (i < n) { + uint8_t c = s[i]; + if (c < 0x80) { + i++; + } else if ((c & 0xE0) == 0xC0) { + /* 2-byte sequence. 0xC0/0xC1 are overlong. */ + if (c < 0xC2) return 0; + if (i + 1 >= n) return 0; + if ((s[i + 1] & 0xC0) != 0x80) return 0; + i += 2; + } else if ((c & 0xF0) == 0xE0) { + /* 3-byte sequence. */ + if (i + 2 >= n) return 0; + uint8_t b1 = s[i + 1], b2 = s[i + 2]; + if ((b1 & 0xC0) != 0x80) return 0; + if ((b2 & 0xC0) != 0x80) return 0; + /* 0xE0 with b1 < 0xA0 is overlong (< U+0800). */ + if (c == 0xE0 && b1 < 0xA0) return 0; + /* 0xED with b1 >= 0xA0 is a UTF-16 surrogate + * (U+D800..U+DFFF), forbidden in UTF-8. */ + if (c == 0xED && b1 >= 0xA0) return 0; + i += 3; + } else if ((c & 0xF8) == 0xF0) { + /* 4-byte sequence. 0xF5..0xFF would encode > U+10FFFF. */ + if (c > 0xF4) return 0; + if (i + 3 >= n) return 0; + uint8_t b1 = s[i + 1], b2 = s[i + 2], b3 = s[i + 3]; + if ((b1 & 0xC0) != 0x80) return 0; + if ((b2 & 0xC0) != 0x80) return 0; + if ((b3 & 0xC0) != 0x80) return 0; + /* 0xF0 with b1 < 0x90 is overlong (< U+10000). */ + if (c == 0xF0 && b1 < 0x90) return 0; + /* 0xF4 with b1 >= 0x90 is > U+10FFFF. */ + if (c == 0xF4 && b1 >= 0x90) return 0; + i += 4; + } else { + return 0; + } + } + return 1; +} + /* Decode a single value of the given `kind` from the stream and push it * onto the Lua stack. Per-kind Lua representations match wire.lua's * decoder (cdata int64/uint64 for 64-bit kinds, Lua number/integer for @@ -2195,7 +2247,19 @@ pun.u = u; lua_pushnumber(c->L, pun.d); return; } - case PB_KIND_STRING: + case PB_KIND_STRING: { + uint64_t plen = dec_varint(c); + if (c->len - c->pos < plen) + luaL_error(c->L, "truncated string/bytes payload"); + if (!is_valid_utf8(c->buf + c->pos, (size_t)plen)) + luaL_error(c->L, + "invalid UTF-8 in string field at offset %d", + (int)c->pos); + lua_pushlstring(c->L, (const char *)(c->buf + c->pos), + (size_t)plen); + c->pos += (size_t)plen; + return; + } case PB_KIND_BYTES: { uint64_t plen = dec_varint(c); if (c->len - c->pos < plen) @@ -2235,6 +2299,8 @@ int sub_plans_idx, int result_idx); static void decode_map_entry(dec_ctx *c, pb_plan_field *f, int sub_plans_idx, int map_idx); static inline int field_is_packable(const pb_plan_field *f); +static void merge_subresult_into(lua_State *L, pb_plan *desc, + int prev_idx, int sub_idx); /* Decode one singular sub-message field. On entry, `c->pos` points at * the length-varint byte; on exit, `c->pos == c->pos + plen`. Pushes @@ -2387,7 +2453,31 @@ if (ext->is_group) decode_group_field(c, ext, sub_plans_idx); else decode_submessage_field(c, ext, sub_plans_idx); - lua_setfield(L, exts_idx, key); + /* stack: ..., exts, sub_result */ + + /* Repeated wire occurrence of a singular message extension + * must merge (bd-rc8 — same rule as in-message fields). */ + pb_plan *subplan_for_merge = NULL; + if (ext->sub_plan_idx > 0) { + lua_rawgeti(L, sub_plans_idx, ext->sub_plan_idx); + subplan_for_merge = + (pb_plan *)lua_touserdata(L, -1); + lua_pop(L, 1); + } + lua_getfield(L, exts_idx, key); + /* stack: ..., sub_result, prev_or_nil */ + if (lua_istable(L, -1) && lua_istable(L, -2) && + subplan_for_merge != NULL && + subplan_for_merge->override_decode_ref == LUA_NOREF) { + int prev_idx = lua_gettop(L); + int sub_idx = prev_idx - 1; + merge_subresult_into(L, subplan_for_merge, + prev_idx, sub_idx); + lua_pop(L, 2); /* prev, sub_result */ + } else { + lua_pop(L, 1); /* pop prev/nil */ + lua_setfield(L, exts_idx, key); /* exts[key] = sub_result */ + } } else { dec_push_one(c, ext); lua_setfield(L, exts_idx, key); @@ -2521,6 +2611,110 @@ return 1; } } +/* Merge `sub_idx` (a freshly-decoded sub-table) into `prev_idx` (the + * sub-table already at result[name] from a prior wire occurrence) per + * proto3 spec: + * - scalar / enum fields: last-wins (replace) + * - repeated fields: concatenate (append decoded elements) + * - map fields: last-wins per key + * - sub-message fields: recursive merge (unless WKT override) + * Mirrors runtime/pb/codec.lua's merge_message — exercised on the wire + * when a singular message field (including a oneof message branch) + * appears more than once. `prev_idx` is modified in-place; `sub_idx` is + * left on the stack for the caller to drop. + * + * Both indices must be absolute (use abs_idx() before calling). */ +static void +merge_subresult_into(lua_State *L, pb_plan *desc, int prev_idx, int sub_idx) +{ + lua_rawgeti(L, LUA_REGISTRYINDEX, desc->field_names_ref); + int names_idx = lua_gettop(L); + lua_rawgeti(L, LUA_REGISTRYINDEX, desc->sub_plans_ref); + int sub_plans_idx = lua_gettop(L); + + for (int i = 0; i < desc->n_fields; i++) { + pb_plan_field *f = &desc->fields[i]; + + /* sub[name] */ + lua_rawgeti(L, names_idx, i + 1); /* name */ + lua_pushvalue(L, -1); + lua_rawget(L, sub_idx); /* name, v */ + if (lua_isnil(L, -1)) { + lua_pop(L, 2); /* nil, name */ + continue; + } + + /* prev[name] */ + lua_pushvalue(L, -2); /* dup name */ + lua_rawget(L, prev_idx); /* name, v, pv */ + + if (lua_isnil(L, -1)) { + /* prev[name] absent — assign. */ + lua_pop(L, 1); /* pop nil */ + lua_rawset(L, prev_idx); /* prev[name] = v */ + continue; + } + + if (f->kind == PB_KIND_MAP && lua_istable(L, -1) && + lua_istable(L, -2)) { + /* Map: copy v's pairs into pv, last-wins per key. */ + int v_idx = lua_gettop(L) - 1; + int pv_idx = lua_gettop(L); + lua_pushnil(L); + while (lua_next(L, v_idx) != 0) { + /* stack: ..., key, val */ + lua_pushvalue(L, -2); /* key */ + lua_pushvalue(L, -2); /* val */ + lua_rawset(L, pv_idx); + lua_pop(L, 1); /* pop val */ + } + lua_pop(L, 3); /* pv, v, name */ + } else if (f->repeated && lua_istable(L, -1) && + lua_istable(L, -2)) { + /* Repeated: concat v[1..#v] onto pv. */ + int v_idx = lua_gettop(L) - 1; + int pv_idx = lua_gettop(L); + int pv_len = (int)lua_objlen(L, pv_idx); + int v_len = (int)lua_objlen(L, v_idx); + for (int j = 1; j <= v_len; j++) { + lua_rawgeti(L, v_idx, j); + lua_rawseti(L, pv_idx, pv_len + j); + } + lua_pop(L, 3); /* pv, v, name */ + } else if (f->kind == PB_KIND_MESSAGE && + lua_istable(L, -1) && lua_istable(L, -2)) { + /* Singular message: recurse, unless WKT (custom + * decode value is opaque — fall through to replace). */ + pb_plan *sub_desc = NULL; + if (f->sub_plan_idx > 0) { + lua_rawgeti(L, sub_plans_idx, + f->sub_plan_idx); + sub_desc = (pb_plan *)lua_touserdata(L, -1); + lua_pop(L, 1); + } + if (sub_desc != NULL && + sub_desc->override_decode_ref == LUA_NOREF) { + int v_idx = lua_gettop(L) - 1; + int pv_idx = lua_gettop(L); + merge_subresult_into(L, sub_desc, + pv_idx, v_idx); + lua_pop(L, 3); /* pv, v, name */ + } else { + /* WKT — last-wins; replace prev[name] with v. */ + lua_pop(L, 1); /* pop pv */ + lua_rawset(L, prev_idx); /* name, v */ + } + } else { + /* Scalar / enum / oneof-cleared / type mismatch: + * last-wins. Replace prev[name] with v. */ + lua_pop(L, 1); /* pop pv */ + lua_rawset(L, prev_idx); /* name, v */ + } + } + + lua_pop(L, 2); /* sub_plans, names */ +} + static void decode_body(dec_ctx *c, pb_plan *plan, int result_idx, uint32_t stop_group_id) @@ -2580,12 +2774,46 @@ * carry their own _unknown_fields, isolated from the parent. */ enc_buf unknown; ebuf_init(&unknown); + /* Track whether the EGROUP was actually observed when decoding a + * group body, so an unterminated SGROUP fails loudly instead of + * silently returning at end-of-buffer. Mirrors wire.lua's behavior + * for the known-field-group case (see bd-rc8). */ + int egroup_seen = 0; + while (c->pos < c->len) { size_t tag_start = c->pos; uint64_t tag = dec_varint(c); - uint32_t field_number = (uint32_t)(tag >> 3); + uint64_t field_number64 = tag >> 3; + uint32_t field_number = (uint32_t)field_number64; uint8_t wt = (uint8_t)(tag & 0x07); + /* Tag validation (bd-rc8 — strict-decode parity with wire.lua's + * decode_tag). The pure-Lua decoder rejects each of these; the + * C path now matches: + * - wire types 6 and 7 are reserved/invalid (legal: 0..5) + * - field number 0 is illegal + * - field number > 2^29-1 is illegal (29-bit per spec) + * - the tag varint must be minimum-length (no overlong + * encodings). dec_varint already enforces the 10-byte + * hard limit; the trailing-zero check catches padded + * encodings that fit in fewer bytes. */ + if (wt >= 6) + luaL_error(L, + "illegal wire type %d at offset %d", + (int)wt, (int)tag_start); + if (field_number == 0) + luaL_error(L, + "illegal field number 0 at offset %d", + (int)tag_start); + if (c->pos - tag_start > 1 && c->buf[c->pos - 1] == 0) + luaL_error(L, + "overlong tag varint at offset %d", + (int)tag_start); + if (field_number64 > 0x1FFFFFFFULL) + luaL_error(L, + "field number out of range at offset %d", + (int)tag_start); + /* Proto2 group body: EGROUP with matching id terminates this * decode_body call. A mismatched id is a hard error per spec. */ if (wt == PB_WIRE_EGROUP) { @@ -2599,6 +2827,7 @@ "EGROUP id %d does not match SGROUP id %d", (int)field_number, (int)stop_group_id); /* Successful close — drop into the unknown-fields * tail handling below. */ + egroup_seen = 1; break; } @@ -2749,9 +2978,43 @@ decode_group_field(c, f, sub_plans_idx); else decode_submessage_field(c, f, sub_plans_idx); /* stack: ..., names, sub_plans, [lists...], sub_result */ + + /* Singular-message merge (bd-rc8): a repeated wire + * occurrence of a singular message field must merge + * into the previous value, not replace it (proto3 + * spec; mirrors codec.lua's merge_message). WKT + * subplans with a custom decode skip the merge — + * their decoded value is opaque (often not a table). + * + * Resolve the subplan first; the sub_result table + * sits at stack top until we either merge or assign. + */ + pb_plan *subplan_for_merge = NULL; + if (f->sub_plan_idx > 0) { + lua_rawgeti(L, sub_plans_idx, f->sub_plan_idx); + subplan_for_merge = + (pb_plan *)lua_touserdata(L, -1); + lua_pop(L, 1); + } lua_rawgeti(L, names_idx, f_idx + 1); - lua_insert(L, -2); /* name, sub_result */ - lua_rawset(L, result_idx); /* result[name] = sub_result */ + lua_pushvalue(L, -1); /* dup name */ + lua_rawget(L, result_idx); /* prev or nil */ + /* stack: ..., sub_result, name, prev_or_nil */ + if (lua_istable(L, -1) && lua_istable(L, -3) && + subplan_for_merge != NULL && + subplan_for_merge->override_decode_ref + == LUA_NOREF) { + int prev_idx = lua_gettop(L); + int sub_idx = prev_idx - 2; + merge_subresult_into(L, subplan_for_merge, + prev_idx, sub_idx); + lua_pop(L, 3); /* prev, name, sub_result */ + } else { + lua_pop(L, 1); /* pop prev/nil */ + /* stack: sub_result, name */ + lua_insert(L, -2); /* name, sub_result */ + lua_rawset(L, result_idx); + } } else { dec_push_one(c, f); /* stack: ..., names, sub_plans, [lists...], value */ @@ -2775,6 +3038,14 @@ lua_rawset(L, result_idx); } } } + + /* Unterminated SGROUP body (bd-rc8): when decoding a group, the + * loop must exit via the EGROUP break — falling out by end-of- + * buffer means the group was never closed. Mirrors wire.lua's + * skip_field SGROUP error path for the known-field-group case. */ + if (stop_group_id != 0 && !egroup_seen) + luaL_error(L, "unterminated SGROUP for field id %d", + (int)stop_group_id); /* Write captured unknown bytes as result._unknown_fields. Skipped * when nothing was captured (key stays absent — matches codec.lua). */ diff --git a/test/conformance_test.lua b/test/conformance_test.lua index d9d35963cfb42cb274ebdfd3bbab02ffcffc1c20..7976f769b76e17e972e55dd3c2d79a3b5c3a9906 100644 --- a/test/conformance_test.lua +++ b/test/conformance_test.lua @@ -817,6 +817,71 @@ t.assert_not(resp.parse_error, resp.parse_error) end -- ========================================================================= +-- bd-rc8: C decoder strict-decode parity with pure-Lua codec. The Lua +-- decoder already rejected illegal map-key/value UTF-8 and unmatched +-- proto2 SGROUPs (covered indirectly by the wire-type and tag-validation +-- tests above); the C decoder used to accept them silently, which was +-- caught only by the Google conformance harness. The tests below add the +-- coverage that was missing — map-key/value UTF-8 was tested for +-- singular/repeated/oneof shapes but not map, and proto2 group balancing +-- had no `make test` coverage at all. +-- ========================================================================= + +core_g.test_invalid_utf8_map_key_rejected = function() + -- map_string_string is field 69 → tag = (69<<3)|2 = 554 → + -- varint 0xaa 0x04. Inside the entry, key tag is (1<<3)|2 = 0x0a; + -- value tag is (2<<3)|2 = 0x12. The key payload 0xa0 0xb0 0xc0 0xd0 + -- starts with a lone continuation byte — invalid UTF-8. Bytes + -- borrowed from RejectInvalidUtf8.String.MapKey. + t.assert_not_equals( + pb_roundtrip('\xaa\x04\x0b\x0a\x04\xa0\xb0\xc0\xd0\x12\x03foo').parse_error, + nil) +end + +core_g.test_invalid_utf8_map_value_rejected = function() + -- Same outer wire shape; key is "foo", value bytes are invalid UTF-8. + -- Mirrors RejectInvalidUtf8.String.MapValue. + t.assert_not_equals( + pb_roundtrip('\xaa\x04\x0b\x0a\x03foo\x12\x04\xa0\xb0\xc0\xd0').parse_error, + nil) +end + +-- ========================================================================= +-- Proto2 group dispatch (bd-rc8). The proto3 fast path skips SGROUP +-- payloads via dec_skip_with_id, but a proto2 known-field group descends +-- into decode_body with a stop_group_id. Without an EGROUP-seen guard, +-- the loop fell off c->len and returned silently — the Lua codec errors, +-- so the C decoder must too. UnmatchedStartGroup uses the same byte +-- pattern: tag(201, SGROUP) = (201<<3)|3 = 1611 → varint 0xcb 0x0c. +-- ========================================================================= + +local PROTO2_NAME = 'protobuf_test_messages.proto2.TestAllTypesProto2' + +local function pb_roundtrip_proto2(input) + return decode_resp(core.handle_request(encode_req({ + protobuf_payload = input, + requested_output_format = PROTOBUF, + message_type = PROTO2_NAME, + }))) +end + +core_g.test_unmatched_start_group_rejected = function() + -- Field 201 SGROUP with no body and no matching EGROUP. + t.assert_not_equals( + pb_roundtrip_proto2('\xcb\x0c').parse_error, nil) +end + +core_g.test_unmatched_start_group_nested_rejected = function() + -- Outer SGROUP for field 201, nested SGROUP for field 202 + -- (tag (202<<3)|3 = 1619 = 0xd3 0x0c), then exactly one EGROUP for + -- field 201 (tag (201<<3)|4 = 1612 = 0xcc 0x0c). The inner group is + -- unterminated; SGROUP-recursive skip in the Lua decoder errors, and + -- the C decoder must match. Mirrors UnmatchedStartGroupNested. + t.assert_not_equals( + pb_roundtrip_proto2('\xcb\x0c\xd3\x0c\xcc\x0c').parse_error, nil) +end + +-- ========================================================================= -- Fix 9: JSON null on a field is "use default" (drop the field), except -- for google.protobuf.Value where null is itself a Value (NullValue). -- Compounded by Tarantool's box.NULL aliasing to nil under __eq, so the