Merge pull request #223 from microsoft/pete-dev

Hardening of service to malformed json, and more console speed improvements
microsoft · Jan 16, 2024 · 0eb49a3 · 0eb49a3
2 parents 556c7e8 + cbfaf4f
commit 0eb49a3
Show file tree

Hide file tree

Showing 11 changed files with 298 additions and 133 deletions.
diff --git a/build/staging/configuration/Default.midiconfig.json b/build/staging/configuration/Default.midiconfig.json
@@ -10,9 +10,7 @@
     {
         "{26FA740D-469C-4D33-BEB1-3885DE7D6DF1}":
         {
-            "_comment": "KS MIDI (USB etc.)",
-
-
+            "_comment": "KS MIDI (USB etc.)"
         },
         "{C95DCD1F-CDE3-4C2D-913C-528CB8A4CBE6}":
         {

diff --git a/build/staging/version/BundleInfo.wxi b/build/staging/version/BundleInfo.wxi
@@ -1,4 +1,4 @@
 <Include>
   <?define SetupVersionName="Developer Preview 4" ?>
-  <?define SetupVersionNumber="1.0.24015.1748" ?>
+  <?define SetupVersionNumber="1.0.24016.1413" ?>
 </Include>
diff --git a/src/api/Abstraction/KSAbstraction/Midi2.KSMidiEndpointManager.cpp b/src/api/Abstraction/KSAbstraction/Midi2.KSMidiEndpointManager.cpp
@@ -47,7 +47,10 @@ CMidi2KSMidiEndpointManager::Initialize(
 
             if (jsonString != L"")
             {
-                m_jsonObject = json::JsonObject::Parse(configurationJson);
+                if (json::JsonObject::TryParse(configurationJson, m_jsonObject))
+                {
+                    // worked
+                }
             }
         }
     }
@@ -475,68 +478,77 @@ _Use_decl_annotations_
 HRESULT
 CMidi2KSMidiEndpointManager::ApplyUserConfiguration(std::wstring deviceInterfaceId)
 {
-    // if we have no configuration, that's ok
-    if (m_jsonObject == nullptr) return S_OK;
+    OutputDebugString(L"\n" __FUNCTION__);
 
+    try
+    {
+        // if we have no configuration, that's ok
+        if (m_jsonObject == nullptr) return S_OK;
 
-    std::vector<DEVPROPERTY> endpointProperties;
 
-    // for now, we only support lookup by the deviceInterfaceId, so this code is easier
+        std::vector<DEVPROPERTY> endpointProperties;
 
-    winrt::hstring endpointSettingsKey = winrt::to_hstring(MIDI_CONFIG_JSON_ENDPOINT_IDENTIFIER_SWD) + deviceInterfaceId;
+        // for now, we only support lookup by the deviceInterfaceId, so this code is easier
 
-    //OutputDebugString(L"\n" __FUNCTION__ L" Key: ");
-    //OutputDebugString(endpointSettingsKey.c_str());
-    //OutputDebugString(L"\n");
+        winrt::hstring endpointSettingsKey = winrt::to_hstring(MIDI_CONFIG_JSON_ENDPOINT_IDENTIFIER_SWD) + deviceInterfaceId;
 
+        //OutputDebugString(L" Key: ");
+        //OutputDebugString(endpointSettingsKey.c_str());
+        //OutputDebugString(L"\n");
 
-    if (m_jsonObject.HasKey(endpointSettingsKey))
-    {
-        json::JsonObject endpointSettings = m_jsonObject.GetNamedObject(endpointSettingsKey);
 
-        // Get the user-specified endpoint name
-        if (endpointSettings != nullptr && endpointSettings.HasKey(MIDI_CONFIG_JSON_ENDPOINT_USER_SUPPLIED_NAME_PROPERTY_KEY))
+        if (m_jsonObject.HasKey(endpointSettingsKey))
         {
-            auto name = endpointSettings.GetNamedString(MIDI_CONFIG_JSON_ENDPOINT_USER_SUPPLIED_NAME_PROPERTY_KEY);
+            json::JsonObject endpointSettings = m_jsonObject.GetNamedObject(endpointSettingsKey);
 
-            endpointProperties.push_back({ {PKEY_MIDI_UserSuppliedEndpointName, DEVPROP_STORE_SYSTEM, nullptr},
-                    DEVPROP_TYPE_STRING, static_cast<ULONG>((name.size() + 1) * sizeof(WCHAR)), (PVOID)name.c_str() });
-        }
+            // Get the user-specified endpoint name
+            if (endpointSettings != nullptr && endpointSettings.HasKey(MIDI_CONFIG_JSON_ENDPOINT_USER_SUPPLIED_NAME_PROPERTY_KEY))
+            {
+                auto name = endpointSettings.GetNamedString(MIDI_CONFIG_JSON_ENDPOINT_USER_SUPPLIED_NAME_PROPERTY_KEY);
 
-        // Get the user-specified endpoint description
-        if (endpointSettings != nullptr && endpointSettings.HasKey(MIDI_CONFIG_JSON_ENDPOINT_USER_SUPPLIED_DESCRIPTION_PROPERTY_KEY))
-        {
-            auto description = endpointSettings.GetNamedString(MIDI_CONFIG_JSON_ENDPOINT_USER_SUPPLIED_DESCRIPTION_PROPERTY_KEY);
+                endpointProperties.push_back({ {PKEY_MIDI_UserSuppliedEndpointName, DEVPROP_STORE_SYSTEM, nullptr},
+                        DEVPROP_TYPE_STRING, static_cast<ULONG>((name.size() + 1) * sizeof(WCHAR)), (PVOID)name.c_str() });
+            }
 
-            endpointProperties.push_back({ {PKEY_MIDI_UserSuppliedDescription, DEVPROP_STORE_SYSTEM, nullptr},
-                    DEVPROP_TYPE_STRING, static_cast<ULONG>((description.size() + 1) * sizeof(WCHAR)), (PVOID)description.c_str() });
-        }
+            // Get the user-specified endpoint description
+            if (endpointSettings != nullptr && endpointSettings.HasKey(MIDI_CONFIG_JSON_ENDPOINT_USER_SUPPLIED_DESCRIPTION_PROPERTY_KEY))
+            {
+                auto description = endpointSettings.GetNamedString(MIDI_CONFIG_JSON_ENDPOINT_USER_SUPPLIED_DESCRIPTION_PROPERTY_KEY);
 
-        // Get the user-specified multiclient override
-        if (endpointSettings != nullptr && endpointSettings.HasKey(MIDI_CONFIG_JSON_ENDPOINT_FORCE_SINGLE_CLIENT_PROPERTY_KEY))
-        {
-            auto forceSingleClient = endpointSettings.GetNamedBoolean(MIDI_CONFIG_JSON_ENDPOINT_FORCE_SINGLE_CLIENT_PROPERTY_KEY);
+                endpointProperties.push_back({ {PKEY_MIDI_UserSuppliedDescription, DEVPROP_STORE_SYSTEM, nullptr},
+                        DEVPROP_TYPE_STRING, static_cast<ULONG>((description.size() + 1) * sizeof(WCHAR)), (PVOID)description.c_str() });
+            }
 
-            if (forceSingleClient)
+            // Get the user-specified multiclient override
+            if (endpointSettings != nullptr && endpointSettings.HasKey(MIDI_CONFIG_JSON_ENDPOINT_FORCE_SINGLE_CLIENT_PROPERTY_KEY))
             {
-                DEVPROP_BOOLEAN devPropFalse = DEVPROP_FALSE;
+                auto forceSingleClient = endpointSettings.GetNamedBoolean(MIDI_CONFIG_JSON_ENDPOINT_FORCE_SINGLE_CLIENT_PROPERTY_KEY);
 
-                endpointProperties.push_back({ {PKEY_MIDI_SupportsMulticlient, DEVPROP_STORE_SYSTEM, nullptr},
-                        DEVPROP_TYPE_BOOLEAN, static_cast<ULONG>(sizeof(devPropFalse)), (PVOID)&devPropFalse });
+                if (forceSingleClient)
+                {
+                    DEVPROP_BOOLEAN devPropFalse = DEVPROP_FALSE;
+
+                    endpointProperties.push_back({ {PKEY_MIDI_SupportsMulticlient, DEVPROP_STORE_SYSTEM, nullptr},
+                            DEVPROP_TYPE_BOOLEAN, static_cast<ULONG>(sizeof(devPropFalse)), (PVOID)&devPropFalse });
+                }
             }
-        }
 
-        // apply supported property changes.
-        if (endpointProperties.size() > 0)
-        {
-            return m_MidiDeviceManager->UpdateEndpointProperties(
-                (LPCWSTR)deviceInterfaceId.c_str(),
-                (ULONG)endpointProperties.size(),
-                (PVOID)endpointProperties.data());
+            // apply supported property changes.
+            if (endpointProperties.size() > 0)
+            {
+                return m_MidiDeviceManager->UpdateEndpointProperties(
+                    (LPCWSTR)deviceInterfaceId.c_str(),
+                    (ULONG)endpointProperties.size(),
+                    (PVOID)endpointProperties.data());
+            }
         }
-    }
 
-    return S_OK;
+        return S_OK;
+    }
+    catch (...)
+    {
+        return E_FAIL;
+    }
 }
 
 

diff --git a/src/api/Abstraction/KSAbstraction/Midi2.KSMidiEndpointManager.h b/src/api/Abstraction/KSAbstraction/Midi2.KSMidiEndpointManager.h
@@ -63,7 +63,7 @@ class CMidi2KSMidiEndpointManager :
     wil::unique_event m_EnumerationCompleted{wil::EventOptions::None};
 
     // may want to change this to the actual json object.
-    winrt::Windows::Data::Json::JsonObject m_jsonObject{};
+    winrt::Windows::Data::Json::JsonObject m_jsonObject{ nullptr };
     HRESULT ApplyUserConfiguration(_In_ std::wstring deviceInterfaceId);
 
 };
