Skip to content

Commit

Permalink
[IAST] Minor cleanup in IAST aspects (#6293)
Browse files Browse the repository at this point in the history
## Summary of changes

- Make all aspects use the `IastModule.Log` object instead of their own
- Use `Error` for cases where we know we're only going to get one log,
and we really wouldn't expect to see it

## Reason for change

Just noticed the differences/issues while working on something else

## Implementation details

- `Log` -> `IastModule.Log`
- `Warning` -> `Error` (in `NewtonsoftJsonAspects` only)

## Test coverage

Covered by existing

## Other details

The aspects are very inconsistent in that sometimes they call `Warning`
and sometimes they call `Error`. We should consider unifying these one
way or the other (or, in some cases where they will be called a _lot_ if
there's an issue, use `Debug` to avoid many logs)
  • Loading branch information
andrewlock authored Nov 15, 2024
1 parent bd8f5d5 commit fa63a0a
Show file tree
Hide file tree
Showing 8 changed files with 15 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ namespace Datadog.Trace.Iast.Aspects.MongoDB;
[global::System.ComponentModel.EditorBrowsable(global::System.ComponentModel.EditorBrowsableState.Never)]
public class MongoDatabaseAspect
{
private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor(typeof(MongoDatabaseAspect));

/// <summary>
/// MongoDB Driver aspect
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ namespace Datadog.Trace.Iast.Aspects.Newtonsoft.Json;
[global::System.ComponentModel.EditorBrowsable(global::System.ComponentModel.EditorBrowsableState.Never)]
public class NewtonsoftJsonAspects
{
private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor<NewtonsoftJsonAspects>();

private static readonly ICanParse? JObjectProxy;
private static readonly ICanParse? JArrayProxy;
private static readonly ICanParse? JTokenProxy;
Expand All @@ -40,7 +38,7 @@ static NewtonsoftJsonAspects()
}
else
{
Log.Warning("Failed to create JObject proxy");
IastModule.Log.Warning("Failed to create JObject proxy");
}

var jArrayProxyResult = DuckType.GetOrCreateProxyType(typeof(ICanParse), jArrayType);
Expand All @@ -50,7 +48,7 @@ static NewtonsoftJsonAspects()
}
else
{
Log.Warning("Failed to create JArray proxy");
IastModule.Log.Warning("Failed to create JArray proxy");
}

var jTokenProxyResult = DuckType.GetOrCreateProxyType(typeof(ICanParse), jTokenType);
Expand All @@ -60,12 +58,12 @@ static NewtonsoftJsonAspects()
}
else
{
Log.Warning("Failed to create JToken proxy");
IastModule.Log.Warning("Failed to create JToken proxy");
}
}
catch (Exception ex)
{
Log.Warning(ex, "Error while initializing NewtonsoftJsonAspects");
IastModule.Log.Error(ex, "Error while initializing NewtonsoftJsonAspects");
}
}

Expand All @@ -87,7 +85,7 @@ static NewtonsoftJsonAspects()
}
catch (Exception ex)
{
Log.Warning(ex, "Error while tainting the JObject");
IastModule.Log.Warning(ex, "Error while tainting the JObject");
}

return result;
Expand Down Expand Up @@ -115,7 +113,7 @@ static NewtonsoftJsonAspects()
}
catch (Exception ex)
{
Log.Warning(ex, "Error while tainting the JArray");
IastModule.Log.Warning(ex, "Error while tainting the JArray");
}

return result;
Expand All @@ -138,7 +136,7 @@ static NewtonsoftJsonAspects()
}
catch (Exception ex)
{
Log.Warning(ex, "Error while tainting the JToken");
IastModule.Log.Warning(ex, "Error while tainting the JToken");
}

