Addressed review commentsfix/validating_forwarded_requests

4 files changed, 41 insertions, 38 deletions

diff --git a/src/components/application_manager/include/application_manager/rpc_handler_impl.h b/src/components/application_manager/include/application_manager/rpc_handler_impl.h
index 50c0a0e96b..489bcd21fd 100644
--- a/src/components/application_manager/include/application_manager/rpc_handler_impl.h
+++ b/src/components/application_manager/include/application_manager/rpc_handler_impl.h
@@ -155,10 +155,8 @@ class RPCHandlerImpl : public RPCHandler,
       utils::SemanticVersion& message_version);
 
   bool ValidateRpcSO(smart_objects::SmartObject* message,
-                     uint32_t connection_key,
-                     uint32_t correlation_id,
-                     int32_t function_id,
                      utils::SemanticVersion& msg_version,
+                     rpc::ValidationReport& report_out,
                      bool remove_unknown_params) OVERRIDE;
 
  private:
@@ -167,7 +165,7 @@ class RPCHandlerImpl : public RPCHandler,
   bool ConvertMessageToSO(const Message& message,
                           smart_objects::SmartObject& output,
                           const bool remove_unknown_parameters = true,
-                          const bool validate_existing_params = true);
+                          const bool validate_params = true);
   std::shared_ptr<Message> ConvertRawMsgToMessage(
       const ::protocol_handler::RawMessagePtr message);
   hmi_apis::HMI_API& hmi_so_factory();
