Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix NullReferenceException in HttpClientResponse.GetCharsetEncoding #5881

Merged
merged 1 commit into from
Aug 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions tracer/missing-nullability-files.csv
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,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 @@ -266,13 +265,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
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;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the actual fix

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
Loading