CBOR does not handle canonical NaN correctly

[vcsjones]

Description

Since System.Formats.Cbor adheres to RFC 7049, I don't believe NaN is handled correctly.

The RFC states for canonical CBOR:

Also, there are many representations for NaN. If NaN is an allowed value, it must always be represented as 0xf97e00.

CborWriter will encode it as 0xF9FE00.

Similarly, CborReader will not throw for any non-canonically encoded NaN.

Reproduction Steps

To reproduce the CborWriter issue:

using System;
using System.Formats.Cbor;

CborWriter writer = new(CborConformanceMode.Canonical);
writer.WriteSingle(float.NaN);
Console.WriteLine(Convert.ToHexString(writer.Encode())); // Outputs F9FE00

---

To reproduce the CborReader issue:

byte[] canonical = Convert.FromHexString("F97E00");
byte[] nonCanonical = Convert.FromHexString("F9FE00");
CborReader canonicalReader = new CborReader(canonical, CborConformanceMode.Canonical);
CborReader nonCanonicalReader = new CborReader(nonCanonical, CborConformanceMode.Canonical);
Console.WriteLine(float.IsNaN(canonicalReader.ReadSingle()));
Console.WriteLine(float.IsNaN(nonCanonicalReader.ReadSingle()));

Expected behavior

CborWriter encodes NaN as 0xf97e00.

---

CborReader should throw when decoding a NaN that is non-canonically encoded.

Actual behavior

CborWriter incorrectly encodes NaN as 0xF9FE00 when in canonical encoding.

---

CborReader incorrectly permits all encodings of NaN when in canonical encoding.

Regression?

No.

Known Workarounds

No response

Configuration

Host:
Version: 8.0.0-rc.1.23419.4
Architecture: arm64
Commit: 9295993
RID: osx-arm64

Using 7.0.0 of System.Formats.Cbor package.

Other information

No response

[ghost]

Tagging subscribers to this area: @dotnet/area-system-formats-cbor, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

Since System.Formats.Cbor adheres to RFC 7049, I don't believe NaN is handled correctly.

The RFC states for canonical CBOR:

Also, there are many representations for NaN. If NaN is an allowed value, it must always be represented as 0xf97e00.

CborWriter will encode it as 0xF9FE00.

Similarly, CborReader will not throw for any non-canonically encoded NaN.

Reproduction Steps

To reproduce the CborWriter issue:

using System;
using System.Formats.Cbor;

CborWriter writer = new(CborConformanceMode.Canonical);
writer.WriteSingle(float.NaN);
Console.WriteLine(Convert.ToHexString(writer.Encode())); // Outputs F9FE00

---

To reproduce the CborReader issue:

byte[] canonical = Convert.FromHexString("F97E00");
byte[] nonCanonical = Convert.FromHexString("F9FE00");
CborReader canonicalReader = new CborReader(canonical, CborConformanceMode.Canonical);
CborReader nonCanonicalReader = new CborReader(nonCanonical, CborConformanceMode.Canonical);
Console.WriteLine(float.IsNaN(canonicalReader.ReadSingle()));
Console.WriteLine(float.IsNaN(nonCanonicalReader.ReadSingle()));

Expected behavior

CborWriter encodes NaN as 0xf97e00.

---

CborReader should throw when decoding a NaN that is non-canonically encoded.

Actual behavior

CborWriter incorrectly encodes NaN as 0xF9FE00 when in canonical encoding.

---

CborReader incorrectly permits all encodings of NaN when in canonical encoding.

Regression?

No.

Known Workarounds

No response

Configuration

Host:
Version: 8.0.0-rc.1.23419.4
Architecture: arm64
Commit: 9295993
RID: osx-arm64

Using 7.0.0 of System.Formats.Cbor package.

Other information

No response

Author:	vcsjones
Assignees:	-
Labels:	area-System.Formats.Cbor
Milestone:	-

[vcsjones]

The issue with CborWriter may actually be an issue with the definition of Half.NaN.

I would expect this to print the same thing:

Half h = Half.Zero / Half.Zero;
Console.WriteLine(BitConverter.HalfToInt16Bits(h).ToString("X"));
Console.WriteLine(BitConverter.HalfToInt16Bits(Half.NaN).ToString("X"));

But it prints

7E00
FE00

It seems that Half.NaN is negative NaN:

runtime/src/libraries/System.Private.CoreLib/src/System/Half.cs

Line 93 in c84ccfa

public static Half NaN => new Half(NegativeQNaNBits); // 0.0 / 0.0

[vcsjones]

@tannergooding do you have any thoughts on the NaN definition of Half here? Should that be PositiveQNaNBits?

[bartonjs]

Since I believe this is limited to System.Single/float, I updated the title. Half, of course, might also be impacted... and if it's also double then I've clearly mistitled the issue :)

[vcsjones]

@bartonjs I believe all of them are impacted since all of them get reduced to a 16-bit NaN, or Half.

CborWriter writer = new(CborConformanceMode.Canonical);
writer.WriteHalf(Half.NaN);
Console.WriteLine(Convert.ToHexString(writer.Encode())); // Writes F9FE00
writer.Reset();
writer.WriteSingle(float.NaN);
Console.WriteLine(Convert.ToHexString(writer.Encode())); // Writes F9FE00
writer.Reset();
writer.WriteDouble(double.NaN);
Console.WriteLine(Convert.ToHexString(writer.Encode())); // Writes F9FE00
writer.Reset();

[tannergooding]

@vcsjones, the "canonical NaN" in hardware is implementation dependent.

For xarch, it has historically been -QNaN and so that's what many systems default to and correspondingly what Half.NaN defaults to as well.

But, the represented value of Half.NaN or float.NaN shouldn't matter. CBOR should simply be doing if (float.IsNaN(x)) { WriteRequiredBitPattern_f97e00(); } (or equivalent for Half/double

[bartonjs]

Huh. I don't know of turning all three into Half.NaN is right. Maybe somewhere says it's allowed. But, https://www.rfc-editor.org/rfc/rfc7049#appendix-A says

Diagnostic	Encoded
Infinity	0xf97c00
NaN	0xf97e00
-Infinity	0xf9fc00
Infinity	0xfa7f800000
NaN	0xfa7fc00000
-Infinity	0xfaff800000
Infinity	0xfb7ff0000000000000
NaN	0xfb7ff8000000000000
-Infinity	0xfbfff0000000000000

One presumes those are for "when encoded as a half / float / double"

[vcsjones]

Huh. I don't know of turning all three into Half.NaN is right

I think it is for Canonical CBOR:

If a protocol allows for IEEE floats, then additional
canonicalization rules might need to be added. One example rule
might be to have all floats start as a 64-bit float, then do a test
conversion to a 32-bit float; if the result is the same numeric
value, use the shorter value and repeat the process with a test
conversion to a 16-bit float. (This rule selects 16-bit float for
positive and negative Infinity as well.) Also, there are many
representations for NaN. If NaN is an allowed value, it must always
be represented as 0xf97e00.

[tomeksowi]

I ran into this issue when working on RISC-V CoreFX tests so I fixed it in PR #92934. Reviews welcome.