return result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ namespace Datadog.Trace.Iast.Aspects;
[global::System.ComponentModel.EditorBrowsable(global::System.ComponentModel.EditorBrowsableState.Never)]
public class HashAlgorithmAspect
{
private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor(typeof(HashAlgorithmAspect));

/// <summary>
/// ComputeHash not static
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ namespace Datadog.Trace.Iast.Aspects;
[global::System.ComponentModel.EditorBrowsable(global::System.ComponentModel.EditorBrowsableState.Never)]
public class SymmetricAlgorithmAspect
{
private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor(typeof(SymmetricAlgorithmAspect));

private static void ProcessCipherClassCreation(SymmetricAlgorithm target)
{
try
Expand All @@ -32,7 +30,7 @@ private static void ProcessCipherClassCreation(SymmetricAlgorithm target)
}
catch (Exception ex)
{
Log.Error(ex, "Error in SymmetricAlgorithmAspect.");
IastModule.Log.Error(ex, "Error in SymmetricAlgorithmAspect.");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
using System.Text.Json;
using Datadog.Trace.DuckTyping;
using Datadog.Trace.Iast.Dataflow;
using Datadog.Trace.Logging;

namespace Datadog.Trace.Iast.Aspects.System.Text.Json;

Expand All @@ -20,8 +19,6 @@ namespace Datadog.Trace.Iast.Aspects.System.Text.Json;
[global::System.ComponentModel.EditorBrowsable(global::System.ComponentModel.EditorBrowsableState.Never)]
public class JsonDocumentAspects
{
private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor<JsonDocumentAspects>();

/// <summary>
/// Parse method aspect
/// Taint all Parent from JsonElement that are string
Expand All @@ -40,7 +37,7 @@ public static object Parse(string json, JsonDocumentOptions options)
}
catch (Exception ex)
{
Log.Warning(ex, "Error tainting JsonDocument.Parse result");
IastModule.Log.Warning(ex, "Error tainting JsonDocument.Parse result");
}

return doc;
Expand All @@ -63,7 +60,7 @@ public static object Parse(string json, JsonDocumentOptions options)
}
catch (Exception ex)
{
Log.Warning(ex, "Error casting to IJsonElement");
IastModule.Log.Warning(ex, "Error casting to IJsonElement");
return null;
}

Expand All @@ -80,7 +77,7 @@ public static object Parse(string json, JsonDocumentOptions options)
}
catch (Exception ex)
{
Log.Warning(ex, "Error tainting JsonElement.GetString result");
IastModule.Log.Warning(ex, "Error tainting JsonElement.GetString result");
}

return str;
Expand All @@ -104,7 +101,7 @@ public static object Parse(string json, JsonDocumentOptions options)
}
catch (Exception ex)
{
Log.Warning(ex, "Error casting to IJsonElement");
IastModule.Log.Warning(ex, "Error casting to IJsonElement");
return null;
}

Expand All @@ -121,7 +118,7 @@ public static object Parse(string json, JsonDocumentOptions options)
}
catch (Exception ex)
{
Log.Warning(ex, "Error tainting JsonElement.GetRawText result");
IastModule.Log.Warning(ex, "Error tainting JsonElement.GetRawText result");
}

return str;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ namespace Datadog.Trace.Iast.Aspects.System.Text;
[global::System.ComponentModel.EditorBrowsable(global::System.ComponentModel.EditorBrowsableState.Never)]
public class StringBuilderAspects
{
private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor(typeof(StringBuilderAspects));

/// <summary> StringBuildr ctor aspect </summary>
/// <param name="value"> Init string </param>
/// <returns> New StringBuilder </returns>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ namespace Datadog.Trace.Iast.Aspects.System.Web.Extensions;
[global::System.ComponentModel.EditorBrowsable(global::System.ComponentModel.EditorBrowsableState.Never)]
public class JavaScriptSerializerAspects
{
private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor<JavaScriptSerializerAspects>();

/// <summary>
/// DeserializeObject aspect
/// </summary>
Expand All @@ -40,7 +38,7 @@ public class JavaScriptSerializerAspects
}
catch (Exception ex)
{
Log.Error(ex, "Error while casting JavaScriptSerializer");
IastModule.Log.Error(ex, "Error while casting JavaScriptSerializer");
return null;
}

Expand All @@ -59,7 +57,7 @@ public class JavaScriptSerializerAspects
}
catch (Exception ex)
{
Log.Warning(ex, "Error while tainting json in DeserializeObject");
IastModule.Log.Warning(ex, "Error while tainting json in DeserializeObject");
}

return result;
Expand Down
2 changes: 0 additions & 2 deletions tracer/src/Datadog.Trace/Iast/Aspects/System/StringAspects.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ namespace Datadog.Trace.Iast.Aspects.System;
[global::System.ComponentModel.EditorBrowsable(global::System.ComponentModel.EditorBrowsableState.Never)]
public class StringAspects
{
private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor(typeof(StringAspects));

/// <summary>
/// String.Trim aspect
/// </summary>
Expand Down

0 comments on commit fa63a0a

Please sign in to comment.