fix: Prevent null reference exceptions while serializing LoadedModule…

…WireModelCollection. (#2185) (#2187)

src/Agent/NewRelic/Agent/Core/JsonConverters/LoadedModuleWireModelCollectionJsonConverter.cs

@@ -41,7 +41,7 @@ private static void WriteJsonImpl(JsonWriter jsonWriter, LoadedModuleWireModelCo
                 foreach (var item in loadedModule.Data)
                 {
                     jsonWriter.WritePropertyName(item.Key);
-                    jsonWriter.WriteValue(item.Value.ToString());
+                    jsonWriter.WriteValue(item.Value?.ToString() ?? " ");
                 }
 
                 jsonWriter.WriteEndObject();

src/Agent/NewRelic/Agent/Core/WireModels/LoadedModuleWireModelCollection.cs

@@ -82,12 +82,7 @@ private static bool TryGetAssemblyName(Assembly assembly, out string assemblyNam
                     assemblyName = Path.GetFileName(assembly.Location);
                 }
 
-                if (string.IsNullOrWhiteSpace(assemblyName))
-                {
-                    return false;
-                }
-
-                return true;
+                return !string.IsNullOrWhiteSpace(assemblyName);
             }
             catch
             {
@@ -101,12 +96,7 @@ private static bool TryGetPublicKeyToken(AssemblyName assemblyDetails, out strin
             try
             {
                 publicKey = BitConverter.ToString(assemblyDetails.GetPublicKeyToken()).Replace("-", "");
-                if (string.IsNullOrWhiteSpace(publicKey))
-                {
-                    return false;
-                }
-
-                return true;
+                return !string.IsNullOrWhiteSpace(publicKey);
             }
             catch
             {
@@ -154,7 +144,7 @@ private static bool TryGetShaFileHashes(Assembly assembly, out string sha1FileHa
                     sha512.Dispose();
                 }
 
-                return true;
+                return !string.IsNullOrWhiteSpace(sha1FileHash) && !string.IsNullOrWhiteSpace(sha512FileHash);
             }
             catch
             {
@@ -169,7 +159,7 @@ private static bool TryGetAssemblyHashCode(Assembly assembly, out string assembl
             try
             {
                 assemblyHashCode = assembly.GetHashCode().ToString();
-                return true;
+                return !string.IsNullOrWhiteSpace(assemblyHashCode);
             }
             catch
             {
@@ -190,7 +180,7 @@ private static bool TryGetCompanyName(Assembly assembly, out string companyName)
                 }
 
                 companyName = ((AssemblyCompanyAttribute)attributes[0]).Company;
-                return true;
+                return !string.IsNullOrWhiteSpace(companyName);
             }
             catch
             {
@@ -211,7 +201,7 @@ private static bool TryGetCopyright(Assembly assembly, out string copyright)
                 }
 
                 copyright = ((AssemblyCopyrightAttribute)attributes[0]).Copyright;
-                return true;
+                return !string.IsNullOrWhiteSpace(copyright);
             }
             catch
             {

tests/Agent/UnitTests/Core.UnitTest/JsonConverters/LoadedModuleWireModelCollectionJsonConverterTests.cs

@@ -48,5 +48,30 @@ public void LoadedModuleWireModelCollectionIsJsonSerializable()
             var serialized = JsonConvert.SerializeObject(new[] { loadedModules }, Formatting.None);
             Assert.AreEqual(expected, serialized);
         }
+
+        [Test]
+        public void LoadedModuleWireModelCollectionHandlesNulls()
+        {
+            var expected = @"[""Jars"",[[""MyTestAssembly"",""1.0.0"",{""namespace"":""MyTestAssembly"",""publicKeyToken"":""7075626C69636B6579746F6B656E"",""assemblyHashCode"":""42""}]]]";
+
+            var baseAssemblyName = new AssemblyName();
+            baseAssemblyName.Name = BaseAssemblyName;
+            baseAssemblyName.Version = new Version(BaseAssemblyVersion);
+            baseAssemblyName.SetPublicKeyToken(Encoding.ASCII.GetBytes(BasePublicKeyToken));
+
+            var baseTestAssembly = new TestAssembly();
+            baseTestAssembly.SetAssemblyName = baseAssemblyName;
+            baseTestAssembly.SetDynamic = true; // false uses on disk assembly and this won't have one.
+            baseTestAssembly.SetHashCode = BaseHashCode;
+            baseTestAssembly.AddCustomAttribute(new AssemblyCompanyAttribute(null));
+            baseTestAssembly.AddCustomAttribute(new AssemblyCopyrightAttribute(null));
+
+            var assemblies = new List<Assembly> { baseTestAssembly };
+
+            var loadedModules = LoadedModuleWireModelCollection.Build(assemblies);
+
+            var serialized = JsonConvert.SerializeObject(new[] { loadedModules }, Formatting.None);
+            Assert.AreEqual(expected, serialized);
+        }
     }
 }

tests/Agent/UnitTests/Core.UnitTest/WireModels/LoadedModuleWireModelCollectionTests.cs

@@ -1,16 +1,11 @@
-// Copyright 2020 New Relic, Inc. All rights reserved.
+// Copyright 2020 New Relic, Inc. All rights reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 using System;
 using System.Collections.Generic;
-using System.Linq;
 using System.Reflection;
-using System.Runtime.InteropServices;
 using System.Text;
-using NewRelic.Agent.Core.Metrics;
-using NewRelic.Collections;
 using NUnit.Framework;
-using Telerik.JustMock;
 using static Google.Protobuf.Reflection.SourceCodeInfo.Types;
 
 namespace NewRelic.Agent.Core.WireModels
@@ -19,20 +14,30 @@ namespace NewRelic.Agent.Core.WireModels
     public class LoadedModuleWireModelCollectionTests
     {
         private const string BaseAssemblyName = "MyTestAssembly";
-        private const string BaseAssemblyVersion = "1.0.0";
-        private const string BaseAssemblyPath = @"C:\path\to\assembly\MyTestAssembly.dll";
-        private const string BaseCompanyName = "MyCompany";
-        private const string BaseCopyrightValue = "Copyright 2008";
-        private const int BaseHashCode = 42;
-        private const string BasePublicKeyToken = "publickeytoken";
-        private const string BasePublicKey = "7075626C69636B6579746F6B656E";
+
+        private string BaseAssemblyVersion;
+        private string BaseAssemblyPath;
+        private string BaseCompanyName;
+        private string BaseCopyrightValue;
+        private int    BaseHashCode;
+        private string BasePublicKeyToken;
+        private string BasePublicKey;
 
         private AssemblyName _baseAssemblyName;
         private TestAssembly _baseTestAssembly;
 
         [SetUp]
         public void SetUp()
         {
+
+            BaseAssemblyVersion = "1.0.0";
+            BaseAssemblyPath = @"C:\path\to\assembly\MyTestAssembly.dll";
+            BaseCompanyName = "MyCompany";
+            BaseCopyrightValue = "Copyright 2008";
+            BaseHashCode = 42;
+            BasePublicKeyToken = "publickeytoken";
+            BasePublicKey = "7075626C69636B6579746F6B656E";
+
             _baseAssemblyName = new AssemblyName();
             _baseAssemblyName.Name = BaseAssemblyName;
             _baseAssemblyName.Version = new Version(BaseAssemblyVersion);
@@ -64,7 +69,6 @@ public int TryGetAssemblyName_UsingCollectionCount(string assemblyName, bool isD
             {
                 _baseTestAssembly.SetLocation = null;
             }
-
             _baseTestAssembly.SetAssemblyName = _baseAssemblyName;
             _baseTestAssembly.SetDynamic = isDynamic;
 
@@ -194,6 +198,35 @@ public void ErrorsHandled_GetCustomAttributes()
             Assert.False(loadedModule.Data.ContainsKey("sha512Checksum"));
         }
 
+        [Test]
+        public void ErrorsHandled_GetCustomAttributes_HandlesNulls()
+        {
+            _baseTestAssembly = new TestAssembly();
+            _baseTestAssembly.SetAssemblyName = _baseAssemblyName;
+            _baseTestAssembly.SetDynamic = true; // false uses on disk assembly and this won't have one.
+            _baseTestAssembly.SetHashCode = BaseHashCode;
+
+
+            _baseTestAssembly.AddCustomAttribute(new AssemblyCompanyAttribute(null));
+            _baseTestAssembly.AddCustomAttribute(new AssemblyCopyrightAttribute(null));
+
+
+            var evilAssembly = new EvilTestAssembly(_baseTestAssembly);
+            evilAssembly.ItemToTest = "";
+
+            var assemblies = new List<Assembly>();
+            assemblies.Add(evilAssembly);
+
+            var loadedModules = LoadedModuleWireModelCollection.Build(assemblies);
+
+            Assert.AreEqual(1, loadedModules.LoadedModules.Count);
+
+            var loadedModule = loadedModules.LoadedModules[0];
+
+            Assert.False(loadedModule.Data.ContainsKey("Implementation-Vendor"));
+            Assert.False(loadedModule.Data.ContainsKey("copyright"));
+        }
+
         [Test]
         public void ErrorsHandled_PublickeyToken()
         {
@@ -266,7 +299,6 @@ public string SetLocation
         }
 
         public override string Location => _location;
-
         public void AddCustomAttribute(object attribute)
         {
             _customAttributes.Add(attribute);