Skip to content

Commit 1e43394

Browse files
authored
fix(xlang): close field tag validation gaps (#3984)
## Why? PR #3982 established the signed-int32 field-tag protocol range. Follow-up integration checks found a small set of validation ownership, exact-schema coverage, and documentation gaps that were not part of the squash-merged boundary. This PR completes those paths without changing the field-tag wire format introduced by #3982. ## What does this PR do? - Rejects duplicate Java native TypeDef tags across the complete inheritance hierarchy. - Validates Go field tags before public registration mutates resolver state while continuing to ignore unexported fields. - Keeps Rust protocol metadata equality independent from local compatible-reader dispatch state. - Adds causal C++ remote-compatible and oversized-tag decoder coverage. - Adds an exact-schema cross-language fixture for tags `15`, `65551`, and `536870911` across all supported peers, including normal concrete Java test discovery. - Runs Kotlin's maximum tag through a real KSP-generated serializer, descriptor, and round trip. - Documents the exact field-tag range, uniqueness, stability, and extended encoding rules in the protocol, compiler, and runtime schema guides. ## Related issues Follow-up to #3982. ## Does this PR introduce any user-facing change? - [ ] Does this PR introduce any public API change? - [ ] Does this PR introduce any binary protocol compatibility change? Invalid duplicate or out-of-range field-tag schemas are now rejected consistently at their owning registration or metadata boundary. Existing valid schemas and wire bytes are unchanged. ## Test - Java native TypeDef, Go registration, C++ metadata, Rust metadata equality, and Kotlin KSP/static serializer tests passed. - Exact-schema and compatible Java-driven field-tag cases passed against C++, C#, Dart, Go, JavaScript, Python, Rust, Scala, and Swift in both Java codegen modes. - Dart IDL regeneration, compilation, and Java/Dart semantic round trips passed. - Java Spotless, Rust fmt/clippy, Go formatting, C++ formatting, Markdown Prettier, and full diff checks passed. ## Benchmark The production changes in this follow-up are Java remote native-TypeDef miss validation, Go registration-time validation, and Rust `FieldInfo` equality. None is reached by the steady-state serialize/deserialize benchmark loops from #3982, so those paired results remain the causal hot-path evidence. Every measured runtime/path stayed below the `+1.0%` regression ceiling; the largest positive paired medians were C++ deserialize at `+0.858%` and Java static serialize at `+0.779%`.
1 parent 3bfa17e commit 1e43394

47 files changed

Lines changed: 560 additions & 50 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/ci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -559,7 +559,7 @@ jobs:
559559
~/.swiftpm
560560
~/Library/Caches/org.swift.swiftpm
561561
swift/.build
562-
key: ${{ runner.os }}-${{ runner.arch }}-swiftpm-xlang-release-${{ steps.swift-package-cache.outputs.toolchain }}-${{ hashFiles('swift/Package.resolved', 'swift/Package.swift', 'swift/Sources/**') }}
562+
key: ${{ runner.os }}-${{ runner.arch }}-swiftpm-xlang-release-${{ steps.swift-package-cache.outputs.toolchain }}-${{ hashFiles('swift/Package.resolved', 'swift/Package.swift', 'swift/Sources/**', 'swift/Tests/ForyXlangTests/**') }}
563563
restore-keys: |
564564
${{ runner.os }}-${{ runner.arch }}-swiftpm-xlang-release-${{ steps.swift-package-cache.outputs.toolchain }}-
565565
- name: Prebuild Swift xlang peer for cache reuse

cpp/fory/serialization/struct_test.cc

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,20 @@ struct MaximumFieldTagStruct {
146146
FORY_STRUCT(MaximumFieldTagStruct, (value, fory::F(536870911)));
147147
};
148148

149+
struct HighTagCompatibleWriter {
150+
std::string removed;
151+
int32_t shared;
152+
153+
FORY_STRUCT(HighTagCompatibleWriter, (removed, fory::F(65551)),
154+
(shared, fory::F(536870911)));
155+
};
156+
157+
struct HighTagCompatibleReader {
158+
int32_t shared = 0;
159+
160+
FORY_STRUCT(HighTagCompatibleReader, (shared, fory::F(536870911)));
161+
};
162+
149163
struct SignedToUnsignedWriter {
150164
int32_t value;
151165

@@ -1403,6 +1417,12 @@ TEST(StructComprehensiveTest, FieldTagRange) {
14031417
invalid.field_id = 536870912;
14041418
EXPECT_FALSE(invalid.to_bytes().ok());
14051419

1420+
Buffer invalid_wire;
1421+
invalid_wire.write_uint8(static_cast<uint8_t>((3u << 6) | (15u << 2)));
1422+
invalid_wire.write_var_uint32(536870912u - 15u);
1423+
invalid_wire.write_uint8(static_cast<uint8_t>(TypeId::INT32));
1424+
EXPECT_FALSE(FieldInfo::from_bytes(invalid_wire).ok());
1425+
14061426
TypeMeta duplicate;
14071427
duplicate.type_id = static_cast<uint32_t>(TypeId::COMPATIBLE_STRUCT);
14081428
duplicate.user_type_id = 632;
@@ -1412,6 +1432,21 @@ TEST(StructComprehensiveTest, FieldTagRange) {
14121432
EXPECT_FALSE(duplicate.to_bytes().ok());
14131433
}
14141434

1435+
TEST(StructComprehensiveTest, CompatibleHighTagsRead) {
1436+
auto writer =
1437+
Fory::builder().xlang(true).compatible(true).track_ref(false).build();
1438+
auto reader =
1439+
Fory::builder().xlang(true).compatible(true).track_ref(false).build();
1440+
ASSERT_TRUE(writer.register_struct<HighTagCompatibleWriter>(633).ok());
1441+
ASSERT_TRUE(reader.register_struct<HighTagCompatibleReader>(633).ok());
1442+
1443+
auto encoded = writer.serialize(HighTagCompatibleWriter{"skip", 42});
1444+
ASSERT_TRUE(encoded.ok()) << encoded.error().to_string();
1445+
auto decoded = reader.deserialize<HighTagCompatibleReader>(*encoded);
1446+
ASSERT_TRUE(decoded.ok()) << decoded.error().to_string();
1447+
EXPECT_EQ(decoded->shared, 42);
1448+
}
1449+
14151450
TEST(StructComprehensiveTest, NonPrimitiveFieldsSortByFieldIdentifier) {
14161451
auto fields = TypeMeta::sort_field_infos({
14171452
make_test_field_info("string_value", 20,

cpp/fory/serialization/xlang_test_main.cc

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,17 @@ struct SimpleStruct {
137137
(f7, fory::F(65551)), f8, (last, fory::F(536870911)));
138138
};
139139

140+
struct ExactFieldTags {
141+
int32_t first;
142+
int32_t second;
143+
int32_t last;
144+
bool operator==(const ExactFieldTags &other) const {
145+
return first == other.first && second == other.second && last == other.last;
146+
}
147+
FORY_STRUCT(ExactFieldTags, (first, fory::F(15)), (second, fory::F(65551)),
148+
(last, fory::F(536870911)));
149+
};
150+
140151
struct EvolvingOverrideStruct {
141152
std::string f1;
142153
bool operator==(const EvolvingOverrideStruct &other) const {
@@ -938,6 +949,7 @@ namespace {
938949
void run_test_buffer(const std::string &data_file);
939950
void run_test_buffer_var(const std::string &data_file);
940951
void run_test_murmur_hash3(const std::string &data_file);
952+
void run_test_exact_field_tags(const std::string &data_file);
941953
void run_test_string_serializer(const std::string &data_file);
942954
void run_test_cross_language_serializer(const std::string &data_file);
943955
void run_test_simple_struct(const std::string &data_file);
@@ -1030,6 +1042,8 @@ int main(int argc, char **argv) {
10301042
run_test_cross_language_serializer(data_file);
10311043
} else if (case_name == "test_simple_struct") {
10321044
run_test_simple_struct(data_file);
1045+
} else if (case_name == "test_exact_field_tags") {
1046+
run_test_exact_field_tags(data_file);
10331047
} else if (case_name == "test_named_simple_struct") {
10341048
run_test_simple_named_struct(data_file);
10351049
} else if (case_name == "test_struct_evolving_override") {
@@ -1570,6 +1584,21 @@ void run_test_simple_struct(const std::string &data_file) {
15701584
write_file(data_file, out);
15711585
}
15721586

1587+
void run_test_exact_field_tags(const std::string &data_file) {
1588+
auto fory = build_fory(false, true);
1589+
ensure_ok(fory.register_struct<ExactFieldTags>(104),
1590+
"register ExactFieldTags");
1591+
auto bytes = read_file(data_file);
1592+
Buffer buffer = make_buffer(bytes);
1593+
auto value = read_next<ExactFieldTags>(fory, buffer);
1594+
if (!(value == ExactFieldTags{39, 40, 41})) {
1595+
fail("ExactFieldTags mismatch");
1596+
}
1597+
std::vector<uint8_t> out;
1598+
append_serialized(fory, value, out);
1599+
write_file(data_file, out);
1600+
}
1601+
15731602
void run_test_simple_named_struct(const std::string &data_file) {
15741603
auto bytes = read_file(data_file);
15751604
auto fory = build_fory(true, true);

csharp/tests/Fory.XlangPeer/Program.cs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,7 @@ private static byte[] ExecuteCase(string caseName, byte[] input)
194194
"test_string_serializer" => CaseStringSerializer(input),
195195
"test_cross_language_serializer" => CaseCrossLanguageSerializer(input),
196196
"test_simple_struct" => CaseSimpleStruct(input),
197+
"test_exact_field_tags" => CaseExactFieldTags(input),
197198
"test_named_simple_struct" => CaseNamedSimpleStruct(input),
198199
"test_struct_evolving_override" => CaseStructEvolvingOverride(input),
199200
"test_list" => CaseList(input),
@@ -461,6 +462,13 @@ private static byte[] CaseSimpleStruct(byte[] input)
461462
return RoundTripSingle<SimpleStruct>(input, fory);
462463
}
463464

465+
private static byte[] CaseExactFieldTags(byte[] input)
466+
{
467+
ForyRuntime fory = BuildFory(compatible: false);
468+
fory.Register<ExactFieldTags>(104);
469+
return RoundTripSingle<ExactFieldTags>(input, fory);
470+
}
471+
464472
private static byte[] CaseNamedSimpleStruct(byte[] input)
465473
{
466474
ForyRuntime fory = BuildFory(compatible: true);
@@ -1282,6 +1290,17 @@ public sealed class SimpleStruct
12821290
public int Last { get; set; }
12831291
}
12841292

1293+
[ForyStruct]
1294+
public sealed class ExactFieldTags
1295+
{
1296+
[ForyField(15)]
1297+
public int First { get; set; }
1298+
[ForyField(65551)]
1299+
public int Second { get; set; }
1300+
[ForyField(536870911)]
1301+
public int Last { get; set; }
1302+
}
1303+
12851304
[ForyStruct]
12861305
public sealed class EvolvingOverrideStruct
12871306
{

dart/packages/fory-test/lib/entity/xlang_test_models.dart

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,20 @@ class SimpleStruct {
7979
int last = 0;
8080
}
8181

82+
@ForyStruct()
83+
class ExactFieldTags {
84+
ExactFieldTags();
85+
86+
@ForyField(id: 15, type: Int32Type())
87+
int first = 0;
88+
89+
@ForyField(id: 65551, type: Int32Type())
90+
int second = 0;
91+
92+
@ForyField(id: 536870911, type: Int32Type())
93+
int last = 0;
94+
}
95+
8296
@ForyStruct()
8397
class EvolvingOverrideStruct {
8498
EvolvingOverrideStruct();

dart/packages/fory-test/test/cross_lang_test/xlang_test_main.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -491,6 +491,11 @@ void _runCase(String caseName) {
491491
_registerSimpleById(fory);
492492
_roundTripFory(fory);
493493
return;
494+
case 'test_exact_field_tags':
495+
final fory = _newFory();
496+
registerXlangType(fory, ExactFieldTags, id: 104);
497+
_roundTripFory(fory);
498+
return;
494499
case 'test_named_simple_struct':
495500
final fory = _newFory(compatible: true);
496501
_registerSimpleByName(fory);

docs/compiler/schema-idl.md

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -976,6 +976,11 @@ Fields define the properties of a message.
976976
field_type field_name = field_number;
977977
```
978978

979+
`field_number` is the field tag ID. It must be unique within the message and
980+
satisfy `0 <= field_number < 2^29` (`0` through `536870911`). Keep
981+
assigned field numbers stable and reserve removed numbers instead of reusing
982+
them for different fields.
983+
979984
### With Modifiers
980985

981986
```protobuf
@@ -1149,22 +1154,22 @@ Use `ref(thread_safe=false)` in Fory IDL (or
11491154

11501155
## Field Numbers
11511156

1152-
Each field must have a unique positive integer identifier:
1157+
Each field must have a unique tag ID in the protocol range:
11531158

11541159
```protobuf
11551160
message Example {
1156-
string first = 1;
1157-
string second = 2;
1158-
string third = 3;
1161+
string first = 0;
1162+
string second = 1;
1163+
string third = 2;
11591164
}
11601165
```
11611166

11621167
**Rules and best practices:**
11631168

11641169
- Numbers must be unique within a message.
1165-
- Numbers must be positive integers.
1170+
- Numbers must satisfy `0 <= field_number < 2^29` (`0` through `536870911`).
11661171
- Gaps are allowed and are useful when fields are removed.
1167-
- Prefer sequential numbering from `1`.
1172+
- Prefer sequential numbering.
11681173
- Never reuse a removed field number for a different field.
11691174

11701175
## Type System

docs/object-serialization/cpp/schema-metadata.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,9 @@ FORY_STRUCT(DataV2, id, (timestamp, fory::F().tagged()), version);
5959
FORY_STRUCT(Counter, FORY_PROPERTY(value, fory::F().varint()));
6060
```
6161

62-
`fory::F(id)` uses explicit id-based field identity. IDs must be
63-
non-negative:
62+
`fory::F(id)` uses explicit id-based field identity. Configured IDs must be
63+
unique within the complete struct schema and satisfy `0 <= id < 2^29` (`0`
64+
through `536870911`):
6465

6566
```cpp
6667
FORY_STRUCT(DataV2, (id, fory::F(0)), (timestamp, fory::F(1).tagged()),

docs/object-serialization/csharp/schema-metadata.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ This page covers schema metadata for C# generated serializers.
2323

2424
## `[ForyStruct]` and `[ForyField]`
2525

26-
Use `[ForyStruct]` to enable source-generated serializers. Use `[ForyField]` to assign an optional stable non-negative field id or to override the Fory schema type used for a field.
26+
Use `[ForyStruct]` to enable source-generated serializers. Use `[ForyField]` to assign an optional stable field ID or to override the Fory schema type used for a field. Configured IDs must be unique within the complete struct schema and satisfy `0 <= id < 2^29` (`0` through `536870911`).
2727

2828
External-type serialization puts `Target` on a local abstract serializer
2929
declaration. Its properties own the field names, IDs, schema descriptors,
@@ -77,6 +77,7 @@ public sealed class Metrics
7777
```
7878

7979
`Id` is optional. When it is omitted, compatible mode still matches the field by name.
80+
Once assigned, keep an ID stable and do not reuse it for a different field.
8081

8182
```csharp
8283
using Apache.Fory;

docs/object-serialization/dart/schema-metadata.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ String name = '';
6969
```
7070

7171
Once a payload is shared across services, never reuse an `id` for a different field.
72+
Configured IDs must satisfy `0 <= id < 2^29` (`0` through `536870911`).
7273

7374
An ordinary child has one flattened field namespace. IDs must therefore be
7475
unique across all fields included from its child, superclass, and

0 commit comments

Comments
 (0)