diff --git a/src/api/Service/Exe/MidiConfigurationManager.cpp b/src/api/Service/Exe/MidiConfigurationManager.cpp
@@ -341,7 +341,7 @@ std::wstring CMidiConfigurationManager::GetCurrentConfigurationFileName() noexce
 
 HRESULT CMidiConfigurationManager::Initialize()
 {
-    //OutputDebugString(L"" __FUNCTION__);
+    OutputDebugString(L"\n" __FUNCTION__);
 
     // load the current configuration
 
@@ -371,13 +371,10 @@ HRESULT CMidiConfigurationManager::Initialize()
             // try to read the text from the file
             fileContents = winrt::Windows::Storage::FileIO::ReadTextAsync(file).get();
 
-
-            //OutputDebugString(L"" __FUNCTION__);
-            //OutputDebugString(fileContents.c_str());
         }
         catch (...)
         {
-            OutputDebugString(L"Exception opening json config file");
+            OutputDebugString(L"Exception opening json config file\n");
 
             // file does not exist or we can't open it
             // we don't fail if no configuration file, we just don't config anything
@@ -397,20 +394,28 @@ HRESULT CMidiConfigurationManager::Initialize()
 
             try
             {
-                m_jsonObject = json::JsonObject::Parse(fileContents);
+                if (json::JsonObject::TryParse(fileContents, m_jsonObject))
+                {
+                    // worked
+                    OutputDebugString(L"Parsing json worked\n");
+                }
+                else
+                {
+                    OutputDebugString(L"Parsing json failed\n");
+                }
             }
             CATCH_LOG()
         }
         else
         {
-            OutputDebugString(L"Config file contents are empty");
+            OutputDebugString(L"Config file contents are empty\n");
         }
 
     }
     else
     {
         // config file is missing
-        OutputDebugString(L"Config file is missing");
+        OutputDebugString(L"Config file is missing, but that's ok.\n");
 
         return S_OK;
     }
