Skip to content

Commit

Permalink
Fix NullReferenceException in `HttpClientResponse.GetCharsetEncodin…
Browse files Browse the repository at this point in the history
…g` (#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:
  • Loading branch information
andrewlock committed Aug 13, 2024
1 parent 3fb67b0 commit 203dcb2
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 24 deletions.
4 changes: 0 additions & 4 deletions tracer/missing-nullability-files.csv
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
20 changes: 11 additions & 9 deletions tracer/src/Datadog.Trace/Agent/IApiResponse.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
// </copyright>

#nullable enable

using System;
using System.Diagnostics.CodeAnalysis;
using System.IO;
Expand All @@ -23,14 +25,14 @@ internal interface IApiResponse : IDisposable
/// <summary>
/// Gets the "raw" content-type header, which may contain additional information like charset or boundary.
/// </summary>
string ContentTypeHeader { get; }
string? ContentTypeHeader { get; }

/// <summary>
/// Gets the "raw" content-encoding header, which may contain multiple values
/// </summary>
string ContentEncodingHeader { get; }
string? ContentEncodingHeader { get; }

string GetHeader(string headerName);
string? GetHeader(string headerName);

Encoding GetCharsetEncoding();

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

public static async Task<T> ReadAsTypeAsync<T>(this IApiResponse apiResponse)
public static async Task<T?> ReadAsTypeAsync<T>(this IApiResponse apiResponse)
{
InitiallyBufferedStream bufferedStream = null;
InitiallyBufferedStream? bufferedStream = null;
try
{
var stream = await apiResponse.GetStreamAsync().ConfigureAwait(false);
Expand Down Expand Up @@ -103,7 +105,7 @@ public static bool ShouldRetry(this IApiResponse response)
/// <param name="contentTypeHeader">The raw content-type header, for example <c>"application/json;charset=utf-8"</c></param>
/// <returns>The encoding associated with the charset, or <see cref="EncodingHelpers.Utf8NoBom"/> if the content-type header was not provided,
/// if the charset was not provided, or if the charset was not recognized</returns>
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)
Expand All @@ -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('=');
Expand Down Expand Up @@ -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;
}
Expand Down
8 changes: 5 additions & 3 deletions tracer/src/Datadog.Trace/Agent/Transports/ApiWebResponse.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
// </copyright>

#nullable enable

using System;
using System.IO;
using System.Net;
Expand All @@ -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);

Expand Down
10 changes: 6 additions & 4 deletions tracer/src/Datadog.Trace/Agent/Transports/HttpClientResponse.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
// </copyright>

#nullable enable

#if NETCOREAPP
using System.IO;
using System.Linq;
Expand All @@ -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
Expand All @@ -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;
Expand All @@ -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))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
// </copyright>

#nullable enable

using System.IO;
using System.Text;
using System.Threading.Tasks;
Expand All @@ -28,17 +30,17 @@ 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; }

public void Dispose()
{
}

public string GetHeader(string headerName) => _headers.GetValue(headerName);
public string? GetHeader(string headerName) => _headers.GetValue(headerName);

public Encoding GetCharsetEncoding() => _encoding;

Expand Down
2 changes: 1 addition & 1 deletion tracer/src/Datadog.Trace/Util/EncodingHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down

0 comments on commit 203dcb2

Please sign in to comment.