fix #18007: std/json now serializes nan,inf,-inf as strings instead of invalid json (#18026)

* fix #18007: std/json now serializes nan,inf,-inf as raw strings instead of invalid json

* fix roundtrip

* fix tests

* fix changelog

* simplify

* add runnableExamples

* fix typo [skip ci]

changelog.md

@@ -70,6 +70,8 @@
 - `jsonutils` now serializes/deserializes holey enums as regular enums (via `ord`) instead of as strings.
   Use `-d:nimLegacyJsonutilsHoleyEnum` for a transition period.
 
+- `json` and `jsonutils` now serialize NaN, Inf, -Inf as strings, so that
+  `%[NaN, -Inf]` is the string `["nan","-inf"]` instead of `[nan,-inf]` which was invalid json.
 
 ## Standard library additions and changes
 - Added support for parenthesized expressions in `strformat`

lib/pure/json.nim

@@ -333,7 +333,16 @@ proc `%`*(n: BiggestInt): JsonNode =
 
 proc `%`*(n: float): JsonNode =
   ## Generic constructor for JSON data. Creates a new `JFloat JsonNode`.
-  result = JsonNode(kind: JFloat, fnum: n)
+  runnableExamples:
+    assert $(%[NaN, Inf, -Inf, 0.0, -0.0, 1.0, 1e-2]) == """["nan","inf","-inf",0.0,-0.0,1.0,0.01]"""
+    assert (%NaN).kind == JString
+    assert (%0.0).kind == JFloat
+  # for those special cases, we could also have used `newJRawNumber` but then
+  # it would've been inconsisten with the case of `parseJson` vs `%` for representing them.
+  if n != n: newJString("nan")
+  elif n == Inf: newJString("inf")
+  elif n == -Inf: newJString("-inf")
+  else: JsonNode(kind: JFloat, fnum: n)
 
 proc `%`*(b: bool): JsonNode =
   ## Generic constructor for JSON data. Creates a new `JBool JsonNode`.
@@ -662,6 +671,47 @@ proc escapeJson*(s: string): string =
   result = newStringOfCap(s.len + s.len shr 3)
   escapeJson(s, result)
 
+proc toUgly*(result: var string, node: JsonNode) =
+  ## Converts `node` to its JSON Representation, without
+  ## regard for human readability. Meant to improve `$` string
+  ## conversion performance.
+  ##
+  ## JSON representation is stored in the passed `result`
+  ##
+  ## This provides higher efficiency than the `pretty` procedure as it
+  ## does **not** attempt to format the resulting JSON to make it human readable.
+  var comma = false
+  case node.kind:
+  of JArray:
+    result.add "["
+    for child in node.elems:
+      if comma: result.add ","
+      else: comma = true
+      result.toUgly child
+    result.add "]"
+  of JObject:
+    result.add "{"
+    for key, value in pairs(node.fields):
+      if comma: result.add ","
+      else: comma = true
+      key.escapeJson(result)
+      result.add ":"
+      result.toUgly value
+    result.add "}"
+  of JString:
+    if node.isUnquoted:
+      result.add node.str
+    else:
+      escapeJson(node.str, result)
+  of JInt:
+    result.addInt(node.num)
+  of JFloat:
+    result.addFloat(node.fnum)
+  of JBool:
+    result.add(if node.bval: "true" else: "false")
+  of JNull:
+    result.add "null"
+
 proc toPretty(result: var string, node: JsonNode, indent = 2, ml = true,
               lstArr = false, currIndent = 0) =
   case node.kind
