Skip to content

Commit 9db38aa

Browse files
andrewlockandrewlock
authored
Fix NullReferenceException in HttpClientResponse.GetCharsetEncoding (#5881 -> v2) (#5887)
## Summary of changes Fixes a `NullReferenceException` that occurred on some responses in .NET 6+ ## Reason for change We were getting null reference exceptions in some cases ## Implementation details Fix the NRE, and add some more `#nullable enable` ## Test coverage Should probably have some :awkward-monkey:
1 parent 8b611d9 commit 9db38aa

File tree

6 files changed

+28
-24
lines changed

6 files changed

+28
-24
lines changed

‎tracer/missing-nullability-files.csv‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ src/Datadog.Trace/Agent/IAgentWriter.cs
5050
src/Datadog.Trace/Agent/IApi.cs
5151
src/Datadog.Trace/Agent/IApiRequest.cs
5252
src/Datadog.Trace/Agent/IApiRequestFactory.cs
53-
src/Datadog.Trace/Agent/IApiResponse.cs
5453
src/Datadog.Trace/Agent/IApiResponseTelemetryExtensions.cs
5554
src/Datadog.Trace/Agent/IKeepRateCalculator.cs
5655
src/Datadog.Trace/Agent/IStatsAggregator.cs
@@ -272,13 +271,10 @@ src/Datadog.Trace/Agent/TraceSamplers/PrioritySampler.cs
272271
src/Datadog.Trace/Agent/TraceSamplers/RareSampler.cs
273272
src/Datadog.Trace/Agent/Transports/ApiWebRequest.cs
274273
src/Datadog.Trace/Agent/Transports/ApiWebRequestFactory.cs
275-
src/Datadog.Trace/Agent/Transports/ApiWebResponse.cs
276274
src/Datadog.Trace/Agent/Transports/HttpClientRequest.cs
277275
src/Datadog.Trace/Agent/Transports/HttpClientRequestFactory.cs
278-
src/Datadog.Trace/Agent/Transports/HttpClientResponse.cs
279276
src/Datadog.Trace/Agent/Transports/HttpStreamRequest.cs
280277
src/Datadog.Trace/Agent/Transports/HttpStreamRequestFactory.cs
281-
src/Datadog.Trace/Agent/Transports/HttpStreamResponse.cs
282278
src/Datadog.Trace/Agent/Transports/MimeTypes.cs
283279
src/Datadog.Trace/Agent/Transports/SocketHandlerRequestFactory.cs
284280
src/Datadog.Trace/AppSec/Concurrency/ReaderWriterLock.Core.cs

‎tracer/src/Datadog.Trace/Agent/IApiResponse.cs‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
44
// </copyright>
55

6+
#nullable enable
7+
68
using System;
79
using System.Diagnostics.CodeAnalysis;
810
using System.IO;
@@ -23,14 +25,14 @@ internal interface IApiResponse : IDisposable
2325
/// <summary>
2426
/// Gets the "raw" content-type header, which may contain additional information like charset or boundary.
2527
/// </summary>
26-
string ContentTypeHeader { get; }
28+
string? ContentTypeHeader { get; }
2729

2830
/// <summary>
2931
/// Gets the "raw" content-encoding header, which may contain multiple values
3032
/// </summary>
31-
string ContentEncodingHeader { get; }
33+
string? ContentEncodingHeader { get; }
3234

33-
string GetHeader(string headerName);
35+
string? GetHeader(string headerName);
3436

3537
Encoding GetCharsetEncoding();
3638

@@ -50,9 +52,9 @@ public static async Task<string> ReadAsStringAsync(this IApiResponse apiResponse
5052
return await reader.ReadToEndAsync().ConfigureAwait(false);
5153
}
5254

53-
public static async Task<T> ReadAsTypeAsync<T>(this IApiResponse apiResponse)
55+
public static async Task<T?> ReadAsTypeAsync<T>(this IApiResponse apiResponse)
5456
{
55-
InitiallyBufferedStream bufferedStream = null;
57+
InitiallyBufferedStream? bufferedStream = null;
5658
try
5759
{
5860
var stream = await apiResponse.GetStreamAsync().ConfigureAwait(false);
@@ -103,7 +105,7 @@ public static bool ShouldRetry(this IApiResponse response)
103105
/// <param name="contentTypeHeader">The raw content-type header, for example <c>"application/json;charset=utf-8"</c></param>
104106
/// <returns>The encoding associated with the charset, or <see cref="EncodingHelpers.Utf8NoBom"/> if the content-type header was not provided,
105107
/// if the charset was not provided, or if the charset was not recognized</returns>
106-
public static Encoding GetCharsetEncoding(string contentTypeHeader)
108+
public static Encoding GetCharsetEncoding(string? contentTypeHeader)
107109
{
108110
// special casing application/json because it's so common
109111
if (string.IsNullOrEmpty(contentTypeHeader)
@@ -114,7 +116,7 @@ public static Encoding GetCharsetEncoding(string contentTypeHeader)
114116
}
115117

116118
// text/plain; charset=utf-8; boundary=foo
117-
foreach (var pair in contentTypeHeader.SplitIntoSpans(';'))
119+
foreach (var pair in contentTypeHeader!.SplitIntoSpans(';'))
118120
{
119121
var parts = pair.AsSpan();
120122
var index = parts.IndexOf('=');
@@ -143,14 +145,14 @@ public static Encoding GetCharsetEncoding(string contentTypeHeader)
143145
return EncodingHelpers.Utf8NoBom;
144146
}
145147

146-
public static ContentEncodingType GetContentEncodingType(string contentEncodingHeader)
148+
public static ContentEncodingType GetContentEncodingType(string? contentEncodingHeader)
147149
{
148150
if (string.IsNullOrEmpty(contentEncodingHeader))
149151
{
150152
return ContentEncodingType.None;
151153
}
152154

153-
if (contentEncodingHeader.Contains(","))
155+
if (contentEncodingHeader!.Contains(","))
154156
{
155157
return ContentEncodingType.Multiple;
156158
}

‎tracer/src/Datadog.Trace/Agent/Transports/ApiWebResponse.cs‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
44
// </copyright>
55

6+
#nullable enable
7+
68
using System;
79
using System.IO;
810
using System.Net;
@@ -24,11 +26,11 @@ public ApiWebResponse(HttpWebResponse response)
2426

2527
public long ContentLength => _response.ContentLength;
2628

27-
public string ContentTypeHeader => _response.ContentType;
29+
public string? ContentTypeHeader => _response.ContentType;
2830

29-
public string ContentEncodingHeader => _response.ContentEncoding;
31+
public string? ContentEncodingHeader => _response.ContentEncoding;
3032

31-
public string GetHeader(string headerName) => _response.Headers[headerName];
33+
public string? GetHeader(string headerName) => _response.Headers[headerName];
3234

3335
public Encoding GetCharsetEncoding() => ApiResponseExtensions.GetCharsetEncoding(ContentTypeHeader);
3436

‎tracer/src/Datadog.Trace/Agent/Transports/HttpClientResponse.cs‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
44
// </copyright>
55

6+
#nullable enable
7+
68
#if NETCOREAPP
79
using System.IO;
810
using System.Linq;
@@ -26,9 +28,9 @@ public HttpClientResponse(HttpResponseMessage response)
2628

2729
public long ContentLength => _response.Content.Headers.ContentLength ?? -1;
2830

29-
public string ContentEncodingHeader => string.Join(',', _response.Content.Headers.ContentEncoding);
31+
public string? ContentEncodingHeader => string.Join(',', _response.Content.Headers.ContentEncoding);
3032

31-
public string ContentTypeHeader => _response.Content.Headers.ContentType?.ToString();
33+
public string? ContentTypeHeader => _response.Content.Headers.ContentType?.ToString();
3234

3335
public ContentEncodingType GetContentEncodingType() =>
3436
_response.Content.Headers.ContentEncoding.Count switch
@@ -40,7 +42,7 @@ public ContentEncodingType GetContentEncodingType() =>
4042

4143
public Encoding GetCharsetEncoding()
4244
{
43-
var charset = _response.Content.Headers.ContentType.CharSet;
45+
var charset = _response.Content.Headers.ContentType?.CharSet;
4446
if (string.IsNullOrEmpty(charset))
4547
{
4648
return EncodingHelpers.Utf8NoBom;
@@ -61,7 +63,7 @@ public void Dispose()
6163
_response.Dispose();
6264
}
6365

64-
public string GetHeader(string headerName)
66+
public string? GetHeader(string headerName)
6567
{
6668
if (_response.Headers.TryGetValues(headerName, out var headers))
6769
{

‎tracer/src/Datadog.Trace/Agent/Transports/HttpStreamResponse.cs‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
44
// </copyright>
55

6+
#nullable enable
7+
68
using System.IO;
79
using System.Text;
810
using System.Threading.Tasks;
@@ -28,17 +30,17 @@ public HttpStreamResponse(int statusCode, long contentLength, Encoding encoding,
2830

2931
public long ContentLength { get; }
3032

31-
public string ContentTypeHeader => _headers.GetValue("Content-Type");
33+
public string? ContentTypeHeader => _headers.GetValue("Content-Type");
3234

33-
public string ContentEncodingHeader => _headers.GetValue("Content-Encoding");
35+
public string? ContentEncodingHeader => _headers.GetValue("Content-Encoding");
3436

3537
public Stream ResponseStream { get; }
3638

3739
public void Dispose()
3840
{
3941
}
4042

41-
public string GetHeader(string headerName) => _headers.GetValue(headerName);
43+
public string? GetHeader(string headerName) => _headers.GetValue(headerName);
4244

4345
public Encoding GetCharsetEncoding() => _encoding;
4446

‎tracer/src/Datadog.Trace/Util/EncodingHelpers.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@
66
#nullable enable
77

88
using System;
9+
using System.Diagnostics.CodeAnalysis;
910
using System.Text;
1011
using Datadog.Trace.Logging;
11-
using Datadog.Trace.VendoredMicrosoftCode.System.Diagnostics.CodeAnalysis;
1212

1313
namespace Datadog.Trace.Util;
1414

0 commit comments

Comments
 (0)