@@ -422,31 +427,37 @@ HRESULT CMidiConfigurationManager::Initialize()
 _Use_decl_annotations_
 std::wstring CMidiConfigurationManager::GetConfigurationForTransportAbstraction(GUID abstractionGuid) const noexcept
 {
-    OutputDebugString(L"" __FUNCTION__);
+    OutputDebugString(L"\n" __FUNCTION__);
 
-    auto key = GuidToString(abstractionGuid);
+    try
+    {
+        OutputDebugString(L"" __FUNCTION__);
 
-//    OutputDebugString(key.c_str());
+        auto key = GuidToString(abstractionGuid);
 
-    if (m_jsonObject != nullptr)
-    {
-        // probably need to normalize these to ignore case. Not sure if WinRT Json dictionary is case-sensitive
-        if (m_jsonObject.HasKey(winrt::to_hstring(MIDI_CONFIG_JSON_TRANSPORT_PLUGIN_SETTINGS_OBJECT)))
-        {
-            auto plugins = m_jsonObject.GetNamedObject(MIDI_CONFIG_JSON_TRANSPORT_PLUGIN_SETTINGS_OBJECT);
+        //    OutputDebugString(key.c_str());
 
-            if (plugins.HasKey(key))
+        if (m_jsonObject != nullptr)
+        {
+            // probably need to normalize these to ignore case. Not sure if WinRT Json dictionary is case-sensitive
+            if (m_jsonObject.HasKey(winrt::to_hstring(MIDI_CONFIG_JSON_TRANSPORT_PLUGIN_SETTINGS_OBJECT)))
             {
-                auto thisPlugin = plugins.GetNamedObject(key);
+                auto plugins = m_jsonObject.GetNamedObject(MIDI_CONFIG_JSON_TRANSPORT_PLUGIN_SETTINGS_OBJECT);
 
-                std::wstring jsonString = (std::wstring)thisPlugin.Stringify();
+                if (plugins.HasKey(key))
+                {
+                    auto thisPlugin = plugins.GetNamedObject(key);
+
+                    std::wstring jsonString = (std::wstring)thisPlugin.Stringify();
 
-                //OutputDebugString(jsonString.c_str());
+                    //OutputDebugString(jsonString.c_str());
 
-                return jsonString;
+                    return jsonString;
+                }
             }
         }
     }
+    CATCH_LOG();
 
     return L"";
 }
@@ -456,31 +467,37 @@ std::wstring CMidiConfigurationManager::GetConfigurationForTransportAbstraction(
 _Use_decl_annotations_
 std::wstring CMidiConfigurationManager::GetConfigurationForEndpointProcessingTransform(GUID abstractionGuid) const noexcept
 {
- //   OutputDebugString(L"" __FUNCTION__);
+    OutputDebugString(L"\n" __FUNCTION__);
+
+    try
+    {
+        //   OutputDebugString(L"" __FUNCTION__);
 
-    auto key = GuidToString(abstractionGuid);
+        auto key = GuidToString(abstractionGuid);
 
-//    OutputDebugString(key.c_str());
+        //    OutputDebugString(key.c_str());
 
-    if (m_jsonObject != nullptr)
-    {
-        // probably need to normalize these to ignore case. Not sure if WinRT Json dictionary is case-sensitive
-        if (m_jsonObject.HasKey(winrt::to_hstring(MIDI_CONFIG_JSON_ENDPOINT_PROCESSING_PLUGIN_SETTINGS_OBJECT)))
+        if (m_jsonObject != nullptr)
         {
-            auto plugins = m_jsonObject.GetNamedObject(MIDI_CONFIG_JSON_ENDPOINT_PROCESSING_PLUGIN_SETTINGS_OBJECT);
-
-            if (plugins.HasKey(key))
+            // probably need to normalize these to ignore case. Not sure if WinRT Json dictionary is case-sensitive
+            if (m_jsonObject.HasKey(winrt::to_hstring(MIDI_CONFIG_JSON_ENDPOINT_PROCESSING_PLUGIN_SETTINGS_OBJECT)))
             {
-                auto thisPlugin = plugins.GetNamedObject(key);
+                auto plugins = m_jsonObject.GetNamedObject(MIDI_CONFIG_JSON_ENDPOINT_PROCESSING_PLUGIN_SETTINGS_OBJECT);
 
-                std::wstring jsonString = (std::wstring)thisPlugin.Stringify();
+                if (plugins.HasKey(key))
+                {
+                    auto thisPlugin = plugins.GetNamedObject(key);
 
-                OutputDebugString(jsonString.c_str());
+                    std::wstring jsonString = (std::wstring)thisPlugin.Stringify();
 
-                return jsonString;
+                    OutputDebugString(jsonString.c_str());
+
+                    return jsonString;
+                }
             }
         }
     }
+    CATCH_LOG();
 
     return L"";
 }
@@ -489,6 +506,7 @@ std::wstring CMidiConfigurationManager::GetConfigurationForEndpointProcessingTra
 
 HRESULT CMidiConfigurationManager::Cleanup() noexcept
 {
+    OutputDebugString(L"\n" __FUNCTION__);
 
 
     return S_OK;

diff --git a/src/oob-setup/api-package/WindowsMidiServices.wxs b/src/oob-setup/api-package/WindowsMidiServices.wxs
@@ -44,7 +44,6 @@
         <!-- Setting the SelfRegCost attribute to any positive number causes DLL Self Registration -->
 
        <Component Id="WindowsServiceAbstractions"
-                   SharedDllRefCount="no"
                    Bitness="always64"
                    Directory="SERVICE_INSTALLFOLDER"
                    Guid="{6a2300f5-ccae-4f61-b849-d41aefb8a999}"  >
@@ -66,7 +65,6 @@
        </Component>
 
         <Component Id="WindowsService"
-                   SharedDllRefCount="no"
                    Bitness="always64"
                    Directory="SERVICE_INSTALLFOLDER"
                    Guid="6a2300f5-ccae-4f61-b849-d41aefb8a2f7" >