@@ -689,10 +739,7 @@ proc toPretty(result: var string, node: JsonNode, indent = 2, ml = true,
       result.add("{}")
   of JString:
     if lstArr: result.indent(currIndent)
-    if node.isUnquoted:
+    toUgly(result, node)
-      result.add node.str
-    else:
-      escapeJson(node.str, result)
   of JInt:
     if lstArr: result.indent(currIndent)
     result.addInt(node.num)
@@ -743,47 +790,6 @@ proc pretty*(node: JsonNode, indent = 2): string =
   result = ""
   toPretty(result, node, indent)
 
-proc toUgly*(result: var string, node: JsonNode) =
-  ## Converts `node` to its JSON Representation, without
-  ## regard for human readability. Meant to improve `$` string
-  ## conversion performance.
-  ##
-  ## JSON representation is stored in the passed `result`
-  ##
-  ## This provides higher efficiency than the `pretty` procedure as it
-  ## does **not** attempt to format the resulting JSON to make it human readable.
-  var comma = false
-  case node.kind:
-  of JArray:
-    result.add "["
-    for child in node.elems:
-      if comma: result.add ","
-      else: comma = true
-      result.toUgly child
-    result.add "]"
-  of JObject:
-    result.add "{"
-    for key, value in pairs(node.fields):
-      if comma: result.add ","
-      else: comma = true
-      key.escapeJson(result)
-      result.add ":"
-      result.toUgly value
-    result.add "}"
-  of JString:
-    if node.isUnquoted:
-      result.add node.str
-    else:
-      node.str.escapeJson(result)
-  of JInt:
-    result.addInt(node.num)
-  of JFloat:
-    result.addFloat(node.fnum)
-  of JBool:
-    result.add(if node.bval: "true" else: "false")
-  of JNull:
-    result.add "null"
-
 proc `$`*(node: JsonNode): string =
   ## Converts `node` to its JSON Representation on one line.
   result = newStringOfCap(node.len shl 1)
@@ -1087,11 +1093,26 @@ when defined(nimFixedForwardGeneric):
       dst = cast[T](jsonNode.num)
 
   proc initFromJson[T: SomeFloat](dst: var T; jsonNode: JsonNode; jsonPath: var string) =
-    verifyJsonKind(jsonNode, {JInt, JFloat}, jsonPath)
+    if jsonNode.kind == JString:
-    if jsonNode.kind == JFloat:
+      case jsonNode.str
-      dst = T(jsonNode.fnum)
+      of "nan":
+        let b = NaN
+        dst = T(b)
+        # dst = NaN # would fail some tests because range conversions would cause CT error
+        # in some cases; but this is not a hot-spot inside this branch and backend can optimize this.
+      of "inf":
+        let b = Inf
+        dst = T(b)
+      of "-inf":
+        let b = -Inf
+        dst = T(b)
+      else: raise newException(JsonKindError, "expected 'nan|inf|-inf', got " & jsonNode.str)
     else:
-      dst = T(jsonNode.num)
+      verifyJsonKind(jsonNode, {JInt, JFloat}, jsonPath)
+      if jsonNode.kind == JFloat:
+        dst = T(jsonNode.fnum)
+      else:
+        dst = T(jsonNode.num)
 
   proc initFromJson[T: enum](dst: var T; jsonNode: JsonNode; jsonPath: var string) =
     verifyJsonKind(jsonNode, {JString}, jsonPath)

lib/std/jsonutils.nim

@@ -12,6 +12,9 @@ runnableExamples:
   let a = (1.5'f32, (b: "b2", a: "a2"), 'x', @[Foo(t: true, z1: -3), nil], [{"name": "John"}.newStringTable])
   let j = a.toJson
   assert j.jsonTo(typeof(a)).toJson == j
+  assert $[NaN, Inf, -Inf, 0.0, -0.0, 1.0, 1e-2].toJson == """["nan","inf","-inf",0.0,-0.0,1.0,0.01]"""
+  assert 0.0.toJson.kind == JFloat
+  assert Inf.toJson.kind == JString
 
 import json, strutils, tables, sets, strtabs, options
 

tests/stdlib/tjson.nim

@@ -8,14 +8,27 @@ Note: Macro tests are in tests/stdlib/tjsonmacro.nim
 ]#
 
 import std/[json,parsejson,strutils]
+from std/math import isNaN
 when not defined(js):
   import std/streams
 
 proc testRoundtrip[T](t: T, expected: string) =
+  # checks that `T => json => T2 => json2` is such that json2 = json
   let j = %t
   doAssert $j == expected, $j
   doAssert %(j.to(T)) == j
 
+proc testRoundtripVal[T](t: T, expected: string) =
+  # similar to testRoundtrip, but also checks that the `T => json => T2` is such that `T2 == T`
+  # note that this isn't always possible, e.g. for pointer-like types or nans
+  let j = %t
+  doAssert $j == expected, $j
+  let j2 = ($j).parseJson
+  doAssert $j2 == expected, $(j2, t)
+  let t2 = j2.to(T)
+  doAssert t2 == t
+  doAssert $(%* t2) == expected # sanity check, because -0.0 = 0.0 but their json representation differs
+
 let testJson = parseJson"""{ "a": [1, 2, 3, 4], "b": "asd", "c": "\ud83c\udf83", "d": "\u00E6"}"""
 # nil passthrough
 doAssert(testJson{"doesnt_exist"}{"anything"}.isNil)
@@ -301,6 +314,16 @@ block: # bug #17383
     testRoundtrip(int64.high): "9223372036854775807"
     testRoundtrip(uint64.high): "18446744073709551615"
 
+block: # bug #18007
+  testRoundtrip([NaN, Inf, -Inf, 0.0, -0.0, 1.0]): """["nan","inf","-inf",0.0,-0.0,1.0]"""
+  # pending https://github.com/nim-lang/Nim/issues/18025 use:
+  # testRoundtrip([float32(NaN), Inf, -Inf, 0.0, -0.0, 1.0])
+  let inf = float32(Inf)
+  testRoundtrip([float32(NaN), inf, -inf, 0.0, -0.0, 1.0]): """["nan","inf","-inf",0.0,-0.0,1.0]"""
+  when not defined(js): # because of Infinity vs inf
+    testRoundtripVal([inf, -inf, 0.0, -0.0, 1.0]): """["inf","-inf",0.0,-0.0,1.0]"""
+  let a = parseJson($(%NaN)).to(float)
+  doAssert a.isNaN
 
 block:
   let a = "18446744073709551615"

tests/stdlib/tjsonutils.nim

@@ -4,15 +4,28 @@ discard """
 
 import std/jsonutils
 import std/json
+from std/math import isNaN
 
 proc testRoundtrip[T](t: T, expected: string) =
+  # checks that `T => json => T2 => json2` is such that json2 = json
   let j = t.toJson
-  doAssert $j == expected, $j
+  doAssert $j == expected, "\n" & $j & "\n" & expected
   doAssert j.jsonTo(T).toJson == j
   var t2: T
   t2.fromJson(j)
   doAssert t2.toJson == j
 
+proc testRoundtripVal[T](t: T, expected: string) =
+  # similar to testRoundtrip, but also checks that the `T => json => T2` is such that `T2 == T`
+  # note that this isn't always possible, e.g. for pointer-like types.
+  let j = t.toJson
+  let j2 = $j
+  doAssert j2 == expected, j2
+  let j3 = j2.parseJson
+  let t2 = j3.jsonTo(T)
+  doAssert t2 == t
+  doAssert $t2.toJson == j2 # still needed, because -0.0 = 0.0 but their json representation differs
+
 import tables, sets, algorithm, sequtils, options, strtabs
 from strutils import contains
 
@@ -91,6 +104,12 @@ template fn() =
       else:
         testRoundtrip(a): "[9223372036854775807,18446744073709551615]"
 
+  block: # bug #18007
+    testRoundtrip((NaN, Inf, -Inf, 0.0, -0.0, 1.0)): """["nan","inf","-inf",0.0,-0.0,1.0]"""
+    testRoundtrip((float32(NaN), Inf, -Inf, 0.0, -0.0, 1.0)): """["nan","inf","-inf",0.0,-0.0,1.0]"""
+    testRoundtripVal((Inf, -Inf, 0.0, -0.0, 1.0)): """["inf","-inf",0.0,-0.0,1.0]"""
+    doAssert ($NaN.toJson).parseJson.jsonTo(float).isNaN
+
   block: # case object
     type Foo = object
       x0: float