Skip to content

Commit 13aa277

Browse files
niemyjskiCopilot
niemyjski
and
Copilot
committed
fix: address STJ migration bugs and PR review feedback
- Fix ConvertJsonElement ternary type coercion: (object)l cast prevents implicit long→double widening in switch expression - Make ObjectToInferredTypesConverter configurable with preferInt64 flag for ES serializer to match JSON.NET DataObjectConverter behavior - Fix ElasticSystemTextJsonSerializer: remove ReadStreamToSpan lifetime bug (span backed by disposed MemoryStream), deserialize from stream directly with MemoryStream fast-path - Fix Serialize<T> indentation: pass JsonWriterOptions to Utf8JsonWriter so SerializationFormatting.Indented actually produces indented output - Handle exponent notation (1e5) as floating-point in ReadNumber - Use double consistently (not decimal) for floating-point to match JSON.NET behavior - Fix RenameAll return value: return whether any renames occurred - Add using var to MemoryStream in EventController and EventPostsJob - Handle empty response bodies in SendRequestAsAsync (STJ throws on empty input, Newtonsoft returned default) - Fix SerializerTests: put unknown properties at root level to test JsonExtensionData→ConvertJsonElement path correctly - Revert AGENTS.md to main Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent d64cd62 commit 13aa277

File tree

14 files changed

+160
-152
lines changed

14 files changed

+160
-152
lines changed

‎AGENTS.md‎

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -82,26 +82,3 @@ pr-reviewer → security pre-screen (before build!) → dependency audit
8282
- Never commit secrets — use environment variables
8383
- NuGet feeds are in `NuGet.Config` — don't add sources
8484
- Prefer additive documentation updates — don't replace strategic docs wholesale, extend them
85-
86-
## Serialization Architecture
87-
88-
The project uses **System.Text.Json (STJ)** exclusively. NEST still brings in Newtonsoft.Json transitively, but all application-level serialization uses STJ:
89-
90-
| Component | Serializer | Notes |
91-
| -------------- | --------------------------------- | -------------------------------------------- |
92-
| Elasticsearch | `ElasticSystemTextJsonSerializer` | Custom `IElasticsearchSerializer` using STJ |
93-
| Event Upgrader | `System.Text.Json.Nodes` | JsonObject/JsonArray for mutable DOM |
94-
| Data Storage | `SystemTextJsonSerializer` | Via Foundatio's STJ support |
95-
| API | STJ (built-in) | ASP.NET Core default with custom options |
96-
97-
**Key files:**
98-
99-
- `ElasticSystemTextJsonSerializer.cs` - Custom `IElasticsearchSerializer` for NEST
100-
- `JsonNodeExtensions.cs` - STJ equivalents of JObject helpers
101-
- `ObjectToInferredTypesConverter.cs` - Handles JObject/JToken from NEST during STJ serialization
102-
- `V*_EventUpgrade.cs` - Event version upgraders using JsonObject
103-
104-
**Security:**
105-
106-
- Safe JSON encoding used everywhere (escapes `<`, `>`, `&`, `'` for XSS protection)
107-
- No `UnsafeRelaxedJsonEscaping` in the codebase

‎src/Exceptionless.Core/Extensions/DataDictionaryExtensions.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ public static class DataDictionaryExtensions
7878
if (result is not null)
7979
return result;
8080
}
81-
catch
81+
catch (Exception ex) when (ex is JsonException or InvalidOperationException or FormatException)
8282
{
8383
// Ignored - fall through to next handler
8484
}
@@ -94,7 +94,7 @@ public static class DataDictionaryExtensions
9494
if (result is not null)
9595
return result;
9696
}
97-
catch
97+
catch (Exception ex) when (ex is JsonException or InvalidOperationException or FormatException)
9898
{
9999
// Ignored - fall through to next handler
100100
}

‎src/Exceptionless.Core/Extensions/JsonNodeExtensions.cs‎

Lines changed: 3 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
1-
using System.Text.Encodings.Web;
21
using System.Text.Json;
32
using System.Text.Json.Nodes;
4-
using System.Text.Unicode;
53

64
namespace Exceptionless.Core.Extensions;
75

