Raise an error on invalid members in json

Summary: This will help us catch typos when writing model generators or rules. Reviewed By: anwesht Differential Revision: D46559562 fbshipit-source-id: a992690b03ebe580192fc6f515c33390e9ad725e
facebook · Jun 8, 2023 · fe18ac4 · fe18ac4
1 parent c37d93e
commit fe18ac4
Show file tree

Hide file tree

Showing 26 changed files with 328 additions and 74 deletions.
diff --git a/source/FeatureMayAlwaysSet.cpp b/source/FeatureMayAlwaysSet.cpp
@@ -75,7 +75,8 @@ void FeatureMayAlwaysSet::add(const FeatureMayAlwaysSet& other) {
 
 FeatureMayAlwaysSet FeatureMayAlwaysSet::from_json(
     const Json::Value& value,
-    Context& context) {
+    Context& context,
+    bool check_unexpected_members) {
   auto may_features = FeatureSet{};
   auto always_features = FeatureSet{};
 
@@ -84,6 +85,11 @@ FeatureMayAlwaysSet FeatureMayAlwaysSet::from_json(
   bool is_bottom = true;
 
   JsonValidation::validate_object(value);
+  if (check_unexpected_members) {
+    JsonValidation::check_unexpected_members(
+        value, {"may_features", "always_features"});
+  }
+
   if (value.isMember("may_features")) {
     JsonValidation::null_or_array(value, /* field */ "may_features");
     may_features.join_with(

diff --git a/source/FeatureMayAlwaysSet.h b/source/FeatureMayAlwaysSet.h
@@ -84,7 +84,8 @@ class FeatureMayAlwaysSet final
 
   static FeatureMayAlwaysSet from_json(
       const Json::Value& value,
-      Context& context);
+      Context& context,
+      bool check_unexpected_members = true);
   Json::Value to_json() const;
 
   friend std::ostream& operator<<(

diff --git a/source/FieldModel.cpp b/source/FieldModel.cpp
@@ -124,6 +124,9 @@ FieldModel FieldModel::from_json(
     const Json::Value& value,
     Context& context) {
   JsonValidation::validate_object(value);
+  JsonValidation::check_unexpected_members(
+      value, {"field", "sources", "sinks"});
+
   FieldModel model(field);
 
   for (auto source_value :

diff --git a/source/JsonValidation.cpp b/source/JsonValidation.cpp
@@ -9,6 +9,7 @@
 
 #include <boost/algorithm/string/trim.hpp>
 #include <boost/filesystem/fstream.hpp>
+#include <boost/range/adaptor/transformed.hpp>
 #include <fmt/format.h>
 
 #include <mariana-trench/Assert.h>
@@ -334,4 +335,29 @@ void JsonValidation::update_object(
   }
 }
 
+void JsonValidation::check_unexpected_members(
+    const Json::Value& value,
+    const std::unordered_set<std::string>& valid_members) {
+  validate_object(value);
+
+  for (const std::string& member : value.getMemberNames()) {
+    if (valid_members.find(member) == valid_members.end()) {
+      throw JsonValidationError(
+          value,
+          /* field */ std::nullopt,
+          /* expected */
+          fmt::format(
+              "fields {}, got `{}`",
+              fmt::join(
+                  boost::adaptors::transform(
+                      valid_members,
+                      [](const std::string& member) {
+                        return fmt::format("`{}`", member);
+                      }),
+                  ", "),
+              member));
+    }
+  }
+}
+
 } // namespace marianatrench
diff --git a/source/JsonValidation.h b/source/JsonValidation.h
@@ -9,6 +9,7 @@
 
 #include <optional>
 #include <string>
+#include <unordered_set>
 
 #include <boost/filesystem.hpp>
 #include <json/json.h>
@@ -87,6 +88,11 @@ class JsonValidation final {
    * object, in place.
    */
   static void update_object(Json::Value& left, const Json::Value& right);
+
+  /* Error on invalid members of a json object. */
+  static void check_unexpected_members(
+      const Json::Value& value,
+      const std::unordered_set<std::string>& valid_members);
 };
 
 } // namespace marianatrench
diff --git a/source/Kind.cpp b/source/Kind.cpp
@@ -22,7 +22,13 @@ std::ostream& operator<<(std::ostream& out, const Kind& kind) {
   return out;
 }
 
-const Kind* Kind::from_json(const Json::Value& value, Context& context) {
+const Kind* Kind::from_json(
+    const Json::Value& value,
+    Context& context,
+    bool check_unexpected_members) {
+  if (check_unexpected_members) {
+    JsonValidation::check_unexpected_members(value, {"kind", "partial_label"});
+  }
   const auto leaf_kind = JsonValidation::string(value, /* field */ "kind");
   if (value.isMember("partial_label")) {
     return context.kind_factory->get_partial(

diff --git a/source/Kind.h b/source/Kind.h
@@ -60,7 +60,10 @@ class Kind {
    * If it is known beforehand (as in Rules) whether the Kind is a Named or
    * Partial kind then use the override of this method from the specific Kind.
    */
-  static const Kind* from_json(const Json::Value& value, Context& context);
+  static const Kind* from_json(
+      const Json::Value& value,
+      Context& context,
+      bool check_unexpected_members = true);
 
   /**
    * String value used for connecting traces of the same kind.

diff --git a/source/Method.cpp b/source/Method.cpp
@@ -157,6 +157,9 @@ const Method* Method::from_json(const Json::Value& value, Context& context) {
         value, /* field */ std::nullopt, /* expected */ "object or string");
   }
 
+  JsonValidation::check_unexpected_members(
+      value, {"name", "parameter_type_overrides"});
+
   auto method_name = JsonValidation::string(value, "name");
 
   auto* dex_method = redex::get_method(method_name);
@@ -168,6 +171,8 @@ const Method* Method::from_json(const Json::Value& value, Context& context) {
   ParameterTypeOverrides parameter_type_overrides;
   for (auto parameter_type_override :
        JsonValidation::null_or_array(value, "parameter_type_overrides")) {
+    JsonValidation::check_unexpected_members(
+        parameter_type_override, {"parameter", "type"});
     auto parameter =
         JsonValidation::integer(parameter_type_override, "parameter");
     auto* type = JsonValidation::dex_type(parameter_type_override, "type");

diff --git a/source/Model.cpp b/source/Model.cpp
@@ -959,8 +959,30 @@ void Model::join_with(const Model& other) {
 Model Model::from_json(
     const Method* method,
     const Json::Value& value,
-    Context& context) {
+    Context& context,
+    bool check_unexpected_members) {
   JsonValidation::validate_object(value);
+  if (check_unexpected_members) {
+    JsonValidation::check_unexpected_members(
+        value,
+        {"method", // Only when called from `Registry`.
+         "modes",
+         "freeze",
+         "generations",
+         "parameter_sources",
+         "sources",
+         "sinks",
+         "effect_sources",
+         "effect_sinks",
+         "propagation",
+         "sanitizers",
+         "attach_to_sources",
+         "attach_to_sinks",
+         "attach_to_propagations",
+         "add_features_to_arguments",
+         "inline_as",
+         "issues"});
+  }
 
   Modes modes;
   for (auto mode_value :
@@ -1075,6 +1097,8 @@ Model Model::from_json(
 
   for (auto attach_to_sources_value :
        JsonValidation::null_or_array(value, /* field */ "attach_to_sources")) {
+    JsonValidation::check_unexpected_members(
+        attach_to_sources_value, {"port", "features"});
     JsonValidation::string(attach_to_sources_value, /* field */ "port");
     auto root = Root::from_json(attach_to_sources_value["port"]);
     JsonValidation::null_or_array(
@@ -1086,6 +1110,8 @@ Model Model::from_json(
 
   for (auto attach_to_sinks_value :
        JsonValidation::null_or_array(value, /* field */ "attach_to_sinks")) {
+    JsonValidation::check_unexpected_members(
+        attach_to_sinks_value, {"port", "features"});
     JsonValidation::string(attach_to_sinks_value, /* field */ "port");
     auto root = Root::from_json(attach_to_sinks_value["port"]);
     JsonValidation::null_or_array(
@@ -1097,6 +1123,8 @@ Model Model::from_json(
 
   for (auto attach_to_propagations_value : JsonValidation::null_or_array(
            value, /* field */ "attach_to_propagations")) {
+    JsonValidation::check_unexpected_members(
+        attach_to_propagations_value, {"port", "features"});
     JsonValidation::string(attach_to_propagations_value, /* field */ "port");
     auto root = Root::from_json(attach_to_propagations_value["port"]);
     JsonValidation::null_or_array(
@@ -1108,6 +1136,8 @@ Model Model::from_json(
 
   for (auto add_features_to_arguments_value : JsonValidation::null_or_array(
            value, /* field */ "add_features_to_arguments")) {
+    JsonValidation::check_unexpected_members(
+        add_features_to_arguments_value, {"port", "features"});
     JsonValidation::string(add_features_to_arguments_value, /* field */ "port");
     auto root = Root::from_json(add_features_to_arguments_value["port"]);
     JsonValidation::null_or_array(

diff --git a/source/Model.h b/source/Model.h
@@ -308,7 +308,8 @@ class Model final {
   static Model from_json(
       const Method* MT_NULLABLE method,
       const Json::Value& value,
-      Context& context);
+      Context& context,
+      bool check_unexpected_members = true);
   Json::Value to_json(ExportOriginsMode export_origins_mode) const;
 
   /* Export the model to json and include the method position. */

diff --git a/source/MultiSourceMultiSinkRule.cpp b/source/MultiSourceMultiSinkRule.cpp
@@ -41,6 +41,9 @@ std::unique_ptr<Rule> MultiSourceMultiSinkRule::from_json(
     const std::string& description,
     const Json::Value& value,
     Context& context) {
+  JsonValidation::check_unexpected_members(
+      value, {"name", "code", "description", "multi_sources", "partial_sinks"});
+
   const auto& sources = JsonValidation::object(value, "multi_sources");
   const auto& labels = sources.getMemberNames();
 

diff --git a/source/PropagationConfig.cpp b/source/PropagationConfig.cpp
@@ -15,6 +15,16 @@ PropagationConfig PropagationConfig::from_json(
     const Json::Value& value,
     Context& context) {
   JsonValidation::validate_object(value);
+  JsonValidation::check_unexpected_members(
+      value,
+      {"output",
+       "input",
+       "may_features",
+       "always_features",
+       "features",
+       "collapse",
+       "collapse-depth",
+       "transforms"});
 
   JsonValidation::string(value, /* field */ "output");
   auto output = AccessPath::from_json(value["output"]);
@@ -42,7 +52,8 @@ PropagationConfig PropagationConfig::from_json(
         "an access path to an argument or supported call effect");
   }
 
-  auto inferred_features = FeatureMayAlwaysSet::from_json(value, context);
+  auto inferred_features = FeatureMayAlwaysSet::from_json(
+      value, context, /* check_unexpected_members */ false);
   FeatureSet user_features = FeatureSet::from_json(value["features"], context);
 
   auto collapse_depth = CollapseDepth::zero();

diff --git a/source/Rule.cpp b/source/Rule.cpp
@@ -32,12 +32,12 @@ std::unique_ptr<Rule> Rule::from_json(
       value.isMember("multi_sources") && value.isMember("partial_sinks")) {
     return MultiSourceMultiSinkRule::from_json(
         name, code, description, value, context);
+  } else {
+    throw JsonValidationError(
+        value,
+        std::nullopt,
+        "keys: sources+sinks or multi_sources+partial_sinks");
   }
-
-  throw JsonValidationError(
-      value,
-      std::nullopt,
-      "keys: sources+sinks or multi_sources+partial_sinks");
 }
 
 Json::Value Rule::to_json() const {

diff --git a/source/Sanitizer.cpp b/source/Sanitizer.cpp
@@ -94,6 +94,12 @@ const Sanitizer Sanitizer::from_json(
     const Json::Value& value,
     Context& context) {
   JsonValidation::validate_object(value);
+  JsonValidation::check_unexpected_members(
+      value,
+      {"port", // Only when called from `TaintConfig::from_json`.
+       "sanitize",
+       "kinds"});
+
   SanitizerKind sanitizer_kind;
   auto sanitizer_kind_string =
       JsonValidation::string(value, /* field */ "sanitize");

diff --git a/source/SourceSinkRule.cpp b/source/SourceSinkRule.cpp
@@ -24,6 +24,9 @@ std::unique_ptr<Rule> SourceSinkRule::from_json(
     const std::string& description,
     const Json::Value& value,
     Context& context) {
+  JsonValidation::check_unexpected_members(
+      value, {"name", "code", "description", "sources", "sinks", "transforms"});
+
   KindSet source_kinds;
   for (const auto& source_kind :
        JsonValidation::nonempty_array(value, /* field */ "sources")) {

diff --git a/source/TaintConfig.cpp b/source/TaintConfig.cpp
@@ -91,8 +91,28 @@ AccessPath validate_and_infer_crtex_callee_port(
 
 TaintConfig TaintConfig::from_json(const Json::Value& value, Context& context) {
   JsonValidation::validate_object(value);
+  JsonValidation::check_unexpected_members(
+      value,
+      {"port", // Only when called from `Model::from_json`
+       "caller_port", // Only when called from `Model::from_json`
+       "type", // Only when called from `Model::from_json` for effects
+       "kind",
+       "partial_label",
+       "callee_port",
+       "callee",
+       "call_position",
+       "distance",
+       "origins",
+       "field_origins",
+       "features",
+       "may_features",
+       "always_features",
+       "via_type_of",
+       "via_value_of",
+       "canonical_names"});
 
-  const Kind* kind = Kind::from_json(value, context);
+  const Kind* kind =
+      Kind::from_json(value, context, /* check_unexpected_members */ false);
 
   auto callee_port = AccessPath(Root(Root::Kind::Leaf));
   if (value.isMember("callee_port")) {
@@ -127,7 +147,8 @@ TaintConfig TaintConfig::from_json(const Json::Value& value, Context& context) {
   // features should go under "user_features", but this gives a way to override
   // that behavior and specify "may/always" features. Note that local inferred
   // features cannot be user-specified.
-  auto inferred_features = FeatureMayAlwaysSet::from_json(value, context);
+  auto inferred_features = FeatureMayAlwaysSet::from_json(
+      value, context, /* check_unexpected_members */ false);
 
   // User specified always-features.
   FeatureSet user_features;

diff --git a/source/constraints/FieldConstraints.cpp b/source/constraints/FieldConstraints.cpp
@@ -179,23 +179,35 @@ std::unique_ptr<FieldConstraint> FieldConstraint::from_json(
   std::string constraint_name =
       JsonValidation::string(constraint, "constraint");
   if (constraint_name == "name") {
+    JsonValidation::check_unexpected_members(
+        constraint, {"constraint", "pattern"});
     return std::make_unique<FieldNameConstraint>(
         JsonValidation::string(constraint, /* field */ "pattern"));
   } else if (constraint_name == "signature") {
+    JsonValidation::check_unexpected_members(
+        constraint, {"constraint", "pattern"});
     return std::make_unique<SignaturePatternFieldConstraint>(
         JsonValidation::string(constraint, /* field */ "pattern"));
   } else if (constraint_name == "signature_pattern") {
+    JsonValidation::check_unexpected_members(
+        constraint, {"constraint", "pattern"});
     return std::make_unique<SignaturePatternFieldConstraint>(
         JsonValidation::string(constraint, /* field */ "pattern"));
   } else if (constraint_name == "is_static") {
+    JsonValidation::check_unexpected_members(
+        constraint, {"constraint", "value"});
     bool expected = constraint.isMember("value")
         ? JsonValidation::boolean(constraint, /* field */ "value")
         : true;
     return std::make_unique<IsStaticFieldConstraint>(expected);
   } else if (constraint_name == "not") {
+    JsonValidation::check_unexpected_members(
+        constraint, {"constraint", "inner"});
     return std::make_unique<NotFieldConstraint>(FieldConstraint::from_json(
         JsonValidation::object(constraint, /* field */ "inner")));
   } else if (constraint_name == "any_of" || constraint_name == "all_of") {
+    JsonValidation::check_unexpected_members(
+        constraint, {"constraint", "inners"});
     std::vector<std::unique_ptr<FieldConstraint>> constraints;
     for (const auto& inner :
          JsonValidation::null_or_array(constraint, /* field */ "inners")) {
@@ -207,13 +219,17 @@ std::unique_ptr<FieldConstraint> FieldConstraint::from_json(
       return std::make_unique<AllOfFieldConstraint>(std::move(constraints));
     }
   } else if (constraint_name == "has_annotation") {
+    JsonValidation::check_unexpected_members(
+        constraint, {"constraint", "type", "pattern"});
     return std::make_unique<HasAnnotationFieldConstraint>(
         JsonValidation::string(constraint, "type"),
         constraint.isMember("pattern")
             ? std::optional<std::string>{JsonValidation::string(
                   constraint, "pattern")}
             : std::nullopt);
   } else if (constraint_name == "parent") {
+    JsonValidation::check_unexpected_members(
+        constraint, {"constraint", "type", "inner"});
     return std::make_unique<ParentFieldConstraint>(TypeConstraint::from_json(
         JsonValidation::object(constraint, /* field */ "inner")));
   } else {