elastic/elasticsearch-net

Broken floating-point source serialization for .NET Framework

Closed this issue · 4 comments

onyxmaster commented

Elastic.Clients.Elasticsearch version:
8.19.5

Elasticsearch version:
Not relevant

.NET runtime version:
.NET Framework 4.8.1

Operating system version:
Windows 11

Description of the problem including expected versus actual behavior:
Floating-point serialization is broken for source serializer on .NET Framework, because the System.Memory shim (even the latest version version 4.6.3) for .NET Framework, does not support specifying custom precision for serialized data.

It crashes with NotSupportedException "The 'G' format combined with a precision is not supported.", stack trace is as follows:

System.Memory.dll!System.Buffers.Text.Utf8Formatter.TryFormatFloatingPoint<double>(double value, System.Span<byte> destination, out int bytesWritten, System.Buffers.StandardFormat format)
	at /_/src/System.Memory/src/System/Buffers/Text/Utf8Formatter/Utf8Formatter.Float.cs(79)
System.Memory.dll!System.Buffers.Text.Utf8Formatter.TryFormat(double value, System.Span<byte> destination, out int bytesWritten, System.Buffers.StandardFormat format)
	at /_/src/System.Memory/src/System/Buffers/Text/Utf8Formatter/Utf8Formatter.Float.cs(33)
Elastic.Clients.Elasticsearch.dll!Elastic.Clients.Elasticsearch.Serialization.DoubleWithFractionalPortionConverter.Write(System.Text.Json.Utf8JsonWriter writer, double value, System.Text.Json.JsonSerializerOptions options)
	at /_/src/Elastic.Clients.Elasticsearch/_Shared/Next/DoubleWithFractionalPortionConverter.cs(94)
System.Text.Json.dll!System.Text.Json.Serialization.JsonConverter<double>.TryWrite(System.Text.Json.Utf8JsonWriter writer, double value, System.Text.Json.JsonSerializerOptions options, ref System.Text.Json.WriteStack state)
	at /_/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs(354)
System.Text.Json.dll!System.Text.Json.Serialization.JsonConverter<double>.WriteCore(System.Text.Json.Utf8JsonWriter writer, double value, System.Text.Json.JsonSerializerOptions options, ref System.Text.Json.WriteStack state)
	at /_/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.WriteCore.cs(16)
System.Text.Json.dll!System.Text.Json.Serialization.Metadata.JsonTypeInfo<double>.Serialize(System.IO.Stream utf8Json, double rootValue, object rootValueBoxed)
	at /_/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.WriteHelpers.cs(314)
System.Text.Json.dll!System.Text.Json.JsonSerializer.Serialize<double>(System.IO.Stream utf8Json, double value, System.Text.Json.JsonSerializerOptions options)
	at /_/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.Stream.cs(83)
Elastic.Transport.dll!Elastic.Transport.SystemTextJsonSerializer.Serialize<double>(double data, System.IO.Stream stream, Elastic.Transport.SerializationFormatting formatting)
	at Elastic.Transport\SystemTextJsonSerializer.cs(66)

Steps to reproduce:

using System.IO;

using Elastic.Clients.Elasticsearch;

using Xunit;

namespace Dnet.Elastic.Tests;

public sealed class SerializationTests
{
    [Fact]
    public void Works()
    {
        using var client = new ElasticsearchClientSettings();
        client.SourceSerializer.Serialize(2.3, new MemoryStream());
    }
}

Expected behavior
I expect floating-point source serialization to work.

onyxmaster commented

I'm not sure why G17 is used instead of G in DoubleWithFractionalPortionConverter, so I do not provide a PR that changes it to be so for FULLFRAMEWORK builds.

flobernd commented

Hi @onyxmaster , thanks for reporting and sorry for the late reply.

The G17 format is used to ensure round-tripping as recommended here (in the r/R row of the table).

  • 👀1
onyxmaster commented

Hi @onyxmaster , thanks for reporting and sorry for the late reply.

The G17 format is used to ensure round-tripping as recommended here (in the r/R row of the table).

No worries, I get that the driver takes a lot of effort.
Thanks for the explanation and for the fix.