diff --git a/src/components/application_manager/src/rpc_handler_impl.cc b/src/components/application_manager/src/rpc_handler_impl.cc
index 5c4f65a4ed..65d5d713e8 100644
--- a/src/components/application_manager/src/rpc_handler_impl.cc
+++ b/src/components/application_manager/src/rpc_handler_impl.cc
@@ -284,7 +284,7 @@ bool RPCHandlerImpl::ConvertMessageToSO(
     const Message& message,
     ns_smart_device_link::ns_smart_objects::SmartObject& output,
     const bool remove_unknown_parameters,
-    const bool validate_existing_params) {
+    const bool validate_params) {
   LOG4CXX_AUTO_TRACE(logger_);
   LOG4CXX_DEBUG(logger_,
                 "\t\t\tMessage to convert: protocol "
@@ -305,6 +305,7 @@ bool RPCHandlerImpl::ConvertMessageToSO(
               message.correlation_id());
 
       smart_objects::SmartObject* so_ptr = (conversion_result) ? &output : NULL;
+      rpc::ValidationReport report("RPC");
 
       // Attach RPC version to SmartObject if it does not exist yet.
       auto app_ptr = app_manager_.application(message.connection_key());
@@ -317,18 +318,26 @@ bool RPCHandlerImpl::ConvertMessageToSO(
         GetMessageVersion(output, msg_version);
       }
 
-      if (validate_existing_params &&
-          !ValidateRpcSO(so_ptr,
-                         message.connection_key(),
-                         message.correlation_id(),
-                         message.function_id(),
-                         msg_version,
-                         remove_unknown_parameters)) {
+      if (validate_params &&
+          !ValidateRpcSO(
+              so_ptr, msg_version, report, remove_unknown_parameters)) {
         LOG4CXX_WARN(logger_,
                      "Failed to parse string to smart object with API version "
                          << msg_version.toString() << " : "
                          << message.json_message());
 
+        std::shared_ptr<smart_objects::SmartObject> response(
+            MessageHelper::CreateNegativeResponse(
+                message.connection_key(),
+                message.function_id(),
+                message.correlation_id(),
+                mobile_apis::Result::INVALID_DATA));
+
+        (*response)[strings::msg_params][strings::info] =
+            rpc::PrettyFormat(report);
+        app_manager_.GetRPCService().ManageMobileCommand(
+            response, commands::Command::SOURCE_SDL);
+
         return false;
       }
 
@@ -380,7 +389,7 @@ bool RPCHandlerImpl::ConvertMessageToSO(
 
       rpc::ValidationReport report("RPC");
 
-      if (validate_existing_params &&
+      if (validate_params &&
           output.validate(&report) != smart_objects::errors::OK) {
         LOG4CXX_ERROR(logger_,
                       "Incorrect parameter from HMI"
@@ -443,30 +452,15 @@ bool RPCHandlerImpl::ConvertMessageToSO(
 }
 
 bool RPCHandlerImpl::ValidateRpcSO(smart_objects::SmartObject* message,
-                                   uint32_t connection_key,
-                                   uint32_t correlation_id,
-                                   int32_t function_id,
                                    utils::SemanticVersion& msg_version,
+                                   rpc::ValidationReport& report_out,
                                    bool remove_unknown_params) {
-  rpc::ValidationReport report("RPC");
   if (!message ||
       !mobile_so_factory().attachSchema(
           *message, remove_unknown_params, msg_version) ||
-      (message->validate(&report, msg_version) != smart_objects::errors::OK)) {
-    LOG4CXX_WARN(logger_,
-                 "Failed to parse string to smart object with API version "
-                     << msg_version.toString() << " : " << message->asString());
-
-    std::shared_ptr<smart_objects::SmartObject> response(
-        MessageHelper::CreateNegativeResponse(
-            connection_key,
-            function_id,
-            correlation_id,
-            mobile_apis::Result::INVALID_DATA));
-
-    (*response)[strings::msg_params][strings::info] = rpc::PrettyFormat(report);
-    app_manager_.GetRPCService().ManageMobileCommand(
-        response, commands::Command::SOURCE_SDL);
+      (message->validate(&report_out, msg_version) !=
+       smart_objects::errors::OK)) {
+    LOG4CXX_WARN(logger_, "Failed to parse string to smart object");
     return false;
   }
   return true;
diff --git a/src/components/application_manager/src/rpc_passing_handler.cc b/src/components/application_manager/src/rpc_passing_handler.cc
index c62dbe1978..165b1f43ba 100644
--- a/src/components/application_manager/src/rpc_passing_handler.cc
+++ b/src/components/application_manager/src/rpc_passing_handler.cc
@@ -235,18 +235,31 @@ void RPCPassingHandler::ForwardRequestToCore(uint32_t correlation_id) {
   rpc_request_queue_lock_.Release();
 
   // Validate rpc message before forwarding to core
+  rpc::ValidationReport report("RPC");
   uint32_t connection_key =
       message[strings::params][strings::connection_key].asUInt();
   int32_t function_id = message[strings::params][strings::function_id].asInt();
-
   auto app_ptr = app_manager_.application(connection_key);
   utils::SemanticVersion msg_version(0, 0, 0);
   if (app_ptr) {
     msg_version = app_ptr->msg_version();
   }
 
-  app_manager_.GetRPCHandler().ValidateRpcSO(
-      &message, connection_key, correlation_id, function_id, msg_version, true);
+  if (!app_manager_.GetRPCHandler().ValidateRpcSO(
+          &message, msg_version, report, true)) {
+    std::shared_ptr<smart_objects::SmartObject> response(
+        MessageHelper::CreateNegativeResponse(
+            connection_key,
+            function_id,
+            correlation_id,
+            mobile_apis::Result::INVALID_DATA));
+
+    (*response)[strings::msg_params][strings::info] = rpc::PrettyFormat(report);
+    app_manager_.GetRPCService().ManageMobileCommand(
+        response, commands::Command::SOURCE_SDL);
+    return;
+  }
+
   app_manager_.GetRPCService().ManageMobileCommand(
       result, commands::Command::SOURCE_MOBILE);
 }
diff --git a/src/components/include/application_manager/rpc_handler.h b/src/components/include/application_manager/rpc_handler.h
index 88377e514a..a64ef44dfb 100644
--- a/src/components/include/application_manager/rpc_handler.h
+++ b/src/components/include/application_manager/rpc_handler.h
@@ -53,10 +53,8 @@ class RPCHandler
       {
  public:
   virtual bool ValidateRpcSO(smart_objects::SmartObject* message,
-                             uint32_t connection_key,
-                             uint32_t correlation_id,
-                             int32_t function_id,
                              utils::SemanticVersion& msg_version,
+                             rpc::ValidationReport& report_out,
                              bool remove_unknown_params) = 0;
 
   virtual ~RPCHandler() {}