Skip to content

Commit 5a56dee

Browse files
Bug fix for edition 2024 visibility checking. Visibility checking was not properly applied to service method input and output types. This is now applied properly and will error if method input/output types do not have visibility to those messages.
NOTE: this is a potentially breaking change. method input or output types outside of the same file as an RPC service, which have been set to 'local' either explicitly or by default will need to be marked `export` to avoid proto compile errors. This should be rare as visibility is not widely adopted. PiperOrigin-RevId: 888762538
1 parent 967e8d5 commit 5a56dee

2 files changed

Lines changed: 58 additions & 0 deletions

File tree

src/google/protobuf/descriptor.cc

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8280,6 +8280,11 @@ void DescriptorBuilder::CrossLinkMethod(MethodDescriptor* method,
82808280
return absl::StrCat("\"", proto.input_type(),
82818281
"\" is not a message type.");
82828282
});
8283+
} else if (!input_type.IsVisibleFrom(file_) &&
8284+
pool_->enforce_symbol_visibility_) {
8285+
AddError(method->full_name(), proto,
8286+
DescriptorPool::ErrorCollector::INPUT_TYPE,
8287+
[&] { return input_type.GetVisibilityError(file_); });
82838288
} else {
82848289
method->input_type_.Set(input_type.descriptor());
82858290
}
@@ -8302,6 +8307,11 @@ void DescriptorBuilder::CrossLinkMethod(MethodDescriptor* method,
83028307
return absl::StrCat("\"", proto.output_type(),
83038308
"\" is not a message type.");
83048309
});
8310+
} else if (!output_type.IsVisibleFrom(file_) &&
8311+
pool_->enforce_symbol_visibility_) {
8312+
AddError(method->full_name(), proto,
8313+
DescriptorPool::ErrorCollector::OUTPUT_TYPE,
8314+
[&] { return output_type.GetVisibilityError(file_); });
83058315
} else {
83068316
method->output_type_.Set(output_type.descriptor());
83078317
}

src/google/protobuf/descriptor_unittest.cc

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13905,6 +13905,54 @@ TEST_F(ValidationErrorTest, VisibilityFromLocalExtender) {
1390513905
"accessed outside its own file\n");
1390613906
}
1390713907

13908+
TEST_F(ValidationErrorTest, VisibilityFromService) {
13909+
pool_.EnforceSymbolVisibility(true);
13910+
ASSERT_THAT(ParseAndBuildFile("vis.proto", R"schema(
13911+
edition = "2024";
13912+
package vis.test;
13913+
13914+
local message LocalMessage {
13915+
}
13916+
export message ExportMessage {
13917+
}
13918+
)schema"),
13919+
NotNull());
13920+
13921+
ParseAndBuildFileWithErrorSubstr(
13922+
"service_bad_input.proto",
13923+
R"schema(
13924+
edition = "2024";
13925+
import "vis.proto";
13926+
13927+
service MyServiceInput {
13928+
rpc MyBadMethod(vis.test.LocalMessage)
13929+
returns (vis.test.ExportMessage) {}
13930+
}
13931+
)schema",
13932+
"service_bad_input.proto: MyServiceInput.MyBadMethod: INPUT_TYPE: Symbol "
13933+
"\"vis.test.LocalMessage\", "
13934+
"defined in \"vis.proto\" is not visible "
13935+
"from \"service_bad_input.proto\". It is explicitly marked 'local' and "
13936+
"cannot be accessed outside its own file\n");
13937+
13938+
ParseAndBuildFileWithErrorSubstr(
13939+
"service_bad_return.proto",
13940+
R"schema(
13941+
edition = "2024";
13942+
import "vis.proto";
13943+
13944+
service MyServiceReturn {
13945+
rpc MyBadMethod(vis.test.ExportMessage)
13946+
returns (vis.test.LocalMessage) {}
13947+
}
13948+
)schema",
13949+
"service_bad_return.proto: MyServiceReturn.MyBadMethod: OUTPUT_TYPE: "
13950+
"Symbol \"vis.test.LocalMessage\", defined in \"vis.proto\" is not "
13951+
"visible from \"service_bad_return.proto\". It is "
13952+
"explicitly marked 'local' and cannot be accessed outside its own "
13953+
"file\n");
13954+
}
13955+
1390813956
struct ExtensionDeclarationsTestParams {
1390913957
std::string test_name;
1391013958
};

0 commit comments

Comments
 (0)