Merge pull request #608 from mtconnect/fix_remote_host_logging_in_rest_sink

Author: wsobel

CMakeLists.txt

@@ -2,7 +2,7 @@
 set(AGENT_VERSION_MAJOR 2)
 set(AGENT_VERSION_MINOR 7)
 set(AGENT_VERSION_PATCH 0)
-set(AGENT_VERSION_BUILD 9)
+set(AGENT_VERSION_BUILD 10)
 set(AGENT_VERSION_RC "")
 
 # This minimum version is to support Visual Studio 2019 and C++ feature checking and FetchContent

src/mtconnect/agent.cpp

@@ -461,7 +461,14 @@ namespace mtconnect {
     {
       auto printer = dynamic_cast<printer::XmlPrinter *>(m_printers["xml"].get());
       auto device = m_xmlParser->parseDevice(deviceXml, printer);
-      loadDevices({device}, source);
+      if (device == nullptr)
+      {
+        LOG(error) << "Error loading device: " << deviceXml;
+      }
+      else
+      {
+        loadDevices({device}, source);
+      }
     }
     catch (runtime_error &e)
     {

src/mtconnect/configuration/agent_config.cpp

@@ -522,7 +522,8 @@ namespace mtconnect::configuration {
     }
 
     ptree empty;
-    auto logger = config.get_child_optional("logger_config").value_or(empty);
+    auto logger = config.get_child_optional("logger_config")
+                      .value_or(config.get_child_optional("logging").value_or(empty));
 
     const string defaultFileName = channelName + ".log";
     const string defaultArchivePattern = channelName + "_%Y-%m-%d_%H-%M-%S_%N.log";

src/mtconnect/parser/xml_parser.cpp

@@ -258,33 +258,87 @@ namespace mtconnect::parser {
   DevicePtr XmlParser::parseDevice(const std::string &deviceXml, printer::XmlPrinter *aPrinter)
   {
     DevicePtr device;
-
-    using namespace boost::adaptors;
-    using namespace boost::range;
-
     std::unique_lock lock(m_mutex);
 
+    // Parse the device XML unti an in memory doc and then see if we have a root MTConnectDevices
+    // node or a Device node. If devices, then we need to find the first device and then use the
+    // entity parser to parse the device. We then log any errors.
+
+    xmlDocPtr doc = nullptr;
     try
     {
-      entity::ErrorList errors;
-      auto entity = entity::XmlParser::parse(Device::getRoot(), deviceXml, errors);
-      if (errors.size() > 0)
+      xmlInitParser();
+      xmlSetGenericErrorFunc(nullptr, agentXMLErrorFunc);
+
+      doc = xmlReadMemory(deviceXml.c_str(), int32_t(deviceXml.length()), "DeviceStream.xml",
+                          nullptr, XML_PARSE_NOBLANKS);
+      if (!doc)
+        throw runtime_error("Failed to parse device XML");
+
+      auto root = xmlDocGetRootElement(doc);
+      if (!root)
+        throw runtime_error("Device XML has no root element");
+
+      xmlNodePtr deviceNode = nullptr;
+      if (xmlStrcmp(root->name, BAD_CAST "MTConnectDevices") == 0)
       {
-        LOG(warning) << "Errors parsing Device: " << deviceXml;
-        for (auto &e : errors)
+        for (auto child = root->children; child; child = child->next)
         {
-          LOG(warning) << "   " << e->what();
+          if (xmlStrcmp(child->name, BAD_CAST "Devices") == 0)
+          {
+            deviceNode = child->children;
+            break;
+          }
         }
       }
+      else if (xmlStrcmp(root->name, BAD_CAST "Device") == 0)
+      {
+        deviceNode = root;
+      }
       else
       {
-        device = dynamic_pointer_cast<Device>(entity);
+        throw runtime_error("Root element of device XML must be either MTConnectDevices or Device");
       }
+
+      if (!deviceNode)
+        throw runtime_error("No Device node found in device XML");
+
+      entity::ErrorList errors;
+      auto entity = entity::XmlParser::parseXmlNode(Device::getRoot(), deviceNode, errors);
+
+      for (auto &e : errors)
+      {
+        if (entity)
+          LOG(warning) << "When parsing device, a problem was skipped: " << e->what();
+        else
+          LOG(error) << "Failed to parse device: " << e->what();
+      }
+
+      if (entity)
+        device = dynamic_pointer_cast<Device>(entity);
+      else
+        LOG(error) << "Failed to parse device, skipping";
+
+      xmlFreeDoc(doc);
+      doc = nullptr;
+    }
+    catch (const runtime_error &e)
+    {
+      if (doc)
+        xmlFreeDoc(doc);
+      LOG(error) << "Cannot parse device XML: " << e.what();
     }
     catch (const string &e)
     {
-      LOG(fatal) << "Cannot parse XML document: " << e;
-      throw FatalException();
+      if (doc)
+        xmlFreeDoc(doc);
+      LOG(error) << "Cannot parse XML document: " << e;
+    }
+    catch (...)
+    {
+      if (doc)
+        xmlFreeDoc(doc);
+      LOG(error) << "Cannot parse XML document: unknown exception";
     }
 
     return device;

src/mtconnect/sink/rest_sink/request.hpp

@@ -45,6 +45,7 @@ namespace mtconnect::sink::rest_sink {
     std::string m_contentType;             ///< The content type for the body
     std::string m_path;                    ///< The URI for the request
     std::string m_foreignIp;               ///< The requestors IP Address
+    std::string m_foreignHost;             ///< The requestors IP Address
     uint16_t m_foreignPort;                ///< The requestors Port
     QueryMap m_query;                      ///< The parsed query parameters
     ParameterMap m_parameters;             ///< The parsed path parameters

src/mtconnect/sink/rest_sink/server.cpp

@@ -121,6 +121,12 @@ namespace mtconnect::sink::rest_sink {
       fail(ec, "Cannot open server socket");
       return;
     }
+    if (m_address.is_v6())
+    {
+      // Enable dual-stack by default. It will allow 0.0.0.0 if ::
+      m_acceptor.set_option(boost::asio::ip::v6_only(false), ec);
+      ec = {};  // not fatal if unsupported
+    }
     m_acceptor.set_option(boost::asio::socket_base::reuse_address(true), ec);
     if (ec)
     {

src/mtconnect/sink/rest_sink/server.hpp

@@ -52,7 +52,7 @@ namespace mtconnect::sink::rest_sink {
     /// @param options configuration options
     /// - Port, defaults to 5000
     /// - AllowPut, defaults to false
-    /// - ServerIp, defaults to 0.0.0.0
+    /// - ServerIp, defaults to ::
     /// - HttpHeaders
     Server(boost::asio::io_context &context, const ConfigOptions &options = {})
       : m_context(context),
@@ -65,7 +65,7 @@ namespace mtconnect::sink::rest_sink {
       auto inter = GetOption<std::string>(options, configuration::ServerIp);
       if (!inter)
       {
-        m_address = boost::asio::ip::make_address("0.0.0.0");
+        m_address = boost::asio::ip::make_address("::");
       }
       else
       {
@@ -201,6 +201,7 @@ namespace mtconnect::sink::rest_sink {
           RestError re(error, request->m_accepts, status::not_found, std::nullopt,
                        request->m_requestId);
           re.setUri(request->getUri());
+          LOG(warning) << "[" << request->m_foreignHost << "]: " << txt.str();
           m_errorFunction(session, re);
         }
       }

src/mtconnect/sink/rest_sink/session_impl.cpp

@@ -239,11 +239,17 @@ namespace mtconnect::sink::rest_sink {
     }
 
     m_request->m_foreignIp = remote.address().to_string();
+    if (auto a = msg.find(http::field::forwarded); a != msg.end())
+      m_request->m_foreignHost = string(a->value());
+    else if (auto a = msg.find(http::field::host); a != msg.end())
+      m_request->m_foreignHost = string(a->value());
+    else
+      m_request->m_foreignHost = m_request->m_foreignIp;
     m_request->m_foreignPort = remote.port();
     if (auto a = msg.find(http::field::connection); a != msg.end())
       m_close = a->value() == "close";
 
-    LOG(info) << "ReST Request: From [" << m_request->m_foreignIp << ':' << remote.port()
+    LOG(info) << "ReST Request: From [" << m_request->m_foreignHost << ':' << remote.port()
               << "]: " << msg.method() << " " << msg.target();
 
     // Check if this is a websocket upgrade request. If so, begin a websocket session.
@@ -302,7 +308,7 @@ namespace mtconnect::sink::rest_sink {
     m_complete = complete;
     m_mimeType = mimeType;
     m_streaming = true;
-    
+
     auto now = std::chrono::floor<std::chrono::seconds>(std::chrono::system_clock::now());
     std::string date = std::format("{:%a, %d %b %Y %T} GMT", now);
 

test_package/agent_asset_test.cpp

@@ -643,7 +643,7 @@ TEST_F(AgentAssetTest, should_add_asset_changed_and_asset_added_with_discrete_in
   }
 }
 
-TEST_F(AgentAssetTest, AssetPrependId)
+TEST_F(AgentAssetTest, asset_id_is_zero_padded_when_prepend_id_prefix_is_used)
 {
   addAdapter();
   auto agent = m_agentTestHelper->getAgent();
@@ -957,7 +957,7 @@ TEST_F(AgentAssetTest, should_respond_to_http_push_with_list_of_errors)
   }
 }
 
-TEST_F(AgentAssetTest, update_asset_count_data_item_v2_0)
+TEST_F(AgentAssetTest, asset_count_data_set_is_updated_when_assets_are_added_or_removed)
 {
   m_agentTestHelper->createAgent("/samples/test_config.xml", 8, 10, "2.0", 4, true);
   addAdapter();
@@ -1031,7 +1031,7 @@ TEST_F(AgentAssetTest, update_asset_count_data_item_v2_0)
   }
 }
 
-TEST_F(AgentAssetTest, asset_count_should_not_occur_in_header_post_20)
+TEST_F(AgentAssetTest, asset_count_is_absent_from_probe_header_in_schema_2_0)
 {
   auto agent = m_agentTestHelper->createAgent("/samples/test_config.xml", 8, 4, "2.0", 4, true);
 
@@ -1057,7 +1057,7 @@ TEST_F(AgentAssetTest, asset_count_should_not_occur_in_header_post_20)
   }
 }
 
-TEST_F(AgentAssetTest, asset_count_should_track_asset_additions_by_type)
+TEST_F(AgentAssetTest, asset_count_tracks_additions_and_removals_per_type)
 {
   auto agent = m_agentTestHelper->createAgent("/samples/test_config.xml", 8, 4, "2.0", 4, true);
 
@@ -1115,7 +1115,7 @@ TEST_F(AgentAssetTest, asset_count_should_track_asset_additions_by_type)
   }
 }
 
-TEST_F(AgentAssetTest, asset_should_also_work_using_post_with_assets)
+TEST_F(AgentAssetTest, assets_endpoint_accepts_post_requests_for_asset_storage)
 {
   auto agent = m_agentTestHelper->createAgent("/samples/test_config.xml", 8, 4, "2.0", 4, true);
 

test_package/agent_test.cpp

@@ -88,7 +88,7 @@ class AgentTest : public testing::Test
   std::chrono::milliseconds m_delay {};
 };
 
-TEST_F(AgentTest, Constructor)
+TEST_F(AgentTest, agent_throws_fatal_exception_for_bad_path_and_initializes_with_valid_path)
 {
   using namespace configuration;
   ConfigOptions options {{BufferSize, 17}, {MaxAssets, 8}, {SchemaVersion, "1.7"s}};
@@ -109,7 +109,7 @@ TEST_F(AgentTest, Constructor)
   ASSERT_NO_THROW(agent->initialize(context));
 }
 
-TEST_F(AgentTest, Probe)
+TEST_F(AgentTest, probe_endpoint_returns_device_name_for_multiple_paths)
 {
   {
     PARSE_XML_RESPONSE("/probe");
@@ -132,7 +132,7 @@ TEST_F(AgentTest, Probe)
   }
 }
 
-TEST_F(AgentTest, FailWithDuplicateDeviceUUID)
+TEST_F(AgentTest, agent_throws_fatal_exception_when_devices_have_duplicate_uuid)
 {
   using namespace configuration;
   ConfigOptions options {{BufferSize, 17}, {MaxAssets, 8}, {SchemaVersion, "1.5"s}};
@@ -519,7 +519,7 @@ TEST_F(AgentTest, should_report_2_6_out_of_range_for_current_at)
   }
 }
 
-TEST_F(AgentTest, AddAdapter) { addAdapter(); }
+TEST_F(AgentTest, adapter_can_be_added_to_agent) { addAdapter(); }
 
 TEST_F(AgentTest, should_download_file)
 {
@@ -1271,7 +1271,7 @@ TEST_F(AgentTest, should_ignore_timestamps_if_configured_to_do_so)
   }
 }
 
-TEST_F(AgentTest, InitialTimeSeriesValues)
+TEST_F(AgentTest, time_series_data_item_reports_unavailable_initially)
 {
   addAdapter();
 
@@ -2401,7 +2401,7 @@ TEST_F(AgentTest, put_condition_should_parse_condition_data)
   }
 }
 
-TEST_F(AgentTest, shound_add_asset_count_when_20)
+TEST_F(AgentTest, asset_count_data_item_is_added_to_probe_in_schema_2_0)
 {
   m_agentTestHelper->createAgent("/samples/min_config.xml", 8, 4, "2.0", 25);