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 bug in IsLong implementation #4655

Merged
merged 1 commit into from
Apr 22, 2020
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
16 changes: 11 additions & 5 deletions src/Elasticsearch.Net/Extensions/ArraySegmentBytesExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Globalization;
using System.Runtime.CompilerServices;
using Elasticsearch.Net.Utf8Json;
using Elasticsearch.Net.Utf8Json.Formatters;
Expand Down Expand Up @@ -29,9 +30,9 @@ public static bool IsDouble(this ref ArraySegment<byte> arraySegment)
return false;
}

private static readonly byte[] LongMaxValue = StringEncoding.UTF8.GetBytes(long.MaxValue.ToString());
private static readonly byte[] LongMaxValue = StringEncoding.UTF8.GetBytes(long.MaxValue.ToString(CultureInfo.InvariantCulture));

private static readonly byte[] LongMinValue = StringEncoding.UTF8.GetBytes(long.MinValue.ToString());
private static readonly byte[] LongMinValue = StringEncoding.UTF8.GetBytes(long.MinValue.ToString(CultureInfo.InvariantCulture));

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static bool IsLong(this ref ArraySegment<byte> arraySegment)
Expand All @@ -45,22 +46,27 @@ public static bool IsLong(this ref ArraySegment<byte> arraySegment)
return false;

var longBytes = isNegative ? LongMinValue : LongMaxValue;
var i = isNegative ? 1 : 0;

// this doesn't handle positive values that are prefixed with + symbol.
// Elasticsearch does not return values with this prefix.
var i = isNegative ? 1 : 0;
var check = arraySegment.Count == longBytes.Length;
while (i < arraySegment.Count)
{
var b = arraySegment.Array[arraySegment.Offset + i];
if (!NumberConverter.IsNumber(b))
return false;

// only compare to long.MinValue or long.MaxValue if we're dealing with same number of bytes
if (arraySegment.Count == longBytes.Length)
if (check)
{
// larger than long.MinValue or long.MaxValue, bail early.
if (b > longBytes[i])
return false;

// only continue to check bytes if current byte is equal to longBytes[i]
if (b < longBytes[i])
return true;
check = false;
}

i++;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public void Serialize(ref JsonWriter writer, object value, IJsonFormatterResolve

if (t.IsEnum)
{
writer.WriteString(t.ToString()); // enum as stringq
writer.WriteString(t.ToString()); // enum as string
return;
}

Expand Down Expand Up @@ -158,13 +158,10 @@ public object Deserialize(ref JsonReader reader, IJsonFormatterResolver formatte
case JsonToken.Number:
var numberSegment = reader.ReadNumberSegment();
// conditional operator here would cast both to double, so don't use.
// Check for IsDouble first, IsDouble && IsLong can both return true, prefer precision
if (numberSegment.IsDouble())
return NumberConverter.ReadDouble(numberSegment.Array, numberSegment.Offset, out _);
// Check for IsLong first, avoid floating point rounding
if (numberSegment.IsLong())
return NumberConverter.ReadInt64(numberSegment.Array, numberSegment.Offset, out _);


return NumberConverter.ReadDouble(numberSegment.Array, numberSegment.Offset, out _);
case JsonToken.String:
return reader.ReadString();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
using System.Text;
using Elastic.Xunit.XunitPlumbing;
using Elasticsearch.Net;
using FluentAssertions;
using Tests.Core.Client;

namespace Tests.CodeStandards.Serialization
{
public class PrimitiveObjectFormatterTests
{
private static DynamicValue GetValue(string json)
{
var bytes = Encoding.UTF8.GetBytes(json);
var client = TestClient.FixedInMemoryClient(bytes);
var response = client.LowLevel.Search<DynamicResponse>(PostData.Empty);
return response.Body["value"];
}

[U]
public void ReadDoubleWhenContainsDecimalPoint()
{
var value = GetValue(@"{""value"":0.43066659210472646}");
value.Value.GetType().Should().Be<double>();
value.Value.Should().Be(0.43066659210472646);
}

[U]
public void ReadDoubleWhenContainsENotation()
{
var value = GetValue(@"{""value"":1.7976931348623157E+308}");
value.Value.GetType().Should().Be<double>();
value.Value.Should().Be(double.MaxValue);
}

[U]
public void ReadDoubleWhenGreaterThanLongMaxValue()
{
var value = GetValue($"{{\"value\":{((double)long.MaxValue) + 1}}}");
value.Value.GetType().Should().Be<double>();
value.Value.Should().Be(((double)long.MaxValue) + 1);
}

[U]
public void ReadDoubleWhenLessThanLongMinValue()
{
var value = GetValue($"{{\"value\":{((double)long.MinValue) - 1}}}");
value.Value.GetType().Should().Be<double>();
value.Value.Should().Be(((double)long.MinValue) - 1);
}

[U]
public void ReadLongWhenLongMaxValue()
{
var value = GetValue($"{{\"value\":{long.MaxValue}}}");
value.Value.GetType().Should().Be<long>();
value.Value.Should().Be(long.MaxValue);
}

[U]
public void ReadLongWhenLessThanLongMaxValue()
{
var value = GetValue($"{{\"value\":{long.MaxValue - 1}}}");
value.Value.GetType().Should().Be<long>();
value.Value.Should().Be(long.MaxValue - 1);
}

[U]
public void ReadLongWhenGreaterThanLongMinValue()
{
var value = GetValue($"{{\"value\":{long.MinValue + 1}}}");
value.Value.GetType().Should().Be<long>();
value.Value.Should().Be(long.MinValue + 1);
}

[U]
public void ReadLongWhenLongMinValue()
{
var value = GetValue($"{{\"value\":{long.MinValue}}}");
value.Value.GetType().Should().Be<long>();
value.Value.Should().Be(long.MinValue);
}
}
}