Extra validation of cmdline arguments.
PiperOrigin-RevId: 431975072
diff --git a/rs_bindings_from_cc/cmdline.cc b/rs_bindings_from_cc/cmdline.cc
index 2a6fead..eaf9357 100644
--- a/rs_bindings_from_cc/cmdline.cc
+++ b/rs_bindings_from_cc/cmdline.cc
@@ -80,20 +80,56 @@
return absl::InvalidArgumentError("please specify --targets_and_headers");
}
nlohmann::json targets_and_headers =
- nlohmann::json::parse(std::move(targets_and_headers_str));
+ nlohmann::json::parse(std::move(targets_and_headers_str),
+ /* cb= */ nullptr,
+ /* allow_exceptions= */ false);
if (!targets_and_headers.is_array()) {
return absl::InvalidArgumentError(
"Expected `--targets_and_headers` to be a JSON array of objects");
}
for (const auto& target_and_headers : targets_and_headers) {
+ if (!target_and_headers.contains("t")) {
+ return absl::InvalidArgumentError(
+ "Missing `t` field in an `--targets_and_headers` object");
+ }
+ if (!target_and_headers["t"].is_string()) {
+ return absl::InvalidArgumentError(
+ "Expected `t` fields of `--targets_and_headers` to be a string");
+ }
+ if (!target_and_headers.contains("h")) {
+ return absl::InvalidArgumentError(
+ "Missing `h` field in an `--targets_and_headers` object");
+ }
if (!target_and_headers["h"].is_array()) {
return absl::InvalidArgumentError(
"Expected `h` fields of `--targets_and_headers` to be an array");
}
- BlazeLabel target = BlazeLabel{target_and_headers["t"]};
+ BlazeLabel target{std::string(target_and_headers["t"])};
+ if (target.value().empty()) {
+ return absl::InvalidArgumentError(
+ "Expected `t` fields of `--targets_and_headers` to be a non-empty "
+ "string");
+ }
for (const auto& header : target_and_headers["h"]) {
- cmdline.headers_to_targets_.insert(
- std::make_pair(HeaderName(std::string(header)), target));
+ if (!header.is_string()) {
+ return absl::InvalidArgumentError(
+ "Expected `h` fields of `--targets_and_headers` to be an array of "
+ "strings");
+ }
+ std::string header_str(header);
+ if (header_str.empty()) {
+ return absl::InvalidArgumentError(
+ "Expected `h` fields of `--targets_and_headers` to be an array of "
+ "non-empty strings");
+ }
+ const auto [it, inserted] = cmdline.headers_to_targets_.insert(
+ std::make_pair(HeaderName(header_str), target));
+ if (!inserted) {
+ return absl::InvalidArgumentError(absl::Substitute(
+ "The `--targets_and_headers` cmdline argument assigns "
+ "`$0` header to two conflicting targets: `$1` vs `$2`",
+ header_str, target.value(), it->second.value()));
+ }
}
}
diff --git a/rs_bindings_from_cc/cmdline_test.cc b/rs_bindings_from_cc/cmdline_test.cc
index e8aa0e4..146d816 100644
--- a/rs_bindings_from_cc/cmdline_test.cc
+++ b/rs_bindings_from_cc/cmdline_test.cc
@@ -56,6 +56,13 @@
HasSubstr("please specify --targets_and_headers")));
}
+TEST(CmdlineTest, TargetsAndHeadersInvalidJson) {
+ ASSERT_THAT(
+ TestCmdline({"h1"}, "#!$%"),
+ StatusIs(absl::StatusCode::kInvalidArgument,
+ AllOf(HasSubstr("--targets_and_headers"), HasSubstr("array"))));
+}
+
TEST(CmdlineTest, TargetsAndHeadersIntInsteadOfTopLevelArray) {
ASSERT_THAT(
TestCmdline({"h1"}, "123"),
@@ -63,6 +70,12 @@
AllOf(HasSubstr("--targets_and_headers"), HasSubstr("array"))));
}
+TEST(CmdlineTest, TargetsAndHeadersIntInTopLevelArray) {
+ ASSERT_THAT(TestCmdline({"h1"}, "[123, 456]"),
+ StatusIs(absl::StatusCode::kInvalidArgument,
+ AllOf(HasSubstr("--targets_and_headers"))));
+}
+
TEST(CmdlineTest, TargetsAndHeadersIntInsteadOfHeadersArray) {
ASSERT_THAT(TestCmdline({"h1"}, R"([{"t": "t1", "h": 123}])"),
StatusIs(absl::StatusCode::kInvalidArgument,
@@ -70,6 +83,59 @@
HasSubstr("`h`"), HasSubstr("array"))));
}
+TEST(CmdlineTest, TargetsAndHeadersMissingTarget) {
+ ASSERT_THAT(TestCmdline({"h1"}, R"([{"h": ["h1", "h2"]}])"),
+ StatusIs(absl::StatusCode::kInvalidArgument,
+ AllOf(HasSubstr("--targets_and_headers"),
+ HasSubstr("`t`"), HasSubstr("Missing"))));
+}
+
+TEST(CmdlineTest, TargetsAndHeadersMissingHeader) {
+ ASSERT_THAT(TestCmdline({"h1"}, R"([{"t": "t1"}])"),
+ StatusIs(absl::StatusCode::kInvalidArgument,
+ AllOf(HasSubstr("--targets_and_headers"),
+ HasSubstr("`h`"), HasSubstr("Missing"))));
+}
+
+TEST(CmdlineTest, TargetsAndHeadersEmptyHeader) {
+ ASSERT_THAT(TestCmdline({"h1"}, R"([{"t": "t1", "h": ["", "h2"]}])"),
+ StatusIs(absl::StatusCode::kInvalidArgument,
+ AllOf(HasSubstr("--targets_and_headers"),
+ HasSubstr("`h`"), HasSubstr("empty string"))));
+}
+
+TEST(CmdlineTest, TargetsAndHeadersEmptyTarget) {
+ ASSERT_THAT(TestCmdline({"h1"}, R"([{"t": "", "h": ["h1", "h2"]}])"),
+ StatusIs(absl::StatusCode::kInvalidArgument,
+ AllOf(HasSubstr("--targets_and_headers"),
+ HasSubstr("`t`"), HasSubstr("empty string"))));
+}
+
+TEST(CmdlineTest, TargetsAndHeadersIntInsteadOfTarget) {
+ ASSERT_THAT(TestCmdline({"h1"}, R"([{"t": 123, "h": ["h1", "h2"]}])"),
+ StatusIs(absl::StatusCode::kInvalidArgument,
+ AllOf(HasSubstr("--targets_and_headers"),
+ HasSubstr("`t`"), HasSubstr("string"))));
+}
+
+TEST(CmdlineTest, TargetsAndHeadersIntInsteadOfHeader) {
+ ASSERT_THAT(TestCmdline({"h1"}, R"([{"t": "t1", "h": [123, "h2"]}])"),
+ StatusIs(absl::StatusCode::kInvalidArgument,
+ AllOf(HasSubstr("--targets_and_headers"),
+ HasSubstr("`h`"), HasSubstr("string"))));
+}
+
+TEST(CmdlineTest, TargetsAndHeadersDuplicateHeader) {
+ constexpr char kTestInput[] = R"([
+ {"t": "t1", "h": ["h1"]},
+ {"t": "t2", "h": ["h1"]} ])";
+ ASSERT_THAT(
+ TestCmdline({"h1"}, kTestInput),
+ StatusIs(absl::StatusCode::kInvalidArgument,
+ AllOf(HasSubstr("--targets_and_headers"), HasSubstr("conflict"),
+ HasSubstr("h1"), HasSubstr("t1"), HasSubstr("t2"))));
+}
+
TEST(CmdlineTest, PublicHeadersEmpty) {
constexpr char kTargetsAndHeaders[] = R"([
{"t": "target1", "h": ["a.h", "b.h"]}
