chore: apply feedbacks

Signed-off-by: Alexandre Rulleau <[email protected]>

appsec/src/extension/attributes.h

@@ -6,8 +6,6 @@
 #ifndef DD_ATTRIBUTES_H
 #define DD_ATTRIBUTES_H
 
-#include "configuration.h"
-
 #ifndef __has_feature
 # define __has_feature(x) 0
 #endif

appsec/src/extension/commands_helpers.c

@@ -574,14 +574,14 @@ static void dd_command_process_settings(mpack_node_t root)
         mpack_node_t value = mpack_node_map_value_at(root, i);
 
         if (mpack_node_type(key) != mpack_type_str) {
-            mlog(dd_log_warning, "Failed to process user collection setting: "
+            mlog(dd_log_warning, "Failed to process unknown setting: "
                                  "invalid type for key");
-            return;
+            continue;
         }
         if (mpack_node_type(value) != mpack_type_str) {
-            mlog(dd_log_warning, "Failed to process user collection setting: "
+            mlog(dd_log_warning, "Failed to process unknown setting: "
                                  "invalid type for value");
-            return;
+            continue;
         }
 
         const char *key_str = mpack_node_str(key);
@@ -599,7 +599,6 @@ static void dd_command_process_settings(mpack_node_t root)
                     "unknown key %.*s",
                     (int)key_len, key_str);
             }
-            return;
         }
     }
 }

appsec/src/extension/commands_helpers.h

@@ -5,8 +5,8 @@
 // (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc.
 #pragma once
 
-#include "attributes.h"
 #include "commands_ctx.h"
+#include "attributes.h"
 #include "dddefs.h"
 #include "network.h"
 #include <mpack.h>

appsec/src/extension/user_tracking.h

@@ -5,6 +5,7 @@
 // (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc.
 #pragma once
 
+#include "zai_string/string.h"
 #include "attributes.h"
 #include <zend.h>
 

appsec/src/helper/remote_config/listeners/asm_features_listener.cpp

@@ -19,12 +19,6 @@ void asm_features_listener::init()
     aggregator_->init(&ruleset_.GetAllocator());
 }
 
-void asm_features_listener::on_unapply(const config & /*config*/)
-{
-    service_config_->unset_asm();
-    service_config_->unset_auto_user_instrum();
-}
-
 void asm_features_listener::on_update(const config &config)
 {
     aggregator_->add(config);
@@ -37,14 +31,14 @@ void asm_features_listener::commit()
     auto asm_itr =
         json_helper::get_field_of_type(ruleset_, "asm", rapidjson::kObjectType);
     if (!asm_itr) {
-        throw error_applying_config("Invalid config json encoded contents: "
-                                    "asm key missing or invalid");
+        service_config_->unset_asm();
+        return;
     }
 
     auto enabled_itr = asm_itr.value()->value.FindMember("enabled");
     if (enabled_itr == asm_itr.value()->value.MemberEnd()) {
-        throw error_applying_config(
-            "Invalid config json encoded contents: enabled key missing");
+        service_config_->unset_asm();
+        return;
     }
 
     if (enabled_itr->value.GetType() == rapidjson::kStringType) {
@@ -62,13 +56,12 @@ void asm_features_listener::commit()
         // when appsec should not be enabled
         service_config_->disable_asm();
     } else {
-        throw error_applying_config(
-            "Invalid config json encoded contents: enabled key invalid");
+        service_config_->unset_asm();
+        return;
     }
 
     if (!json_helper::field_exists(ruleset_, "auto_user_instrum")) {
-        service_config_->set_auto_user_instrum(
-            auto_user_instrum_mode::UNDEFINED);
+        service_config_->unset_auto_user_instrum();
         return;
     }
 

appsec/src/helper/remote_config/listeners/asm_features_listener.hpp

@@ -21,7 +21,7 @@ class asm_features_listener : public listener_base {
 
     void init() override;
     void on_update(const config &config) override;
-    void on_unapply(const config &config) override;
+    void on_unapply(const config &config) override {}
     void commit() override;
 
     [[nodiscard]] std::unordered_set<product> get_supported_products() override

appsec/tests/helper/remote_config/listeners/asm_features_listener_test.cpp

@@ -457,24 +457,19 @@ TEST(RemoteConfigAsmFeaturesListener, ErrorConfigAsmKeyMissing)
     EXPECT_EQ(auto_user_instrum_mode::UNDEFINED,
         remote_config_service->get_auto_user_intrum_mode());
 
-    std::string error_message = "";
-    std::string expected_error_message =
-        "Invalid config json encoded contents: asm key missing or invalid";
-
     remote_config::config config = rcmock::get_config("ASM_FEATURES", "{}");
 
     try {
         listener.on_update(config);
         listener.commit();
     } catch (remote_config::error_applying_config &error) {
-        error_message = error.what();
+        std::cout << error.what() << std::endl;
     }
 
     EXPECT_EQ(enable_asm_status::NOT_SET,
         remote_config_service->get_asm_enabled_status());
     EXPECT_EQ(auto_user_instrum_mode::UNDEFINED,
         remote_config_service->get_auto_user_intrum_mode());
-    EXPECT_EQ(0, error_message.compare(expected_error_message));
 }
 
 TEST(RemoteConfigAsmFeaturesListener, ErrorConfigInvalidAsmKey)
@@ -519,25 +514,20 @@ TEST(RemoteConfigAsmFeaturesListener, ErrorConfigEnabledKeyMissing)
     EXPECT_EQ(auto_user_instrum_mode::UNDEFINED,
         remote_config_service->get_auto_user_intrum_mode());
 
-    std::string error_message = "";
-    std::string expected_error_message =
-        "Invalid config json encoded contents: enabled key missing";
-
     remote_config::config config =
         rcmock::get_config("ASM_FEATURES", R"({ "asm": { "disabled": true }})");
 
     try {
         listener.on_update(config);
         listener.commit();
     } catch (remote_config::error_applying_config &error) {
-        error_message = error.what();
+        std::cout << error.what() << std::endl;
     }
 
     EXPECT_EQ(enable_asm_status::NOT_SET,
         remote_config_service->get_asm_enabled_status());
     EXPECT_EQ(auto_user_instrum_mode::UNDEFINED,
         remote_config_service->get_auto_user_intrum_mode());
-    EXPECT_EQ(0, error_message.compare(expected_error_message));
 }
 
 TEST(RemoteConfigAsmFeaturesListener, ErrorConfigInvalidEnabledKey)
