DescriptorPool resolves type names across the whole set, ignoring each file's import graph

[feiyuceng06-prog]

DescriptorPool resolves every type name against the whole set, so it links files that reference types they never import.

What

A FileDescriptorProto may only reference types defined in itself or in a file it lists in dependency (transitively through public_dependency). DescriptorPool does not enforce that. Pass 1 registers every message and enum in the set into one flat by_name table, and resolve_type_name/resolve_message_type_name search that table, so a reference resolves against any file in the set regardless of what the referring file imports.

dependency, public_dependency and weak_dependency do not appear anywhere in buffa-descriptor/src/ outside generated/.

Repro

b.proto references .a.Thing and declares no dependency:

let a = FileDescriptorProto {
    name: Some("a.proto".into()),
    package: Some("a".into()),
    message_type: vec![DescriptorProto { name: Some("Thing".into()), ..Default::default() }],
    ..Default::default()
};
let b = FileDescriptorProto {
    name: Some("b.proto".into()),
    package: Some("b".into()),
    // dependency intentionally empty
    message_type: vec![DescriptorProto {
        name: Some("Holder".into()),
        field: vec![FieldDescriptorProto {
            name: Some("thing".into()),
            number: Some(1),
            label: Some(Label::LABEL_OPTIONAL),
            r#type: Some(Type::TYPE_MESSAGE),
            type_name: Some(".a.Thing".into()),
            ..Default::default()
        }],
        ..Default::default()
    }],
    ..Default::default()
};
assert!(DescriptorPool::new(FileDescriptorSet { file: vec![a, b], ..Default::default() }).is_ok());

That assertion passes on main (fff33f9-era checkout, 0.9.1).

protoc 36.0 rejects the same set, read back through --descriptor_set_in:

$ protoc --descriptor_set_in=no_import.fds -o /dev/null b.proto
b.proto: "a.Thing" seems to be defined in "a.proto", which is not imported by "b.proto".  To use it here, please add the necessary import.
$ echo $?
1

Why it matters

The pool's own docs present it as validating structure — new() lists dangling type names among the failures it catches — and every other check in add_file_descriptor_set_staged is one protoc also makes. Import scope is the one that is silently dropped, which makes the accepted-input surface wider than protoc's in a way nothing in the docs says.

Two concrete consequences:

• A set assembled without --include_imports, or one whose files were merged from several independently-compiled sets, links cleanly even when a file's imports are not actually present. The reference happens to resolve because some other file in the set declares that name. Add the same file to a pool that does not happen to carry the neighbour and it fails, so the same descriptor set is valid or not depending on what it is loaded alongside.
• Reflection consumers (gRPC server reflection is named as a target in the module docs) hand out a file whose dependency list does not describe what it actually needs. A client that fetches the transitive closure by following dependency gets an incomplete set.

Scope

I do not think this is a small fix, and I am not proposing one. Enforcing it means resolution has to become per-file — a visible-symbol set built from the file's own declarations plus the transitive closure of its dependency list through public_dependency — which changes the shape of the resolve path and would reject descriptor sets that link today. That is a real behaviour change and a maintainer call, not something to slip in.

What seems worth deciding first is whether the current behaviour is intended. Nothing in DESIGN.md, docs/, or the pool.rs module docs mentions import scope either way, so today a reader cannot tell whether global resolution is a deliberate relaxation or an oversight. Documenting it would close the gap between what the pool promises and what it checks even if the resolution model stays as it is.

Related

The dependency indices themselves are a separate, self-contained gap: public_dependency and weak_dependency are positions in dependency, and out-of-range ones are accepted too (protoc: "Invalid public dependency index."). That one is small, so it is #422 rather than part of this issue.

How this was found

Differential testing of DescriptorPool::new against protoc --descriptor_set_in on hand-built FileDescriptorSets, while looking for validation gaps in the same area as the recent descriptor: reject ... series.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MP4xv7WtAw6hteCNMu7cfn