From 203dcb2034624d3c1f4179911ac5a6f18667f629 Mon Sep 17 00:00:00 2001 From: Andrew Lock Date: Tue, 13 Aug 2024 12:10:39 +0100 Subject: [PATCH] Fix `NullReferenceException` in `HttpClientResponse.GetCharsetEncoding` (#5881) ## 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: --- tracer/missing-nullability-files.csv | 4 ---- .../src/Datadog.Trace/Agent/IApiResponse.cs | 20 ++++++++++--------- .../Agent/Transports/ApiWebResponse.cs | 8 +++++--- .../Agent/Transports/HttpClientResponse.cs | 10 ++++++---- .../Agent/Transports/HttpStreamResponse.cs | 8 +++++--- .../src/Datadog.Trace/Util/EncodingHelpers.cs | 2 +- 6 files changed, 28 insertions(+), 24 deletions(-) diff --git a/tracer/missing-nullability-files.csv b/tracer/missing-nullability-files.csv index c2d90e8f074f..0687edf14a6d 100644 --- a/tracer/missing-nullability-files.csv +++ b/tracer/missing-nullability-files.csv @@ -50,7 +50,6 @@ src/Datadog.Trace/Agent/IAgentWriter.cs src/Datadog.Trace/Agent/IApi.cs src/Datadog.Trace/Agent/IApiRequest.cs src/Datadog.Trace/Agent/IApiRequestFactory.cs -src/Datadog.Trace/Agent/IApiResponse.cs src/Datadog.Trace/Agent/IApiResponseTelemetryExtensions.cs src/Datadog.Trace/Agent/IKeepRateCalculator.cs src/Datadog.Trace/Agent/IStatsAggregator.cs @@ -272,13 +271,10 @@ src/Datadog.Trace/Agent/TraceSamplers/PrioritySampler.cs src/Datadog.Trace/Agent/TraceSamplers/RareSampler.cs src/Datadog.Trace/Agent/Transports/ApiWebRequest.cs src/Datadog.Trace/Agent/Transports/ApiWebRequestFactory.cs -src/Datadog.Trace/Agent/Transports/ApiWebResponse.cs src/Datadog.Trace/Agent/Transports/HttpClientRequest.cs src/Datadog.Trace/Agent/Transports/HttpClientRequestFactory.cs -src/Datadog.Trace/Agent/Transports/HttpClientResponse.cs src/Datadog.Trace/Agent/Transports/HttpStreamRequest.cs src/Datadog.Trace/Agent/Transports/HttpStreamRequestFactory.cs -src/Datadog.Trace/Agent/Transports/HttpStreamResponse.cs src/Datadog.Trace/Agent/Transports/MimeTypes.cs src/Datadog.Trace/Agent/Transports/SocketHandlerRequestFactory.cs src/Datadog.Trace/AppSec/Concurrency/ReaderWriterLock.Core.cs diff --git a/tracer/src/Datadog.Trace/Agent/IApiResponse.cs b/tracer/src/Datadog.Trace/Agent/IApiResponse.cs index 425c9a42944e..cc570ab2847c 100644 --- a/tracer/src/Datadog.Trace/Agent/IApiResponse.cs +++ b/tracer/src/Datadog.Trace/Agent/IApiResponse.cs @@ -3,6 +3,8 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. // +#nullable enable + using System; using System.Diagnostics.CodeAnalysis; using System.IO; @@ -23,14 +25,14 @@ internal interface IApiResponse : IDisposable /// /// Gets the "raw" content-type header, which may contain additional information like charset or boundary. /// - string ContentTypeHeader { get; } + string? ContentTypeHeader { get; } /// /// Gets the "raw" content-encoding header, which may contain multiple values /// - string ContentEncodingHeader { get; } + string? ContentEncodingHeader { get; } - string GetHeader(string headerName); + string? GetHeader(string headerName); Encoding GetCharsetEncoding(); @@ -50,9 +52,9 @@ public static async Task ReadAsStringAsync(this IApiResponse apiResponse return await reader.ReadToEndAsync().ConfigureAwait(false); } - public static async Task ReadAsTypeAsync(this IApiResponse apiResponse) + public static async Task ReadAsTypeAsync(this IApiResponse apiResponse) { - InitiallyBufferedStream bufferedStream = null; + InitiallyBufferedStream? bufferedStream = null; try { var stream = await apiResponse.GetStreamAsync().ConfigureAwait(false); @@ -103,7 +105,7 @@ public static bool ShouldRetry(this IApiResponse response) /// The raw content-type header, for example "application/json;charset=utf-8" /// The encoding associated with the charset, or if the content-type header was not provided, /// if the charset was not provided, or if the charset was not recognized - public static Encoding GetCharsetEncoding(string contentTypeHeader) + public static Encoding GetCharsetEncoding(string? contentTypeHeader) { // special casing application/json because it's so common if (string.IsNullOrEmpty(contentTypeHeader) @@ -114,7 +116,7 @@ public static Encoding GetCharsetEncoding(string contentTypeHeader) } // text/plain; charset=utf-8; boundary=foo - foreach (var pair in contentTypeHeader.SplitIntoSpans(';')) + foreach (var pair in contentTypeHeader!.SplitIntoSpans(';')) { var parts = pair.AsSpan(); var index = parts.IndexOf('='); @@ -143,14 +145,14 @@ public static Encoding GetCharsetEncoding(string contentTypeHeader) return EncodingHelpers.Utf8NoBom; } - public static ContentEncodingType GetContentEncodingType(string contentEncodingHeader) + public static ContentEncodingType GetContentEncodingType(string? contentEncodingHeader) { if (string.IsNullOrEmpty(contentEncodingHeader)) { return ContentEncodingType.None; } - if (contentEncodingHeader.Contains(",")) + if (contentEncodingHeader!.Contains(",")) { return ContentEncodingType.Multiple; } diff --git a/tracer/src/Datadog.Trace/Agent/Transports/ApiWebResponse.cs b/tracer/src/Datadog.Trace/Agent/Transports/ApiWebResponse.cs index 7beace55e4d4..ced78e83c14e 100644 --- a/tracer/src/Datadog.Trace/Agent/Transports/ApiWebResponse.cs +++ b/tracer/src/Datadog.Trace/Agent/Transports/ApiWebResponse.cs @@ -3,6 +3,8 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. // +#nullable enable + using System; using System.IO; using System.Net; @@ -24,11 +26,11 @@ public ApiWebResponse(HttpWebResponse response) public long ContentLength => _response.ContentLength; - public string ContentTypeHeader => _response.ContentType; + public string? ContentTypeHeader => _response.ContentType; - public string ContentEncodingHeader => _response.ContentEncoding; + public string? ContentEncodingHeader => _response.ContentEncoding; - public string GetHeader(string headerName) => _response.Headers[headerName]; + public string? GetHeader(string headerName) => _response.Headers[headerName]; public Encoding GetCharsetEncoding() => ApiResponseExtensions.GetCharsetEncoding(ContentTypeHeader); diff --git a/tracer/src/Datadog.Trace/Agent/Transports/HttpClientResponse.cs b/tracer/src/Datadog.Trace/Agent/Transports/HttpClientResponse.cs index b5c64355dbd8..b47f4655c340 100644 --- a/tracer/src/Datadog.Trace/Agent/Transports/HttpClientResponse.cs +++ b/tracer/src/Datadog.Trace/Agent/Transports/HttpClientResponse.cs @@ -3,6 +3,8 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. // +#nullable enable + #if NETCOREAPP using System.IO; using System.Linq; @@ -26,9 +28,9 @@ public HttpClientResponse(HttpResponseMessage response) public long ContentLength => _response.Content.Headers.ContentLength ?? -1; - public string ContentEncodingHeader => string.Join(',', _response.Content.Headers.ContentEncoding); + public string? ContentEncodingHeader => string.Join(',', _response.Content.Headers.ContentEncoding); - public string ContentTypeHeader => _response.Content.Headers.ContentType?.ToString(); + public string? ContentTypeHeader => _response.Content.Headers.ContentType?.ToString(); public ContentEncodingType GetContentEncodingType() => _response.Content.Headers.ContentEncoding.Count switch @@ -40,7 +42,7 @@ public ContentEncodingType GetContentEncodingType() => public Encoding GetCharsetEncoding() { - var charset = _response.Content.Headers.ContentType.CharSet; + var charset = _response.Content.Headers.ContentType?.CharSet; if (string.IsNullOrEmpty(charset)) { return EncodingHelpers.Utf8NoBom; @@ -61,7 +63,7 @@ public void Dispose() _response.Dispose(); } - public string GetHeader(string headerName) + public string? GetHeader(string headerName) { if (_response.Headers.TryGetValues(headerName, out var headers)) { diff --git a/tracer/src/Datadog.Trace/Agent/Transports/HttpStreamResponse.cs b/tracer/src/Datadog.Trace/Agent/Transports/HttpStreamResponse.cs index 5bfc7fd57701..466aa50dde4d 100644 --- a/tracer/src/Datadog.Trace/Agent/Transports/HttpStreamResponse.cs +++ b/tracer/src/Datadog.Trace/Agent/Transports/HttpStreamResponse.cs @@ -3,6 +3,8 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. // +#nullable enable + using System.IO; using System.Text; using System.Threading.Tasks; @@ -28,9 +30,9 @@ public HttpStreamResponse(int statusCode, long contentLength, Encoding encoding, public long ContentLength { get; } - public string ContentTypeHeader => _headers.GetValue("Content-Type"); + public string? ContentTypeHeader => _headers.GetValue("Content-Type"); - public string ContentEncodingHeader => _headers.GetValue("Content-Encoding"); + public string? ContentEncodingHeader => _headers.GetValue("Content-Encoding"); public Stream ResponseStream { get; } @@ -38,7 +40,7 @@ public void Dispose() { } - public string GetHeader(string headerName) => _headers.GetValue(headerName); + public string? GetHeader(string headerName) => _headers.GetValue(headerName); public Encoding GetCharsetEncoding() => _encoding; diff --git a/tracer/src/Datadog.Trace/Util/EncodingHelpers.cs b/tracer/src/Datadog.Trace/Util/EncodingHelpers.cs index 1cab50500761..2a9f2b58d1a7 100644 --- a/tracer/src/Datadog.Trace/Util/EncodingHelpers.cs +++ b/tracer/src/Datadog.Trace/Util/EncodingHelpers.cs @@ -6,9 +6,9 @@ #nullable enable using System; +using System.Diagnostics.CodeAnalysis; using System.Text; using Datadog.Trace.Logging; -using Datadog.Trace.VendoredMicrosoftCode.System.Diagnostics.CodeAnalysis; namespace Datadog.Trace.Util;