Skip to content

Commit 75f8a72

Browse files
committed
encoder: Lossless double encoding, reject NaN and Inf
Doubles were formatted with %f, which pads to six decimal places and silently loses everything beyond it: 0.1234567890123 went out as 0.123457, and both 1e-10 and 1e-300 became 0.000000. Values now use the shortest decimal representation that round-trips exactly and satisfies the specification. The specification allows only decimal point notation - "a plus or a minus, followed by any number of numeric characters, followed by a period and any number of numeric characters" - so exponent notation is avoided and a period is always present, including for whole numbers. NaN and infinities have no representation per the same section of the specification, and previously went on the wire as NaN/+Inf/-Inf, which no receiver can read as a number. They are now rejected with an error.
1 parent 1894b84 commit 75f8a72

2 files changed

Lines changed: 103 additions & 5 deletions

File tree

encode.go

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@ import (
44
"encoding/base64"
55
"fmt"
66
"io"
7+
"math"
78
"reflect"
89
"strconv"
10+
"strings"
911
"time"
1012
)
1113

@@ -138,7 +140,9 @@ func (e *StdEncoder) encodeValue(x *xmlWriter, value interface{}) error {
138140
e.encodeInteger(x, value.(int))
139141

140142
case reflect.Float64:
141-
e.encodeDouble(x, value.(float64))
143+
if err := e.encodeDouble(x, value.(float64)); err != nil {
144+
return fmt.Errorf("cannot encode double value: %w", err)
145+
}
142146

143147
case reflect.String:
144148
e.encodeString(x, value.(string))
@@ -186,8 +190,33 @@ func (e *StdEncoder) encodeInteger(x *xmlWriter, val int) {
186190
x.element("int", strconv.Itoa(val))
187191
}
188192

189-
func (e *StdEncoder) encodeDouble(x *xmlWriter, val float64) {
190-
x.element("double", fmt.Sprintf("%f", val))
193+
func (e *StdEncoder) encodeDouble(x *xmlWriter, val float64) error {
194+
// XML-RPC has no representation for these, and emitting them produces a document the
195+
// receiver cannot interpret as a number
196+
if math.IsNaN(val) {
197+
return fmt.Errorf("unsupported value NaN")
198+
}
199+
200+
if math.IsInf(val, 1) {
201+
return fmt.Errorf("unsupported value +Inf")
202+
}
203+
204+
if math.IsInf(val, -1) {
205+
return fmt.Errorf("unsupported value -Inf")
206+
}
207+
208+
// The specification allows only decimal point notation - no exponent - so 'f' is used
209+
// with a precision of -1, giving the fewest digits that still round-trip exactly
210+
formatted := strconv.FormatFloat(val, 'f', -1, 64)
211+
212+
// A whole number formats without a period, which the grammar does not allow
213+
if !strings.ContainsRune(formatted, '.') {
214+
formatted += ".0"
215+
}
216+
217+
x.element("double", formatted)
218+
219+
return nil
191220
}
192221

193222
func (e *StdEncoder) encodeBoolean(x *xmlWriter, val bool) {

encode_test.go

Lines changed: 71 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"encoding/xml"
55
"errors"
66
"fmt"
7+
"math"
8+
"strconv"
79
"strings"
810
"testing"
911
"time"
@@ -145,7 +147,7 @@ func TestStdEncoder_Encode(t *testing.T) {
145147
Int: 123,
146148
Double: float64(12345),
147149
},
148-
paramValidator: exactParamsValidator(`<param><value><int>123</int></value></param><param><value><double>12345.000000</double></value></param>`),
150+
paramValidator: exactParamsValidator(`<param><value><int>123</int></value></param><param><value><double>12345.0</double></value></param>`),
149151
},
150152
{
151153
name: "String arg - simple",
@@ -536,7 +538,7 @@ func Test_encodeMap(t *testing.T) {
536538
"<member><name>string</name><value><string>value</string></value></member>",
537539
"<member><name>int</name><value><int>42</int></value></member>",
538540
"<member><name>bool</name><value><boolean>1</boolean></value></member>",
539-
"<member><name>float</name><value><double>3.140000</double></value></member>",
541+
"<member><name>float</name><value><double>3.14</double></value></member>",
540542
},
541543
err: nil,
542544
},
@@ -660,6 +662,73 @@ func Test_encodeTime_writerErrors(t *testing.T) {
660662
}
661663
}
662664

665+
func Test_encodeDouble(t *testing.T) {
666+
tests := []struct {
667+
name string
668+
input float64
669+
expect string
670+
errMsg string
671+
}{
672+
{name: "zero", input: 0, expect: "<double>0.0</double>"},
673+
{name: "negative zero", input: math.Copysign(0, -1), expect: "<double>-0.0</double>"},
674+
{name: "integral", input: 12345, expect: "<double>12345.0</double>"},
675+
{name: "negative", input: -12.214, expect: "<double>-12.214</double>"},
676+
{name: "no padding to six decimals", input: 3.14, expect: "<double>3.14</double>"},
677+
// %f used to truncate all of these to six decimal places
678+
{name: "full float64 precision", input: 3.14159265358979, expect: "<double>3.14159265358979</double>"},
679+
{name: "many significant digits", input: 0.1234567890123, expect: "<double>0.1234567890123</double>"},
680+
{name: "small magnitude", input: 1e-10, expect: "<double>0.0000000001</double>"},
681+
{name: "large magnitude", input: 1e20, expect: "<double>100000000000000000000.0</double>"},
682+
{name: "no exponent notation", input: 1e21, expect: "<double>1000000000000000000000.0</double>"},
683+
{name: "NaN is rejected", input: math.NaN(), errMsg: "unsupported value NaN"},
684+
{name: "positive infinity is rejected", input: math.Inf(1), errMsg: "unsupported value +Inf"},
685+
{name: "negative infinity is rejected", input: math.Inf(-1), errMsg: "unsupported value -Inf"},
686+
}
687+
688+
for _, tt := range tests {
689+
t.Run(tt.name, func(t *testing.T) {
690+
buf := new(strings.Builder)
691+
x := newXMLWriter(buf)
692+
693+
err := (&StdEncoder{}).encodeDouble(x, tt.input)
694+
if tt.errMsg != "" {
695+
require.EqualError(t, err, tt.errMsg)
696+
697+
return
698+
}
699+
700+
require.NoError(t, err)
701+
require.NoError(t, x.err)
702+
require.Equal(t, tt.expect, buf.String())
703+
})
704+
}
705+
}
706+
707+
func Test_encodeDouble_roundTrips(t *testing.T) {
708+
// Every emitted value must parse back to the exact same float64
709+
values := []float64{
710+
0, 1, -1, 0.5, 3.14159265358979, 0.1234567890123, 1e-10, 1e20, 1e21,
711+
math.SmallestNonzeroFloat64, math.MaxFloat64, -math.MaxFloat64,
712+
}
713+
714+
for _, want := range values {
715+
t.Run(strconv.FormatFloat(want, 'g', -1, 64), func(t *testing.T) {
716+
buf := new(strings.Builder)
717+
x := newXMLWriter(buf)
718+
require.NoError(t, (&StdEncoder{}).encodeDouble(x, want))
719+
720+
wire := strings.TrimSuffix(strings.TrimPrefix(buf.String(), "<double>"), "</double>")
721+
got, err := strconv.ParseFloat(wire, 64)
722+
require.NoError(t, err)
723+
require.Equal(t, want, got, "wire value %q must round-trip", wire)
724+
725+
// The specification permits only decimal point notation
726+
require.Contains(t, wire, ".", "wire value %q must carry a decimal point", wire)
727+
require.NotContains(t, wire, "e", "wire value %q must not use exponent notation", wire)
728+
})
729+
}
730+
}
731+
663732
func Test_Encode_escapesCallerSuppliedNames(t *testing.T) {
664733
tests := []struct {
665734
name string

0 commit comments

Comments
 (0)