@@ -551,53 +541,16 @@ TEST(RemoteConfigAsmFeaturesListener, ErrorConfigInvalidEnabledKey)
     EXPECT_EQ(auto_user_instrum_mode::UNDEFINED,
         remote_config_service->get_auto_user_intrum_mode());
 
-    std::string error_message = "";
-    std::string expected_error_message =
-        "Invalid config json encoded contents: enabled key invalid";
-
     remote_config::config config =
         rcmock::get_config("ASM_FEATURES", R"({ "asm": { "enabled": 123 }})");
 
     try {
         listener.on_update(config);
         listener.commit();
-    } catch (remote_config::error_applying_config &error) {
-        error_message = error.what();
-    }
-
-    EXPECT_EQ(enable_asm_status::NOT_SET,
-        remote_config_service->get_asm_enabled_status());
-    EXPECT_EQ(auto_user_instrum_mode::UNDEFINED,
-        remote_config_service->get_auto_user_intrum_mode());
-    EXPECT_EQ(0, error_message.compare(expected_error_message));
-}
-
-TEST(RemoteConfigAsmFeaturesListener, UnapplyWithAsmEnabled)
-{
-    auto remote_config_service = std::make_shared<service_config>();
-    remote_config::asm_features_listener listener(remote_config_service);
-    listener.init();
-
-    EXPECT_EQ(enable_asm_status::NOT_SET,
-        remote_config_service->get_asm_enabled_status());
-    EXPECT_EQ(auto_user_instrum_mode::UNDEFINED,
-        remote_config_service->get_auto_user_intrum_mode());
-
-    try {
-        listener.on_update(get_asm_enabled_config());
-        listener.on_update(get_auto_user_instrum_config());
-        listener.commit();
     } catch (remote_config::error_applying_config &error) {
         std::cout << error.what() << std::endl;
     }
 
-    EXPECT_EQ(enable_asm_status::ENABLED,
-        remote_config_service->get_asm_enabled_status());
-    EXPECT_EQ(auto_user_instrum_mode::IDENTIFICATION,
-        remote_config_service->get_auto_user_intrum_mode());
-
-    listener.on_unapply(get_asm_enabled_config());
-
     EXPECT_EQ(enable_asm_status::NOT_SET,
         remote_config_service->get_asm_enabled_status());
     EXPECT_EQ(auto_user_instrum_mode::UNDEFINED,