Skip to content

Commit afb6352

Browse files
tippmar-nrtippmar-nr
authored
fix: Prevent null reference exceptions while serializing LoadedModuleWireModelCollection. (#2185) (#2187)
1 parent ba65bd0 commit afb6352

File tree

4 files changed

+79
-32
lines changed

4 files changed

+79
-32
lines changed

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ private static void WriteJsonImpl(JsonWriter jsonWriter, LoadedModuleWireModelCo
4141
foreach (var item in loadedModule.Data)
4242
{
4343
jsonWriter.WritePropertyName(item.Key);
44-
jsonWriter.WriteValue(item.Value.ToString());
44+
jsonWriter.WriteValue(item.Value?.ToString() ?? " ");
4545
}
4646

4747
jsonWriter.WriteEndObject();

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

+6-16
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,7 @@ private static bool TryGetAssemblyName(Assembly assembly, out string assemblyNam
8282
assemblyName = Path.GetFileName(assembly.Location);
8383
}
8484

85-
if (string.IsNullOrWhiteSpace(assemblyName))
86-
{
87-
return false;
88-
}
89-
90-
return true;
85+
return !string.IsNullOrWhiteSpace(assemblyName);
9186
}
9287
catch
9388
{
@@ -101,12 +96,7 @@ private static bool TryGetPublicKeyToken(AssemblyName assemblyDetails, out strin
10196
try
10297
{
10398
publicKey = BitConverter.ToString(assemblyDetails.GetPublicKeyToken()).Replace("-", "");
104-
if (string.IsNullOrWhiteSpace(publicKey))
105-
{
106-
return false;
107-
}
108-
109-
return true;
99+
return !string.IsNullOrWhiteSpace(publicKey);
110100
}
111101
catch
112102
{
@@ -154,7 +144,7 @@ private static bool TryGetShaFileHashes(Assembly assembly, out string sha1FileHa
154144
sha512.Dispose();
155145
}
156146

157-
return true;
147+
return !string.IsNullOrWhiteSpace(sha1FileHash) && !string.IsNullOrWhiteSpace(sha512FileHash);
158148
}
159149
catch
160150
{
@@ -169,7 +159,7 @@ private static bool TryGetAssemblyHashCode(Assembly assembly, out string assembl
169159
try
170160
{
171161
assemblyHashCode = assembly.GetHashCode().ToString();
172-
return true;
162+
return !string.IsNullOrWhiteSpace(assemblyHashCode);
173163
}
174164
catch
175165
{
@@ -190,7 +180,7 @@ private static bool TryGetCompanyName(Assembly assembly, out string companyName)
190180
}
191181

192182
companyName = ((AssemblyCompanyAttribute)attributes[0]).Company;
193-
return true;
183+
return !string.IsNullOrWhiteSpace(companyName);
194184
}
195185
catch
196186
{
@@ -211,7 +201,7 @@ private static bool TryGetCopyright(Assembly assembly, out string copyright)
211201
}
212202

213203
copyright = ((AssemblyCopyrightAttribute)attributes[0]).Copyright;
214-
return true;
204+
return !string.IsNullOrWhiteSpace(copyright);
215205
}
216206
catch
217207
{

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

+25
Original file line numberDiff line numberDiff line change
@@ -48,5 +48,30 @@ public void LoadedModuleWireModelCollectionIsJsonSerializable()
4848
var serialized = JsonConvert.SerializeObject(new[] { loadedModules }, Formatting.None);
4949
Assert.AreEqual(expected, serialized);
5050
}
51+
52+
[Test]
53+
public void LoadedModuleWireModelCollectionHandlesNulls()
54+
{
55+
var expected = @"[""Jars"",[[""MyTestAssembly"",""1.0.0"",{""namespace"":""MyTestAssembly"",""publicKeyToken"":""7075626C69636B6579746F6B656E"",""assemblyHashCode"":""42""}]]]";
56+
57+
var baseAssemblyName = new AssemblyName();
58+
baseAssemblyName.Name = BaseAssemblyName;
59+
baseAssemblyName.Version = new Version(BaseAssemblyVersion);
60+
baseAssemblyName.SetPublicKeyToken(Encoding.ASCII.GetBytes(BasePublicKeyToken));
61+
62+
var baseTestAssembly = new TestAssembly();
63+
baseTestAssembly.SetAssemblyName = baseAssemblyName;
64+
baseTestAssembly.SetDynamic = true; // false uses on disk assembly and this won't have one.
65+
baseTestAssembly.SetHashCode = BaseHashCode;
66+
baseTestAssembly.AddCustomAttribute(new AssemblyCompanyAttribute(null));
67+
baseTestAssembly.AddCustomAttribute(new AssemblyCopyrightAttribute(null));
68+
69+
var assemblies = new List<Assembly> { baseTestAssembly };
70+
71+
var loadedModules = LoadedModuleWireModelCollection.Build(assemblies);
72+
73+
var serialized = JsonConvert.SerializeObject(new[] { loadedModules }, Formatting.None);
74+
Assert.AreEqual(expected, serialized);
75+
}
5176
}
5277
}

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

+47-15
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,11 @@
1-
// Copyright 2020 New Relic, Inc. All rights reserved.
1+
// Copyright 2020 New Relic, Inc. All rights reserved.
22
// SPDX-License-Identifier: Apache-2.0
33

44
using System;
55
using System.Collections.Generic;
6-
using System.Linq;
76
using System.Reflection;
8-
using System.Runtime.InteropServices;
97
using System.Text;
10-
using NewRelic.Agent.Core.Metrics;
11-
using NewRelic.Collections;
128
using NUnit.Framework;
13-
using Telerik.JustMock;
149
using static Google.Protobuf.Reflection.SourceCodeInfo.Types;
1510

1611
namespace NewRelic.Agent.Core.WireModels
@@ -19,20 +14,30 @@ namespace NewRelic.Agent.Core.WireModels
1914
public class LoadedModuleWireModelCollectionTests
2015
{
2116
private const string BaseAssemblyName = "MyTestAssembly";
22-
private const string BaseAssemblyVersion = "1.0.0";
23-
private const string BaseAssemblyPath = @"C:\path\to\assembly\MyTestAssembly.dll";
24-
private const string BaseCompanyName = "MyCompany";
25-
private const string BaseCopyrightValue = "Copyright 2008";
26-
private const int BaseHashCode = 42;
27-
private const string BasePublicKeyToken = "publickeytoken";
28-
private const string BasePublicKey = "7075626C69636B6579746F6B656E";
17+
18+
private string BaseAssemblyVersion;
19+
private string BaseAssemblyPath;
20+
private string BaseCompanyName;
21+
private string BaseCopyrightValue;
22+
private int BaseHashCode;
23+
private string BasePublicKeyToken;
24+
private string BasePublicKey;
2925

3026
private AssemblyName _baseAssemblyName;
3127
private TestAssembly _baseTestAssembly;
3228

3329
[SetUp]
3430
public void SetUp()
3531
{
32+
33+
BaseAssemblyVersion = "1.0.0";
34+
BaseAssemblyPath = @"C:\path\to\assembly\MyTestAssembly.dll";
35+
BaseCompanyName = "MyCompany";
36+
BaseCopyrightValue = "Copyright 2008";
37+
BaseHashCode = 42;
38+
BasePublicKeyToken = "publickeytoken";
39+
BasePublicKey = "7075626C69636B6579746F6B656E";
40+
3641
_baseAssemblyName = new AssemblyName();
3742
_baseAssemblyName.Name = BaseAssemblyName;
3843
_baseAssemblyName.Version = new Version(BaseAssemblyVersion);
@@ -64,7 +69,6 @@ public int TryGetAssemblyName_UsingCollectionCount(string assemblyName, bool isD
6469
{
6570
_baseTestAssembly.SetLocation = null;
6671
}
67-
6872
_baseTestAssembly.SetAssemblyName = _baseAssemblyName;
6973
_baseTestAssembly.SetDynamic = isDynamic;
7074

@@ -194,6 +198,35 @@ public void ErrorsHandled_GetCustomAttributes()
194198
Assert.False(loadedModule.Data.ContainsKey("sha512Checksum"));
195199
}
196200

201+
[Test]
202+
public void ErrorsHandled_GetCustomAttributes_HandlesNulls()
203+
{
204+
_baseTestAssembly = new TestAssembly();
205+
_baseTestAssembly.SetAssemblyName = _baseAssemblyName;
206+
_baseTestAssembly.SetDynamic = true; // false uses on disk assembly and this won't have one.
207+
_baseTestAssembly.SetHashCode = BaseHashCode;
208+
209+
210+
_baseTestAssembly.AddCustomAttribute(new AssemblyCompanyAttribute(null));
211+
_baseTestAssembly.AddCustomAttribute(new AssemblyCopyrightAttribute(null));
212+
213+
214+
var evilAssembly = new EvilTestAssembly(_baseTestAssembly);
215+
evilAssembly.ItemToTest = "";
216+
217+
var assemblies = new List<Assembly>();
218+
assemblies.Add(evilAssembly);
219+
220+
var loadedModules = LoadedModuleWireModelCollection.Build(assemblies);
221+
222+
Assert.AreEqual(1, loadedModules.LoadedModules.Count);
223+
224+
var loadedModule = loadedModules.LoadedModules[0];
225+
226+
Assert.False(loadedModule.Data.ContainsKey("Implementation-Vendor"));
227+
Assert.False(loadedModule.Data.ContainsKey("copyright"));
228+
}
229+
197230
[Test]
198231
public void ErrorsHandled_PublickeyToken()
199232
{
@@ -266,7 +299,6 @@ public string SetLocation
266299
}
267300

268301
public override string Location => _location;
269-
270302
public void AddCustomAttribute(object attribute)
271303
{
272304
_customAttributes.Add(attribute);

0 commit comments

Comments
 (0)