diff --git a/src/Argon.FSharp/FSharpMapConverter.cs b/src/Argon.FSharp/FSharpMapConverter.cs index 5cf578484..ff2464c04 100644 --- a/src/Argon.FSharp/FSharpMapConverter.cs +++ b/src/Argon.FSharp/FSharpMapConverter.cs @@ -31,7 +31,10 @@ public override void WriteJson(JsonWriter writer, object value, JsonSerializer s public static void WriteMap(JsonWriter writer, FSharpMap value, JsonSerializer serializer) where T : notnull => - serializer.Serialize(writer, value.ToDictionary(_ => _.Key, _ => _.Value)); + // wrap rather than copy: ReadOnlyDictionary is an O(1) view over the map. Its runtime type is not + // FSharpMap<,> so it routes through the dictionary contract instead of re-entering this converter, + // whereas ToDictionary allocated a whole Dictionary and rehashed every entry up front. + serializer.Serialize(writer, new System.Collections.ObjectModel.ReadOnlyDictionary(value)); public override object? ReadJson(JsonReader reader, Type type, object? existingValue, JsonSerializer serializer) { diff --git a/src/Argon.JsonPath/BooleanQueryExpression.cs b/src/Argon.JsonPath/BooleanQueryExpression.cs index 2c75306a0..e2fe6513c 100644 --- a/src/Argon.JsonPath/BooleanQueryExpression.cs +++ b/src/Argon.JsonPath/BooleanQueryExpression.cs @@ -11,6 +11,32 @@ class BooleanQueryExpression(QueryOperator @operator, object left, object? right readonly JToken[]? leftConstant = left is JToken leftToken ? [leftToken] : null; readonly JToken[]? rightConstant = right is JToken rightToken ? [rightToken] : null; + // the regex operand is parsed once here instead of re-sliced and re-parsed per candidate token; + // a malformed pattern is surfaced as a JsonException at parse time rather than an + // ArgumentOutOfRangeException during evaluation + readonly (string Pattern, RegexOptions Options)? regex = + @operator == QueryOperator.RegexEquals ? ParseRegex(right) : null; + + static (string Pattern, RegexOptions Options) ParseRegex(object? right) + { + if (right is not JValue {Value: string regexText}) + { + throw new JsonException("A regex query operator '=~' requires a regex operand."); + } + + var patternOptionDelimiterIndex = regexText.LastIndexOf('/'); + + // a valid pattern is enclosed in slashes: /pattern/ or /pattern/options + if (regexText.Length < 2 || regexText[0] != '/' || patternOptionDelimiterIndex < 1) + { + throw new JsonException($"Path regex must be enclosed in slashes, for example /pattern/: {regexText}"); + } + + var pattern = regexText.Substring(1, patternOptionDelimiterIndex - 1); + var options = MiscellaneousUtils.GetRegexOptions(regexText.AsSpan(patternOptionDelimiterIndex + 1)); + return (pattern, options); + } + static IEnumerable GetFilterResult(JToken root, JToken t, object? o) { if (o is List pathFilters) @@ -80,7 +106,7 @@ bool MatchTokens(JToken? leftResult, JToken? rightResult, JsonSelectSettings set switch (Operator) { case QueryOperator.RegexEquals: - if (RegexEquals(leftValue, rightValue, settings)) + if (RegexEquals(leftValue, settings)) { return true; } @@ -161,22 +187,16 @@ QueryOperator.Exists or return false; } - static bool RegexEquals(JValue input, JValue pattern, JsonSelectSettings settings) + bool RegexEquals(JValue input, JsonSelectSettings settings) { - if (input.Type != JTokenType.String || - pattern.Type != JTokenType.String) + if (input.Type != JTokenType.String) { return false; } - var regexText = ((string) pattern.GetValue()).AsSpan(); - var patternOptionDelimiterIndex = regexText.LastIndexOf('/'); - - var patternText = regexText.Slice(1, patternOptionDelimiterIndex - 1); - var optionsText = regexText[(patternOptionDelimiterIndex + 1)..]; - + var (pattern, options) = regex!.Value; var timeout = settings.RegexMatchTimeout ?? Regex.InfiniteMatchTimeout; - return Regex.IsMatch((string) input.GetValue(), patternText.ToString(), MiscellaneousUtils.GetRegexOptions(optionsText), timeout); + return Regex.IsMatch((string) input.GetValue(), pattern, options, timeout); } static bool EqualsWithStringCoercion(JValue value, JValue queryValue) diff --git a/src/Argon.JsonPath/JPath.cs b/src/Argon.JsonPath/JPath.cs index 6982cbda9..bb43c4759 100644 --- a/src/Argon.JsonPath/JPath.cs +++ b/src/Argon.JsonPath/JPath.cs @@ -32,7 +32,8 @@ void ParseMain() if (expression[currentIndex] == '$') { - if (expression.Length == 1) + // '$' is the last character (guard against leading whitespace, where currentIndex > 0) + if (currentIndex == expression.Length - 1) { return; } @@ -186,6 +187,9 @@ PathFilter ParseIndexer(char indexerOpenChar, bool scan) EatWhitespace(); + // EatWhitespace can consume the remaining characters (e.g. "$[ ") + EnsureLength("Path ended with open indexer."); + if (expression[currentIndex] == '\'') { return ParseQuotedField(indexerCloseChar, scan); @@ -387,6 +391,9 @@ PathFilter ParseQuery(char indexerCloseChar, bool scan) EnsureLength("Path ended with open indexer."); EatWhitespace(); + // EatWhitespace can consume the remaining characters + EnsureLength("Path ended with open indexer."); + if (this.expression[currentIndex] != indexerCloseChar) { throw new JsonException($"Unexpected character while parsing path indexer: {this.expression[currentIndex]}"); @@ -449,90 +456,97 @@ object ParseSide() return new JValue(value); } + // a value that ran to the end of the expression (e.g. "$[?(1") leaves currentIndex at the end; + // report it as an open query rather than indexing past the end + EnsureLength("Path ended with open query."); + throw CreateUnexpectedCharacterException(); } QueryExpression ParseExpression() { - QueryExpression? rootExpression = null; - CompositeExpression? parentExpression = null; + // '&&' binds tighter than '||': collect '&&'-joined terms into an And group, then + // join the And groups with '||' into an Or. So "a && b || c" parses as "(a && b) || c". + var orGroups = new List(); + var andGroup = new List(); - while (currentIndex < expression.Length) + while (true) { - var left = ParseSide(); - object? right = null; - - QueryOperator op; - if (expression[currentIndex] == ')' - || expression[currentIndex] == '|' - || expression[currentIndex] == '&') - { - op = QueryOperator.Exists; - } - else - { - op = ParseOperator(); + // guards re-entry after consuming a '&&'/'||' that was the last thing in the expression + EnsureLength("Path ended with open query."); - right = ParseSide(); - } + andGroup.Add(ParseBooleanExpression()); - var booleanExpression = new BooleanQueryExpression(op, left, right); + // ParseBooleanExpression always leaves currentIndex on an in-bounds terminator + var currentChar = expression[currentIndex]; - if (expression[currentIndex] == ')') + if (currentChar == '&') { - if (parentExpression != null) + if (!Match("&&")) { - parentExpression.Expressions.Add(booleanExpression); - return rootExpression!; + throw CreateUnexpectedCharacterException(); } - return booleanExpression; + continue; } - if (expression[currentIndex] == '&') + // the '&&' group is complete: fold it into a single expression + orGroups.Add(Combine(andGroup, QueryOperator.And)); + andGroup = []; + + if (currentChar == '|') { - if (!Match("&&")) + if (!Match("||")) { throw CreateUnexpectedCharacterException(); } - if (parentExpression is not {Operator: QueryOperator.And}) - { - var andExpression = new CompositeExpression(QueryOperator.And); - - parentExpression?.Expressions.Add(andExpression); - - parentExpression = andExpression; - - rootExpression ??= parentExpression; - } - - parentExpression.Expressions.Add(booleanExpression); + continue; } - if (expression[currentIndex] == '|') + if (currentChar == ')') { - if (!Match("||")) - { - throw CreateUnexpectedCharacterException(); - } + return Combine(orGroups, QueryOperator.Or); + } - if (parentExpression is not {Operator: QueryOperator.Or}) - { - var orExpression = new CompositeExpression(QueryOperator.Or); + throw CreateUnexpectedCharacterException(); + } + } + + // wraps the expressions in a composite of the given operator, or returns the single expression as-is + static QueryExpression Combine(List expressions, QueryOperator @operator) + { + if (expressions.Count == 1) + { + return expressions[0]; + } - parentExpression?.Expressions.Add(orExpression); + return new CompositeExpression(@operator) + { + Expressions = expressions + }; + } - parentExpression = orExpression; + BooleanQueryExpression ParseBooleanExpression() + { + var left = ParseSide(); + object? right = null; - rootExpression ??= parentExpression; - } + QueryOperator op; + if (expression[currentIndex] == ')' + || expression[currentIndex] == '|' + || expression[currentIndex] == '&') + { + op = QueryOperator.Exists; + } + else + { + op = ParseOperator(); - parentExpression.Expressions.Add(booleanExpression); - } + right = ParseSide(); } - throw new JsonException("Path ended with open query."); + return new(op, left, right); } bool TryParseValue(out object? value) diff --git a/src/Argon/Converters/ConverterReaderExtensions.cs b/src/Argon/Converters/ConverterReaderExtensions.cs new file mode 100644 index 000000000..fa25569a3 --- /dev/null +++ b/src/Argon/Converters/ConverterReaderExtensions.cs @@ -0,0 +1,21 @@ +// Copyright (c) 2007 James Newton-King. All rights reserved. +// Use of this source code is governed by The MIT License, +// as found in the license.md file. + +namespace Argon; + +static class ConverterReaderExtensions +{ + // Reads the current token as a string, throwing a JsonSerializationException that carries the + // JSON path/line info when the token is not a string. A direct (string)reader.Value cast would + // otherwise surface a raw InvalidCastException (or null-reference) with no context. + public static string GetConverterString(this JsonReader reader, Type targetType) + { + if (reader.Value is string value) + { + return value; + } + + throw JsonSerializationException.Create(reader, $"Unexpected token {reader.TokenType} when parsing a {targetType.Name}. Expected a string."); + } +} diff --git a/src/Argon/Converters/DriveInfoConverter.cs b/src/Argon/Converters/DriveInfoConverter.cs index 64b83f055..51cec89fe 100644 --- a/src/Argon/Converters/DriveInfoConverter.cs +++ b/src/Argon/Converters/DriveInfoConverter.cs @@ -5,5 +5,5 @@ public override void WriteJson(JsonWriter writer, DriveInfo value, JsonSerialize writer.WriteValue(value.Name.Replace('\\', '/')); public override DriveInfo ReadJson(JsonReader reader, Type type, DriveInfo? existingValue, bool hasExisting, JsonSerializer serializer) => - new(reader.StringValue); + new(reader.GetConverterString(type)); } \ No newline at end of file diff --git a/src/Argon/Converters/EncodingConverter.cs b/src/Argon/Converters/EncodingConverter.cs index 03398c69f..2ecb55a6b 100644 --- a/src/Argon/Converters/EncodingConverter.cs +++ b/src/Argon/Converters/EncodingConverter.cs @@ -7,5 +7,5 @@ public override void WriteJson(JsonWriter writer, Encoding value, JsonSerializer writer.WriteValue(value.WebName); public override Encoding ReadJson(JsonReader reader, Type type, Encoding? existingValue, bool hasExisting, JsonSerializer serializer) => - Encoding.GetEncoding(reader.StringValue); + Encoding.GetEncoding(reader.GetConverterString(type)); } \ No newline at end of file diff --git a/src/Argon/Converters/PathInfoConverter.cs b/src/Argon/Converters/PathInfoConverter.cs index 81975acb2..5f6999fe6 100644 --- a/src/Argon/Converters/PathInfoConverter.cs +++ b/src/Argon/Converters/PathInfoConverter.cs @@ -8,11 +8,18 @@ public override void WriteJson(JsonWriter writer, object value, JsonSerializer s public override object? ReadJson(JsonReader reader, Type type, object? existingValue, JsonSerializer serializer) { - if (reader.Value is not string value) + if (reader.TokenType is JsonToken.Null or JsonToken.Undefined) { return null; } + // only a genuine null maps to null; a non-string token (number, bool, ...) would otherwise + // be silently dropped to null, losing data instead of reporting the mismatch + if (reader.Value is not string value) + { + throw JsonSerializationException.Create(reader, $"Unexpected token {reader.TokenType} when parsing a {type.Name}. Expected a string or null."); + } + var path = value.Replace(Path.AltDirectorySeparatorChar, Path.DirectorySeparatorChar); if (type == typeof(DirectoryInfo)) diff --git a/src/Argon/Converters/StringBuilderConverter.cs b/src/Argon/Converters/StringBuilderConverter.cs index 1efca1e3a..219167e5d 100644 --- a/src/Argon/Converters/StringBuilderConverter.cs +++ b/src/Argon/Converters/StringBuilderConverter.cs @@ -7,5 +7,5 @@ public override void WriteJson(JsonWriter writer, StringBuilder value, JsonSeria writer.WriteValue(value); public override StringBuilder ReadJson(JsonReader reader, Type type, StringBuilder? existingValue, bool hasExisting, JsonSerializer serializer) => - new(reader.StringValue); + new(reader.GetConverterString(type)); } \ No newline at end of file diff --git a/src/Argon/Converters/StringWriterConverter.cs b/src/Argon/Converters/StringWriterConverter.cs index 1c6b9856c..966070aad 100644 --- a/src/Argon/Converters/StringWriterConverter.cs +++ b/src/Argon/Converters/StringWriterConverter.cs @@ -7,5 +7,5 @@ public override void WriteJson(JsonWriter writer, StringWriter value, JsonSerial writer.WriteValue(value.GetStringBuilder()); public override StringWriter ReadJson(JsonReader reader, Type type, StringWriter? existingValue, bool hasExisting, JsonSerializer serializer) => - new(new StringBuilder(reader.StringValue)); + new(new StringBuilder(reader.GetConverterString(type))); } \ No newline at end of file diff --git a/src/Argon/Converters/TimeZoneInfoConverter.cs b/src/Argon/Converters/TimeZoneInfoConverter.cs index 89d478bd4..cea2620bc 100644 --- a/src/Argon/Converters/TimeZoneInfoConverter.cs +++ b/src/Argon/Converters/TimeZoneInfoConverter.cs @@ -5,5 +5,5 @@ public override void WriteJson(JsonWriter writer, TimeZoneInfo value, JsonSerial writer.WriteValue(value.Id); public override TimeZoneInfo ReadJson(JsonReader reader, Type type, TimeZoneInfo? existingValue, bool hasExisting, JsonSerializer serializer) => - TimeZoneInfo.FindSystemTimeZoneById(reader.StringValue); + TimeZoneInfo.FindSystemTimeZoneById(reader.GetConverterString(type)); } \ No newline at end of file diff --git a/src/Argon/Converters/VersionConverter.cs b/src/Argon/Converters/VersionConverter.cs index 5aec38447..c26275633 100644 --- a/src/Argon/Converters/VersionConverter.cs +++ b/src/Argon/Converters/VersionConverter.cs @@ -14,5 +14,5 @@ public override void WriteJson(JsonWriter writer, Version value, JsonSerializer writer.WriteValue(value.ToString()); public override Version ReadJson(JsonReader reader, Type type, Version? existingValue, bool hasExisting, JsonSerializer serializer) => - new(reader.StringValue); + new(reader.GetConverterString(type)); } \ No newline at end of file diff --git a/src/Argon/DefaultJsonNameTable.cs b/src/Argon/DefaultJsonNameTable.cs index ba364dccb..25b05da67 100644 --- a/src/Argon/DefaultJsonNameTable.cs +++ b/src/Argon/DefaultJsonNameTable.cs @@ -12,6 +12,9 @@ public class DefaultJsonNameTable : JsonNameTable // used to defeat hashtable DoS attack where someone passes in lots of strings that hash to the same hash code static readonly int hashCodeRandomizer; + // dedicated lock: Add mutates the entries field (Grow replaces it), so the field + // itself cannot be the monitor - locking it would break mutual exclusion across a resize + readonly object addLock = new(); int count; Entry[] entries; int mask = 31; @@ -76,7 +79,7 @@ public DefaultJsonNameTable() => /// The resolved string. public override string Add(string key) { - lock (entries) + lock (addLock) { return InnerAdd(key); } diff --git a/src/Argon/JsonPosition.cs b/src/Argon/JsonPosition.cs index f11af5a97..30f873bbd 100644 --- a/src/Argon/JsonPosition.cs +++ b/src/Argon/JsonPosition.cs @@ -83,14 +83,22 @@ internal static string BuildPath(List positions, JsonPosition? cur var builder = new StringBuilder(capacity); StringWriter? writer = null; char[]? buffer = null; - foreach (var state in positions) + try { - state.WriteTo(builder, ref writer, ref buffer); - } + foreach (var state in positions) + { + state.WriteTo(builder, ref writer, ref buffer); + } - currentPosition?.WriteTo(builder, ref writer, ref buffer); + currentPosition?.WriteTo(builder, ref writer, ref buffer); - return builder.ToString(); + return builder.ToString(); + } + finally + { + // WriteTo rents a pooled buffer when escaping property names with special characters; return it + BufferUtils.ReturnBuffer(buffer); + } } internal static string FormatMessage(IJsonLineInfo? lineInfo, string path, string message) diff --git a/src/Argon/JsonTextReader.cs b/src/Argon/JsonTextReader.cs index ca832f680..fffdb66d6 100644 --- a/src/Argon/JsonTextReader.cs +++ b/src/Argon/JsonTextReader.cs @@ -32,6 +32,11 @@ public class JsonTextReader : JsonReader, IJsonLineInfo /// public JsonTextReader(TextReader reader, int bufferSize = 1024) { + if (bufferSize <= 0) + { + throw new ArgumentOutOfRangeException(nameof(bufferSize), bufferSize, "Buffer size must be a positive value."); + } + this.reader = reader; lineNumber = 1; diff --git a/src/Argon/JsonTextWriter.cs b/src/Argon/JsonTextWriter.cs index 85feffa82..c5da8068f 100644 --- a/src/Argon/JsonTextWriter.cs +++ b/src/Argon/JsonTextWriter.cs @@ -709,6 +709,18 @@ public override void WriteComment(string? text) writer.Write("*/"); } + /// + /// Writes a comment /*...*/ containing the specified text. + /// + public override void WriteComment(CharSpan text) + { + InternalWriteComment(); + + writer.Write("/*"); + writer.Write(text); + writer.Write("*/"); + } + /// /// Writes the given white space. /// @@ -719,6 +731,16 @@ public override void WriteWhitespace(string ws) writer.Write(ws); } + /// + /// Writes the given white space. + /// + public override void WriteWhitespace(CharSpan ws) + { + InternalWriteWhitespace(ws); + + writer.Write(ws); + } + void EnsureBuffer() => // maximum buffer sized used when writing iso date writeBuffer ??= BufferUtils.RentBuffer(35); diff --git a/src/Argon/Linq/JContainer.cs b/src/Argon/Linq/JContainer.cs index 575ba6a28..c5b19d3fc 100644 --- a/src/Argon/Linq/JContainer.cs +++ b/src/Argon/Linq/JContainer.cs @@ -31,7 +31,7 @@ internal JContainer(JContainer other) : TryAddInternal(i, children[i], false); } - SetLineInfo(this, null); + SetLineInfo(other, null); } /// diff --git a/src/Argon/Linq/JTokenWriter.cs b/src/Argon/Linq/JTokenWriter.cs index 5b19d8af2..9d5509837 100644 --- a/src/Argon/Linq/JTokenWriter.cs +++ b/src/Argon/Linq/JTokenWriter.cs @@ -130,6 +130,24 @@ public override void WritePropertyName(string name) base.WritePropertyName(name); } + /// + /// Writes the property name of a name/value pair on a JSON object. + /// + public override void WritePropertyName(CharSpan name) + { + var nameString = name.ToString(); + + // avoid duplicate property name exception + // last property name wins + (parent as JObject)?.Remove(nameString); + + AddParent(new JProperty(nameString)); + + // don't set state until after in case of an error + // incorrect state will cause issues if writer is disposed when closing open properties + base.WritePropertyName(name); + } + void AddRawValue(object? value, JTokenType type) => AddJValue(new(value, type)); @@ -250,6 +268,15 @@ public override void WriteValue(string? value) AddJValue(new(value)); } + /// + /// Writes a value. + /// + public override void WriteValue(CharSpan value) + { + base.WriteValue(value); + AddJValue(new(value.ToString())); + } + /// /// Writes a value. /// @@ -391,6 +418,12 @@ public override void WriteValue(DateTimeOffset value) /// public override void WriteValue(byte[]? value) { + if (value == null) + { + WriteNull(); + return; + } + base.WriteValue(value); AddJValue(new(value, JTokenType.Bytes)); } @@ -418,6 +451,12 @@ public override void WriteValue(Guid value) /// public override void WriteValue(Uri? value) { + if (value == null) + { + WriteNull(); + return; + } + base.WriteValue(value); AddJValue(new(value)); } diff --git a/src/Argon/Linq/JValue.cs b/src/Argon/Linq/JValue.cs index 0c51c004e..25a01081c 100644 --- a/src/Argon/Linq/JValue.cs +++ b/src/Argon/Linq/JValue.cs @@ -779,6 +779,18 @@ int GetValueHashCode() return d.GetHashCode(); } + // A String JValue can be backed by either a string or a boxed char (new JValue('a')). + // Compare treats them as equal via Convert.ToString, so hash the same canonical form. + if (valueType == JTokenType.String) + { + if (value is string stringValue) + { + return stringValue.GetHashCode(); + } + + return Convert.ToString(value, InvariantCulture)!.GetHashCode(); + } + return value.GetHashCode(); } diff --git a/src/Argon/NamingStrategy/CamelCasePropertyNamesContractResolver.cs b/src/Argon/NamingStrategy/CamelCasePropertyNamesContractResolver.cs index b61e48150..db661968b 100644 --- a/src/Argon/NamingStrategy/CamelCasePropertyNamesContractResolver.cs +++ b/src/Argon/NamingStrategy/CamelCasePropertyNamesContractResolver.cs @@ -14,7 +14,11 @@ public class CamelCasePropertyNamesContractResolver : { static readonly object typeContractCacheLock = new(); static readonly DefaultJsonNameTable NameTable = new(); - static Dictionary, JsonContract>? contractCache; + static Dictionary? contractCache; + + // struct key: a Tuple class key would allocate on every ResolveContract probe, + // i.e. once per value serialized or deserialized + readonly record struct ContractCacheKey(Type ResolverType, Type ContractType); /// /// Initializes a new instance of the class. @@ -34,7 +38,7 @@ public CamelCasePropertyNamesContractResolver() => public override JsonContract ResolveContract(Type type) { // for backwards compatibility the CamelCasePropertyNamesContractResolver shares contracts between instances - var key = new Tuple(GetType(), type); + var key = new ContractCacheKey(GetType(), type); var cache = contractCache; if (cache == null || !cache.TryGetValue(key, out var contract)) @@ -45,7 +49,7 @@ public override JsonContract ResolveContract(Type type) lock (typeContractCacheLock) { cache = contractCache; - Dictionary, JsonContract> updatedCache; + Dictionary updatedCache; if (cache == null) { updatedCache = []; diff --git a/src/Argon/Serialization/DefaultContractResolver.cs b/src/Argon/Serialization/DefaultContractResolver.cs index 1cdb359ed..57e758bdc 100644 --- a/src/Argon/Serialization/DefaultContractResolver.cs +++ b/src/Argon/Serialization/DefaultContractResolver.cs @@ -28,7 +28,10 @@ public class DefaultContractResolver : IContractResolver /// public NamingStrategy? NamingStrategy { get; set; } - public static List Converters { get; } = + // read-only: these built-in converters are read (unsynchronized) during contract creation on + // arbitrary threads, so the collection must not be publicly mutable. Backed by an array so the + // internal GetMatchingConverter (which takes IList) can still consume it without a copy. + static readonly JsonConverter[] builtInConverters = [ new StringBuilderConverter(), new ExpandoObjectConverter(), @@ -42,6 +45,8 @@ public class DefaultContractResolver : IContractResolver new StringWriterConverter() ]; + public static IReadOnlyList Converters => builtInConverters; + /// /// Initializes a new instance of the class. /// @@ -87,8 +92,10 @@ protected virtual IEnumerable GetSerializableMembers(Type type) if (memberSerialization == MemberSerialization.Fields) { - // Do not filter ByRef types here because accessing FieldType/PropertyType can trigger additional assembly loads - return type.GetFields(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance); + // Do not filter ByRef types here because accessing FieldType/PropertyType can trigger additional assembly loads. + // Use ReflectionUtils.GetFields (not Type.GetFields) so private fields declared on base classes are included - + // Type.GetFields never returns inherited private fields, which would silently drop them from round-trips. + return ReflectionUtils.GetFields(type, BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance); } var serializableMembers = new List(); @@ -248,18 +255,24 @@ static bool TryGetImmutableConstructor(Type type, JsonPropertyCollection memberP continue; } + var allParametersMatch = true; foreach (var parameter in parameters) { var memberProperty = MatchProperty(memberProperties, parameter.Name, parameter.ParameterType); if (memberProperty == null || memberProperty.Writable) { - constructor = null; - return false; + // this constructor does not match; keep looking at the remaining constructors + // rather than abandoning the whole search + allParametersMatch = false; + break; } } - constructor = constructorItem; - return true; + if (allParametersMatch) + { + constructor = constructorItem; + return true; + } } constructor = null; @@ -390,7 +403,7 @@ void InitializeContract(JsonContract contract) contract.Converter = ResolveContractConverter(nonNullableUnderlyingType); // then see whether object is compatible with any of the built in converters - contract.InternalConverter = JsonSerializer.GetMatchingConverter(Converters, nonNullableUnderlyingType); + contract.InternalConverter = JsonSerializer.GetMatchingConverter(builtInConverters, nonNullableUnderlyingType); if (!contract.IsInstantiable) { diff --git a/src/Argon/Utilities/EnumInfo.cs b/src/Argon/Utilities/EnumInfo.cs index d1c4ab124..83cad857e 100644 --- a/src/Argon/Utilities/EnumInfo.cs +++ b/src/Argon/Utilities/EnumInfo.cs @@ -2,9 +2,12 @@ // Use of this source code is governed by The MIT License, // as found in the license.md file. -class EnumInfo(bool isFlags, ulong[] values, string[] names, string[] resolvedNames) +class EnumInfo(bool isFlags, PrimitiveTypeCode typeCode, ulong[] values, string[] names, string[] resolvedNames) { public readonly bool IsFlags = isFlags; + + // the enum's underlying type code, cached so it is not re-derived reflectively per value written + public readonly PrimitiveTypeCode TypeCode = typeCode; public readonly ulong[] Values = values; public readonly string[] Names = names; public readonly string[] ResolvedNames = resolvedNames; diff --git a/src/Argon/Utilities/EnumUtils.cs b/src/Argon/Utilities/EnumUtils.cs index 3631f5064..5da804674 100644 --- a/src/Argon/Utilities/EnumUtils.cs +++ b/src/Argon/Utilities/EnumUtils.cs @@ -20,12 +20,13 @@ static EnumInfo InitializeValuesAndNames(EnumKey key) var names = Enum.GetNames(enumType); var resolvedNames = new string[names.Length]; var values = new ulong[names.Length]; + var typeCode = ConvertUtils.GetTypeCode(enumType, out _); for (var i = 0; i < names.Length; i++) { var name = names[i]; var f = enumType.GetField(name, BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Static)!; - values[i] = ToUInt64(f.GetValue(null)!); + values[i] = ToUInt64(f.GetValue(null)!, typeCode); var specifiedName = f.GetCustomAttributes(typeof(EnumMemberAttribute), true) .Cast() @@ -46,13 +47,13 @@ static EnumInfo InitializeValuesAndNames(EnumKey key) var isFlags = enumType.IsDefined(typeof(FlagsAttribute), false); - return new(isFlags, values, names, resolvedNames); + return new(isFlags, typeCode, values, names, resolvedNames); } public static bool TryToString(Type enumType, object value, NamingStrategy? namingStrategy, [NotNullWhen(true)] out string? name) { var enumInfo = ValuesAndNamesPerEnum.Get(new(enumType, namingStrategy)); - var v = ToUInt64(value); + var v = ToUInt64(value, enumInfo.TypeCode); if (enumInfo.IsFlags) { @@ -136,10 +137,8 @@ public static bool TryToString(Type enumType, object value, NamingStrategy? nami return stringBuilder.ToString(); // Return the string representation } - static ulong ToUInt64(object value) + static ulong ToUInt64(object value, PrimitiveTypeCode typeCode) { - var typeCode = ConvertUtils.GetTypeCode(value.GetType(), out _); - switch (typeCode) { case PrimitiveTypeCode.SByte: diff --git a/src/Argon/Utilities/ImmutableCollectionsUtils.cs b/src/Argon/Utilities/ImmutableCollectionsUtils.cs index 29851db35..164c7e12a 100644 --- a/src/Argon/Utilities/ImmutableCollectionsUtils.cs +++ b/src/Argon/Utilities/ImmutableCollectionsUtils.cs @@ -34,13 +34,17 @@ static ImmutableCollectionsUtils() var immutableListInfo = new TypeInfo(typeof(ImmutableList<>), GetArrayCreateRange(typeof(ImmutableList))); var immutableStackInfo = new TypeInfo(typeof(ImmutableStack<>), GetArrayCreateRange(typeof(ImmutableStack))); var immutableHashSetInfo = new TypeInfo(typeof(ImmutableHashSet<>), GetArrayCreateRange(typeof(ImmutableHashSet))); - var immutableQueueCreateRange = GetArrayCreateRange(typeof(ImmutableQueue)); + // CreatedType must be the concrete ImmutableQueue<> (as with every sibling entry), not the + // IImmutableQueue<> interface. An interface CreatedType is not instantiable, so the contract's + // IsInstantiable is false and InitializeContract skips default-creator setup - inconsistent + // with IImmutableList/IImmutableStack/IImmutableSet, which all map to their concrete type. + var immutableQueueInfo = new TypeInfo(typeof(ImmutableQueue<>), GetArrayCreateRange(typeof(ImmutableQueue))); arrayDefinitions = new KeyValuePair[] { new(typeof(IImmutableList<>), immutableListInfo), new(typeof(ImmutableList<>), immutableListInfo), - new(typeof(IImmutableQueue<>), new(typeof(IImmutableQueue<>), immutableQueueCreateRange)), - new(typeof(ImmutableQueue<>), new(typeof(ImmutableQueue<>), immutableQueueCreateRange)), + new(typeof(IImmutableQueue<>), immutableQueueInfo), + new(typeof(ImmutableQueue<>), immutableQueueInfo), new(typeof(IImmutableStack<>), immutableStackInfo), new(typeof(ImmutableStack<>), immutableStackInfo), new(typeof(IImmutableSet<>), immutableHashSetInfo), diff --git a/src/Argon/Utilities/JavaScriptUtils.cs b/src/Argon/Utilities/JavaScriptUtils.cs index 15152fd66..2a564ecdb 100644 --- a/src/Argon/Utilities/JavaScriptUtils.cs +++ b/src/Argon/Utilities/JavaScriptUtils.cs @@ -284,10 +284,19 @@ public static string ToEscapedJavaScriptString(CharSpan value, char delimiter, b { var escapeFlags = GetCharEscapeFlags(escapeHandling, delimiter); - using var w = StringUtils.CreateStringWriter(value.Length); + // size for the delimiters too, otherwise the StringBuilder always grows on the first write + using var w = StringUtils.CreateStringWriter(value.Length + (appendDelimiters ? 2 : 0)); char[]? buffer = null; - WriteEscapedJavaScriptString(w, value, delimiter, appendDelimiters, escapeFlags, escapeHandling, ref buffer); - return w.ToString(); + try + { + WriteEscapedJavaScriptString(w, value, delimiter, appendDelimiters, escapeFlags, escapeHandling, ref buffer); + return w.ToString(); + } + finally + { + // WriteEscapedJavaScriptString rents a pooled buffer for \uXXXX escapes; return it + BufferUtils.ReturnBuffer(buffer); + } } static int FirstCharToEscape(CharSpan value, bool[] escapeFlags, EscapeHandling escapeHandling) diff --git a/src/Argon/Utilities/ReflectionUtils.cs b/src/Argon/Utilities/ReflectionUtils.cs index 24cb766b7..d219e98b1 100644 --- a/src/Argon/Utilities/ReflectionUtils.cs +++ b/src/Argon/Utilities/ReflectionUtils.cs @@ -539,7 +539,7 @@ public static TypeNameKey SplitFullyQualifiedTypeName(CharSpan fullTypeName) } [RequiresUnreferencedCode(MiscellaneousUtils.TrimWarning)] - static List GetFields( + internal static List GetFields( [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.NonPublicFields | DynamicallyAccessedMemberTypes.None | DynamicallyAccessedMemberTypes.PublicFields)] Type targetType, BindingFlags bindingFlags) { diff --git a/src/ArgonTests/Benchmarks/AuditPerfBenchmarks.cs b/src/ArgonTests/Benchmarks/AuditPerfBenchmarks.cs new file mode 100644 index 000000000..716a43220 --- /dev/null +++ b/src/ArgonTests/Benchmarks/AuditPerfBenchmarks.cs @@ -0,0 +1,108 @@ +// Copyright (c) 2007 James Newton-King. All rights reserved. +// Use of this source code is governed by The MIT License, +// as found in the license.md file. + +using BenchmarkDotNet.Attributes; + +// Benchmarks covering the performance fixes from the deep-dive audit. + +// DefaultJsonNameTable.Add: the lock-target fix keeps a single monitor across a Grow(), so +// concurrent property-name interning stays correct and contended adds do not corrupt the table. +[MemoryDiagnoser] +public class NameTableAddBenchmark +{ + string[] names; + + [GlobalSetup] + public void Setup() => + names = Enumerable.Range(0, 256) + .Select(_ => $"propertyName{_}") + .ToArray(); + + [Benchmark] + public string AddAcrossGrowth() + { + var table = new DefaultJsonNameTable(); + string last = null; + foreach (var name in names) + { + last = table.Add(name); + } + + return last; + } +} + +// JavaScriptUtils.ToEscapedJavaScriptString: the \uXXXX branch rents a pooled buffer. Before the +// fix that buffer was never returned, turning pooling into pure allocation plus pool churn. +[MemoryDiagnoser] +public class EscapeToStringBenchmark +{ + string[] values; + + [GlobalSetup] + public void Setup() => + // the \u00xx accented characters force the \uXXXX escape path under EscapeNonAscii + values = Enumerable.Range(0, 100) + .Select(_ => $"caf\u00e9 na\u00efve \u00fcber sn\u00f6w {_} tail text here") + .ToArray(); + + [Benchmark] + public string ToStringEscapeNonAscii() + { + string last = null; + foreach (var value in values) + { + last = JsonConvert.ToString(value, '"', EscapeHandling.EscapeNonAscii); + } + + return last; + } +} + +// EnumUtils.ToUInt64: the underlying type code is now cached on EnumInfo instead of being +// re-derived reflectively for every enum value written. +[MemoryDiagnoser] +public class EnumWriteBenchmark +{ + AuditColor[] values; + JsonConverter[] converters; + + [GlobalSetup] + public void Setup() + { + values = Enumerable.Range(0, 500) + .Select(i => (AuditColor) (i % 3)) + .ToArray(); + converters = [new StringEnumConverter()]; + } + + [Benchmark] + public string SerializeEnums() => + JsonConvert.SerializeObject(values, converters); + + public enum AuditColor + { + Red, + Green, + Blue + } +} + +// BooleanQueryExpression regex: the pattern/options are now parsed once at path construction +// instead of being re-sliced, re-allocated and re-parsed for every candidate token. +[MemoryDiagnoser] +public class JsonPathRegexBenchmark +{ + JArray data; + + [GlobalSetup] + public void Setup() => + data = new( + Enumerable.Range(0, 500) + .Select(i => new JObject {["name"] = $"Argon.Package{i}"})); + + [Benchmark] + public int RegexFilter() => + data.SelectTokens("$[?(@.name =~ /^Argon/)]").Count(); +} diff --git a/src/ArgonTests/Benchmarks/CamelCaseBenchmarks.cs b/src/ArgonTests/Benchmarks/CamelCaseBenchmarks.cs index 0db915195..a2cb18b25 100644 --- a/src/ArgonTests/Benchmarks/CamelCaseBenchmarks.cs +++ b/src/ArgonTests/Benchmarks/CamelCaseBenchmarks.cs @@ -26,6 +26,8 @@ public class CamelCaseBenchmarks "ABC" ]; + static readonly CamelCasePropertyNamesContractResolver resolver = new(); + [Benchmark] public void Run() { @@ -34,4 +36,24 @@ public void Run() CamelCaseNamingStrategy.ToCamelCase(names[i]); } } + + // ResolveContract runs once per value serialized/deserialized. It allocated a Tuple key on + // every probe (including cache hits) before the struct-key fix. + [Benchmark] + public JsonContract ResolveContract() + { + JsonContract contract = null; + for (var i = 0; i < 100; i++) + { + contract = resolver.ResolveContract(typeof(ResolveTarget)); + } + + return contract; + } + + public class ResolveTarget + { + public int Id { get; set; } + public string Name { get; set; } + } } diff --git a/src/ArgonTests/BugFixes/AuditFindingsTests.cs b/src/ArgonTests/BugFixes/AuditFindingsTests.cs new file mode 100644 index 000000000..4bb9468de --- /dev/null +++ b/src/ArgonTests/BugFixes/AuditFindingsTests.cs @@ -0,0 +1,360 @@ +// Copyright (c) 2007 James Newton-King. All rights reserved. +// Use of this source code is governed by The MIT License, +// as found in the license.md file. + +using System.Collections.Immutable; + +// Regression tests for the issues found during the deep-dive audit. +public class AuditFindingsTests : TestFixtureBase +{ + #region DefaultJsonNameTable lock + + [Fact] + public void NameTable_resolves_all_names_after_growth() + { + var table = new DefaultJsonNameTable(); + + // more than the initial 32 buckets, forcing at least one Grow() + var names = Enumerable.Range(0, 300) + .Select(_ => $"property{_}") + .ToList(); + + foreach (var name in names) + { + Assert.Equal(name, table.Add(name)); + } + + foreach (var name in names) + { + var key = name.ToCharArray(); + Assert.Equal(name, table.Get(key, 0, key.Length)); + } + } + + [Fact] + public void NameTable_is_thread_safe_under_concurrent_add() + { + var table = new DefaultJsonNameTable(); + var names = Enumerable.Range(0, 400) + .Select(_ => $"name{_}") + .ToArray(); + + // Add locks a dedicated object; before the fix it locked the entries array that Grow() + // replaces, so concurrent adders spanning a resize could corrupt the shared table. + Parallel.For(0, 32, _ => + { + foreach (var name in names) + { + Assert.Equal(name, table.Add(name)); + } + }); + + foreach (var name in names) + { + var key = name.ToCharArray(); + Assert.Equal(name, table.Get(key, 0, key.Length)); + } + } + + #endregion + + #region JTokenWriter span overloads + + [Fact] + public void JTokenWriter_writes_span_property_name_and_value() + { + using var writer = new JTokenWriter(); + writer.WriteStartObject(); + writer.WritePropertyName("name".AsSpan()); + writer.WriteValue("value".AsSpan()); + writer.WriteEndObject(); + + var token = (JObject) writer.Token!; + Assert.Single(token.Properties()); + Assert.Equal("value", (string) token["name"]!); + } + + [Fact] + public void JTokenWriter_writes_span_value_into_array() + { + using var writer = new JTokenWriter(); + writer.WriteStartArray(); + writer.WriteValue("a".AsSpan()); + writer.WriteValue("b".AsSpan()); + writer.WriteEndArray(); + + var array = (JArray) writer.Token!; + Assert.Equal(2, array.Count); + Assert.Equal("a", (string) array[0]!); + Assert.Equal("b", (string) array[1]!); + } + + #endregion + + #region JTokenWriter null byte[] / Uri + + [Fact] + public void JTokenWriter_writes_null_byte_array_as_single_null() + { + using var writer = new JTokenWriter(); + writer.WriteStartArray(); + writer.WriteValue((byte[]) null); + writer.WriteEndArray(); + + var array = (JArray) writer.Token!; + Assert.Single(array); + Assert.Equal(JTokenType.Null, array[0].Type); + } + + [Fact] + public void JTokenWriter_writes_null_uri_in_object_without_throwing() + { + using var writer = new JTokenWriter(); + writer.WriteStartObject(); + writer.WritePropertyName("uri"); + writer.WriteValue((Uri) null); + writer.WriteEndObject(); + + var token = (JObject) writer.Token!; + Assert.Equal(JTokenType.Null, token["uri"]!.Type); + } + + #endregion + + #region JsonTextWriter span comment / whitespace + + [Fact] + public void JsonTextWriter_writes_span_comment() + { + var stringWriter = new StringWriter(); + using (var writer = new JsonTextWriter(stringWriter)) + { + writer.WriteStartArray(); + writer.WriteComment("hello".AsSpan()); + writer.WriteEndArray(); + } + + Assert.Contains("/*hello*/", stringWriter.ToString()); + } + + [Fact] + public void JsonTextWriter_writes_span_whitespace() + { + var stringWriter = new StringWriter(); + using (var writer = new JsonTextWriter(stringWriter)) + { + writer.WriteWhitespace(" ".AsSpan()); + writer.WriteValue(1); + } + + Assert.Equal(" 1", stringWriter.ToString()); + } + + #endregion + + #region JContainer.DeepClone line info + + [Fact] + public void DeepClone_preserves_line_info_on_containers() + { + var settings = new JsonLoadSettings {LineInfoHandling = LineInfoHandling.Load}; + var original = JObject.Parse("{\r\n \"a\": 1\r\n}", settings); + + var originalLineInfo = (IJsonLineInfo) original; + Assert.True(originalLineInfo.HasLineInfo()); + + var clone = (JObject) original.DeepClone(); + var cloneLineInfo = (IJsonLineInfo) clone; + + // before the fix the container clone copied line info from itself (a no-op), so it was lost + Assert.True(cloneLineInfo.HasLineInfo()); + Assert.Equal(originalLineInfo.LineNumber, cloneLineInfo.LineNumber); + Assert.Equal(originalLineInfo.LinePosition, cloneLineInfo.LinePosition); + } + + #endregion + + #region JValue char/string hash consistency + + [Fact] + public void JValue_char_and_string_hash_consistently_in_equality_comparer() + { + var charValue = new JValue('a'); + var stringValue = new JValue("a"); + + Assert.True(charValue.Equals(stringValue)); + + var comparer = JToken.EqualityComparer; + Assert.Equal(comparer.GetHashCode(charValue), comparer.GetHashCode(stringValue)); + + var set = new HashSet(comparer) {charValue}; + Assert.Contains(stringValue, set); + } + + #endregion + + #region JsonTextReader buffer size guard + + [Fact] + public void JsonTextReader_rejects_non_positive_buffer_size() + { + Assert.Throws(() => new JsonTextReader(new StringReader("1"), 0)); + Assert.Throws(() => new JsonTextReader(new StringReader("1"), -5)); + } + + #endregion + + #region MemberSerialization.Fields inherited private fields + + [Fact] + public void Fields_serialization_includes_base_class_private_fields() + { + var model = new FieldsModel(baseValue: 7, derivedValue: 9); + + var json = JsonConvert.SerializeObject(model); + // Type.GetFields does not return inherited private fields, so before the fix baseField was dropped + Assert.Contains("baseField", json); + + var roundTripped = JsonConvert.DeserializeObject(json)!; + Assert.Equal(7, roundTripped.GetBaseValue()); + Assert.Equal(9, roundTripped.DerivedValue); + } + + #endregion + + #region IImmutableQueue contract created type + + [Fact] + public void IImmutableQueue_contract_uses_the_concrete_created_type() + { + var resolver = new DefaultContractResolver(); + var contract = (JsonArrayContract) resolver.ResolveContract(typeof(IImmutableQueue)); + + // was the (non-instantiable) IImmutableQueue<> interface, unlike every sibling immutable + // interface which maps to its concrete type + Assert.Equal(typeof(ImmutableQueue), contract.CreatedType); + + var model = new ImmutableQueueHolder {Queue = ImmutableQueue.Create(1, 2, 3)}; + var json = JsonConvert.SerializeObject(model); + var roundTripped = JsonConvert.DeserializeObject(json)!; + Assert.Equal(new[] {1, 2, 3}, roundTripped.Queue.ToArray()); + } + + #endregion + + #region Converter error handling + + [Fact] + public void VersionConverter_reports_non_string_token_as_json_exception() => + // was a raw InvalidCastException with no path/line info + Assert.Throws(() => JsonConvert.DeserializeObject("123")); + + [Fact] + public void FileInfo_converter_throws_for_non_string_token_instead_of_dropping_to_null() => + // before the fix a non-string, non-null token silently deserialized to null + Assert.Throws(() => JsonConvert.DeserializeObject("123")); + + [Fact] + public void FileInfo_converter_still_maps_null_token_to_null() + { + var result = JsonConvert.DeserializeObject("""{"File":null}"""); + Assert.Null(result.File); + } + + #endregion + + #region Built-in converters exposed read-only + + [Fact] + public void BuiltInConverters_are_exposed_as_read_only_list() + { + var propertyType = typeof(DefaultContractResolver) + .GetProperty(nameof(DefaultContractResolver.Converters))! + .PropertyType; + + Assert.Equal(typeof(IReadOnlyList), propertyType); + Assert.NotEmpty(DefaultContractResolver.Converters); + } + + #endregion + + #region Immutable struct with multiple constructors + + [Fact] + public void Immutable_struct_with_multiple_constructors_round_trips() + { + var json = JsonConvert.SerializeObject(new MultiConstructorImmutable(42)); + var result = JsonConvert.DeserializeObject(json); + Assert.Equal(42, result.Value); + } + + #endregion + + #region JsonPath && / || precedence + + [Fact] + public void JsonPath_and_binds_tighter_than_or() + { + var array = JArray.Parse("""[{"b":1,"c":1},{"a":1,"b":1},{"c":1}]"""); + + // '&&' binds tighter than '||': "(a && b) || c" matches all three items + var result = array.SelectTokens("$[?(@.a && @.b || @.c)]").ToList(); + Assert.Equal(3, result.Count); + + // the logically identical "c || (a && b)" must select the same set regardless of order + var reordered = array.SelectTokens("$[?(@.c || @.a && @.b)]").ToList(); + Assert.Equal(3, reordered.Count); + } + + #endregion + + [JsonObject(MemberSerialization.Fields)] + public class FieldsModel : FieldsModelBase + { + public int DerivedValue; + + public FieldsModel() + { + } + + public FieldsModel(int baseValue, int derivedValue) + { + SetBaseValue(baseValue); + DerivedValue = derivedValue; + } + } + + public class FieldsModelBase + { + int baseField; + + protected void SetBaseValue(int value) => + baseField = value; + + public int GetBaseValue() => + baseField; + } + + public class ImmutableQueueHolder + { + public IImmutableQueue Queue { get; set; } = ImmutableQueue.Empty; + } + + public class FileInfoHolder + { + public FileInfo File { get; set; } + } + + public readonly struct MultiConstructorImmutable + { + // declared before the matching constructor: before the fix a non-matching first + // constructor aborted the whole search + public MultiConstructorImmutable(string ignored) => + Value = -1; + + public MultiConstructorImmutable(int value) => + Value = value; + + public int Value { get; } + } +} diff --git a/src/ArgonTests/Linq/JsonPath/JPathParseTests.cs b/src/ArgonTests/Linq/JsonPath/JPathParseTests.cs index ba443ad16..581d81d73 100644 --- a/src/ArgonTests/Linq/JsonPath/JPathParseTests.cs +++ b/src/ArgonTests/Linq/JsonPath/JPathParseTests.cs @@ -459,23 +459,26 @@ public void FilterExistWithAnd() [Fact] public void FilterExistWithAndOr() { + // '&&' binds tighter than '||', so "name && title || pie" parses as "(name && title) || pie": + // the root is an Or of [ (name && title), pie ]. var path = new JPath("[?(@.name&&@.title||@.pie)]"); - var andExpression = (CompositeExpression) ((QueryFilter) path.Filters[0]).Expression; + var orExpression = (CompositeExpression) ((QueryFilter) path.Filters[0]).Expression; + Assert.Equal(QueryOperator.Or, orExpression.Operator); + Assert.Equal(2, orExpression.Expressions.Count); + + var andExpression = (CompositeExpression) orExpression.Expressions[0]; Assert.Equal(QueryOperator.And, andExpression.Operator); Assert.Equal(2, andExpression.Expressions.Count); - var first = (BooleanQueryExpression) andExpression.Expressions[0]; - var firstPaths = (List) first.Left; - Assert.Equal("name", ((FieldFilter) firstPaths[0]).Name); - Assert.Equal(QueryOperator.Exists, first.Operator); + var andFirst = (BooleanQueryExpression) andExpression.Expressions[0]; + var andFirstPaths = (List) andFirst.Left; + Assert.Equal("name", ((FieldFilter) andFirstPaths[0]).Name); + Assert.Equal(QueryOperator.Exists, andFirst.Operator); - var orExpression = (CompositeExpression) andExpression.Expressions[1]; - Assert.Equal(2, orExpression.Expressions.Count); - - var orFirst = (BooleanQueryExpression) orExpression.Expressions[0]; - var orFirstPaths = (List) orFirst.Left; - Assert.Equal("title", ((FieldFilter) orFirstPaths[0]).Name); - Assert.Equal(QueryOperator.Exists, orFirst.Operator); + var andSecond = (BooleanQueryExpression) andExpression.Expressions[1]; + var andSecondPaths = (List) andSecond.Left; + Assert.Equal("title", ((FieldFilter) andSecondPaths[0]).Name); + Assert.Equal(QueryOperator.Exists, andSecond.Operator); var orSecond = (BooleanQueryExpression) orExpression.Expressions[1]; var orSecondPaths = (List) orSecond.Left; @@ -747,4 +750,49 @@ public void PropertyFollowingEscapedPropertyName() Assert.Equal("System.Xml.ReaderWriter", ((FieldFilter) path.Filters[3]).Name); Assert.Equal("source", ((FieldFilter) path.Filters[4]).Name); } + + // The following malformed paths previously threw IndexOutOfRangeException (indexing past the end + // of the expression) instead of the documented JsonException. + + [Fact] + public void RootDollarAtEndAfterWhitespace() + { + var path = new JPath(" $"); + Assert.Empty(path.Filters); + } + + [Fact] + public void OpenIndexerEndingWithWhitespace() + { + var exception = Assert.Throws(() => new JPath("$[ ")); + Assert.Equal("Path ended with open indexer.", exception.Message); + } + + [Fact] + public void QueryEndingWithUnterminatedNumber() + { + var exception = Assert.Throws(() => new JPath("$[?(1")); + Assert.Equal("Path ended with open query.", exception.Message); + } + + [Fact] + public void QueryEndingWithUnterminatedComparisonValue() + { + var exception = Assert.Throws(() => new JPath("$[?(@.a == 12")); + Assert.Equal("Path ended with open query.", exception.Message); + } + + // '&&' immediately followed by '||' was silently accepted (and evaluated the left operand twice). + [Fact] + public void AndImmediatelyFollowedByOr() + { + var exception = Assert.Throws(() => new JPath("$[?(@.a &&|| @.b)]")); + Assert.Equal("Unexpected character while parsing path query: |", exception.Message); + } + + // '=~' with a non-slash pattern threw ArgumentOutOfRangeException during evaluation; it is now + // reported as a JsonException at parse time. + [Fact] + public void RegexOperatorWithoutSlashesThrowsAtParseTime() => + Assert.Throws(() => new JPath("$[?(@.name =~ 'abc')]")); } \ No newline at end of file diff --git a/src/Benchmark.Tests/Benchmark.Tests.csproj b/src/Benchmark.Tests/Benchmark.Tests.csproj index 15511320b..56acb8a20 100644 --- a/src/Benchmark.Tests/Benchmark.Tests.csproj +++ b/src/Benchmark.Tests/Benchmark.Tests.csproj @@ -1,7 +1,7 @@ Exe - net11.0 + net10.0 false diff --git a/src/Benchmark.Tests/Program.cs b/src/Benchmark.Tests/Program.cs index 3c1133570..49f1fb93b 100644 --- a/src/Benchmark.Tests/Program.cs +++ b/src/Benchmark.Tests/Program.cs @@ -26,7 +26,11 @@ public static void Main(string[] args) typeof(WriterBenchmarks), typeof(LinqBenchmarks), typeof(CreatorDeserializeBenchmark), - typeof(SatelliteBenchmarks) + typeof(SatelliteBenchmarks), + typeof(NameTableAddBenchmark), + typeof(EscapeToStringBenchmark), + typeof(EnumWriteBenchmark), + typeof(JsonPathRegexBenchmark) ]); if (args.Length == 0) { diff --git a/src/Directory.Build.props b/src/Directory.Build.props index 46afbda3b..e2735a28f 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -2,7 +2,7 @@ CS1591;CS1573;NU1605;NU1608;NU1109 preview - 0.35.0 + 0.35.1 1.0.0 Copyright © James Newton-King 2008, Copyright © Simon Cropp 2022 Json