@@ -11,28 +9,6 @@ namespace Exceptionless.Core.Extensions;
119
/// </summary>
1210
public static class JsonNodeExtensions
1311
{
14-
/// <summary>
15-
/// XSS-safe encoder for JSON output formatting.
16-
/// This encoder ensures proper XSS protection while allowing Unicode characters
17-
/// for internationalization support.
18-
///
19-
/// Security features:
20-
/// - HTML-sensitive characters (&lt;, &gt;, &amp;) are escaped for XSS protection
21-
/// - Single quotes are escaped as \u0027 (per ECMAScript spec)
22-
/// - Control characters are escaped for security
23-
/// </summary>
24-
private static readonly JavaScriptEncoder SafeJsonEncoder = JavaScriptEncoder.Create(new TextEncoderSettings(UnicodeRanges.All));
25-
26-
/// <summary>
27-
/// JSON options with safe XSS encoding for tests.
28-
/// Validates that dangerous characters (&lt;, &gt;, &amp;, ') are properly escaped.
29-
/// Production code should use <see cref="JsonSerializerOptions"/> from DI.
30-
/// </summary>
31-
internal static readonly JsonSerializerOptions SafeSerializerOptions = new()
32-
{
33-
Encoder = SafeJsonEncoder
34-
};
35-
3612
/// <summary>
3713
/// Checks if a JsonNode is null or empty (no values for objects/arrays).
3814
/// </summary>
@@ -192,11 +168,9 @@ public static bool RenameOrRemoveIfNullOrEmpty(this JsonObject target, string cu
192168
/// </summary>
193169
public static void MoveOrRemoveIfNullOrEmpty(this JsonObject target, JsonObject source, params string[] names)
194170
{
195-
foreach (string name in names)
171+
foreach (string name in names.Where(source.ContainsKey))
196172
{
197-
if (!source.TryGetPropertyValue(name, out var value))
198-
continue;
199-
173+
source.TryGetPropertyValue(name, out var value);
200174
bool isNullOrEmpty = value.IsNullOrEmpty();
201175
source.Remove(name);
202176

@@ -222,7 +196,7 @@ public static bool RenameAll(this JsonObject target, string currentName, string
222196
obj.Rename(currentName, newName);
223197
}
224198

225-
return true;
199+
return objectsWithProperty.Count > 0;
226200
}
227201

228202
/// <summary>
@@ -293,14 +267,6 @@ public static bool RenameAll(this JsonObject target, string currentName, string
293267
yield return desc;
294268
}
295269

296-
/// <summary>
297-
/// Converts an object to a JsonNode using System.Text.Json serialization.
298-
/// </summary>
299-
public static JsonNode? ToJsonNode<T>(T value, JsonSerializerOptions options)
300-
{
301-
return JsonSerializer.SerializeToNode(value, options);
302-
}
303-
304270
/// <summary>
305271
/// Checks if a JsonNode has any values (for objects: has properties, for arrays: has items).
306272
/// </summary>
@@ -331,14 +297,6 @@ public static bool HasValues(this JsonNode? node)
331297
return array.Deserialize<List<T>>(options);
332298
}
333299

334-
/// <summary>
335-
/// Creates a JsonValue from a primitive value.
336-
/// </summary>
337-
public static JsonValue? CreateValue<T>(T value)
338-
{
339-
return JsonValue.Create(value);
340-
}
341-
342300
/// <summary>
343301
/// Converts a JsonNode to a pretty-printed JSON string.
344302
/// Uses 2-space indentation. Normalizes dates to match existing data format (Z → +00:00).

‎src/Exceptionless.Core/Extensions/ProjectExtensions.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System.Text;
2+
using System.Text.Json;
23
using Exceptionless.Core.Models;
34
using Exceptionless.DateTimeExtensions;
45
using Foundatio.Serializer;
@@ -58,9 +59,9 @@ public static string BuildFilter(this IList<Project> projects)
5859
{
5960
return project.Data.GetValue<SlackToken>(Project.KnownDataKeys.SlackToken, serializer);
6061
}
61-
catch (Exception)
62+
catch (Exception ex) when (ex is JsonException or InvalidOperationException or FormatException)
6263
{
63-
// Ignored
64+
// Ignored — data may be stored in an incompatible format
6465
}
6566

6667
return null;

‎src/Exceptionless.Core/Jobs/EventPostsJob.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ private async Task RetryEventsAsync(List<PersistentEvent> eventsToRetry, EventPo
302302
{
303303
try
304304
{
305-
var stream = new MemoryStream(ev.GetBytes(_serializer));
305+
using var stream = new MemoryStream(ev.GetBytes(_serializer));
306306

307307
// Put this single event back into the queue so we can retry it separately.
308308
await _eventPostService.EnqueueAsync(new EventPost(false)

‎src/Exceptionless.Core/Models/Event.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ void IJsonOnDeserialized.OnDeserialized()
9898
return element.ValueKind switch
9999
{
100100
JsonValueKind.String => element.GetString(),
101-
JsonValueKind.Number => element.TryGetInt64(out long l) ? l : element.GetDouble(),
101+
JsonValueKind.Number => element.TryGetInt64(out long l) ? (object)l : element.GetDouble(),
102102
JsonValueKind.True => true,
103103
JsonValueKind.False => false,
104104
JsonValueKind.Null or JsonValueKind.Undefined => null,

‎src/Exceptionless.Core/Plugins/EventUpgrader/Default/V2_EventUpgrade.cs‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -150,11 +150,8 @@ private void RenameAndValidateExtraExceptionProperties(string? id, JsonObject er
150150
var extraProperties = JsonNode.Parse(json) as JsonObject;
151151
if (extraProperties is not null)
152152
{
153-
foreach (var property in extraProperties.ToList())
153+
foreach (var property in extraProperties.ToList().Where(p => !p.Value.IsNullOrEmpty()))
154154
{
155-
if (property.Value.IsNullOrEmpty())
156-
continue;
157-
158155
string dataKey = property.Key;
159156
if (extendedData[dataKey] is not null)
160157
dataKey = "_" + dataKey;
@@ -165,7 +162,7 @@ private void RenameAndValidateExtraExceptionProperties(string? id, JsonObject er
165162
}
166163
}
167164
}
168-
catch (Exception) { }
165+
catch (JsonException) { }
169166

170167
if (ext.IsNullOrEmpty())
171168
return;

‎src/Exceptionless.Core/Serialization/ElasticSystemTextJsonSerializer.cs‎

Lines changed: 27 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,19 @@ private static JsonSerializerOptions CreateOptions(JsonSerializerOptions? baseOp
5959
options.DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull;
6060
options.WriteIndented = writeIndented;
6161

62-
// Insert Elasticsearch converters at the beginning for priority
62+
// Replace the default ObjectToInferredTypesConverter with one that returns Int64
63+
// for all integers, matching JSON.NET DataObjectConverter behavior. This ensures
64+
// Event.Data values round-trip through Elasticsearch with consistent types.
65+
var defaultConverter = options.Converters.FirstOrDefault(c => c is ObjectToInferredTypesConverter);
66+
if (defaultConverter is not null)
67+
options.Converters.Remove(defaultConverter);
68+
options.Converters.Insert(0, new ObjectToInferredTypesConverter(preferInt64: true));
69+
70+
// Insert Elasticsearch converters for priority
6371
// Order matters: more specific converters should come first
64-
options.Converters.Insert(0, new DynamicDictionaryConverter());
65-
options.Converters.Insert(1, new Iso8601DateTimeOffsetConverter());
66-
options.Converters.Insert(2, new Iso8601DateTimeConverter());
72+
options.Converters.Insert(1, new DynamicDictionaryConverter());
73+
options.Converters.Insert(2, new Iso8601DateTimeOffsetConverter());
74+
options.Converters.Insert(3, new Iso8601DateTimeConverter());
6775

6876
return options;
6977
}
@@ -79,8 +87,11 @@ private JsonSerializerOptions GetOptions(SerializationFormatting formatting) =>
7987
if (IsEmptyStream(stream))
8088
return null;
8189

82-
var buffer = ReadStreamToSpan(stream);
83-
return JsonSerializer.Deserialize(buffer, type, _optionsCompact.Value);
90+
// Fast path: MemoryStream with accessible buffer avoids buffering
91+
if (stream is MemoryStream ms && ms.TryGetBuffer(out var segment))
92+
return JsonSerializer.Deserialize(segment.AsSpan((int)ms.Position), type, _optionsCompact.Value);
93+
94+
return JsonSerializer.Deserialize(stream, type, _optionsCompact.Value);
8495
}
8596

8697
/// <inheritdoc />
@@ -89,15 +100,22 @@ private JsonSerializerOptions GetOptions(SerializationFormatting formatting) =>
89100
if (IsEmptyStream(stream))
90101
return default;
91102

92-
var buffer = ReadStreamToSpan(stream);
93-
return JsonSerializer.Deserialize<T>(buffer, _optionsCompact.Value);
103+
// Fast path: MemoryStream with accessible buffer avoids buffering
104+
if (stream is MemoryStream ms && ms.TryGetBuffer(out var segment))
105+
return JsonSerializer.Deserialize<T>(segment.AsSpan((int)ms.Position), _optionsCompact.Value);
106+
107+
return JsonSerializer.Deserialize<T>(stream, _optionsCompact.Value);
94108
}
95109

96110
/// <inheritdoc />
97111
public void Serialize<T>(T data, Stream stream, SerializationFormatting formatting = SerializationFormatting.None)
98112
{
99-
using var writer = new Utf8JsonWriter(stream);
100113
var options = GetOptions(formatting);
114+
using var writer = new Utf8JsonWriter(stream, new JsonWriterOptions
115+
{
116+
Indented = formatting == SerializationFormatting.Indented,
117+
Encoder = options.Encoder
118+
});
101119

102120
if (data is null)
103121
{
@@ -153,31 +171,10 @@ public Task SerializeAsync<T>(
153171

154172
#endregion
155173

156-
#region Stream Helpers
157-
158174
private static bool IsEmptyStream(Stream? stream)
159175
{
160176
return stream is null || stream == Stream.Null || (stream.CanSeek && stream.Length == 0);
161177
}
162-
163-
private static ReadOnlySpan<byte> ReadStreamToSpan(Stream stream)
164-
{
165-
// Fast path: if already a MemoryStream with accessible buffer, use it directly
166-
if (stream is MemoryStream ms && ms.TryGetBuffer(out var segment))
167-
{
168-
return segment.AsSpan();
169-
}
170-
171-
// Slow path: copy to new buffer
172-
using var buffer = stream.CanSeek
173-
? new MemoryStream((int)stream.Length)
174-
: new MemoryStream();
175-
176-
stream.CopyTo(buffer);
177-
return buffer.TryGetBuffer(out var seg) ? seg.AsSpan() : buffer.ToArray();
178-
}
179-
180-
#endregion
181178
}
182179

183180
#region Elasticsearch-Specific Converters

0 commit comments

Comments
 (0)