bugfix: incorrect MP_DECIMAL decoding with scale < 0

The `scale` value in `MP_DECIMAL` may be negative [1]. We need
to handle the case.

1. https://www.tarantool.io/en/doc/latest/dev_guide/internals/msgpack_extensions/#the-decimal-type

Author: oleg-jukovec

CHANGELOG.md

@@ -45,6 +45,7 @@ Versioning](http://semver.org/spec/v2.0.0.html) except to the first release.
 ### Fixed
 
 - Flaky decimal/TestSelect (#300)
+- Incorrect decoding of an MP_DECIMAL when the `scale` value is negative (#314)
 
 ## [1.12.0] - 2023-06-07
 

decimal/bcd.go

@@ -183,10 +183,10 @@ func encodeStringToBCD(buf string) ([]byte, error) {
 // ended by a 4-bit sign nibble in the least significant four bits of the final
 // byte. The scale is used (negated) as the exponent of the decimal number.
 // Note that zeroes may have a sign and/or a scale.
-func decodeStringFromBCD(bcdBuf []byte) (string, error) {
+func decodeStringFromBCD(bcdBuf []byte) (string, int, error) {
 	// Index of a byte with scale.
 	const scaleIdx = 0
-	scale := int(bcdBuf[scaleIdx])
+	scale := int(int8(bcdBuf[scaleIdx]))
 
 	// Get a BCD buffer without scale.
 	bcdBuf = bcdBuf[scaleIdx+1:]
@@ -204,10 +204,6 @@ func decodeStringFromBCD(bcdBuf []byte) (string, error) {
 
 	// Reserve bytes for dot and sign.
 	numLen := ndigits + 2
-	// Reserve bytes for zeroes.
-	if scale >= ndigits {
-		numLen += scale - ndigits
-	}
 
 	var bld strings.Builder
 	bld.Grow(numLen)
@@ -219,26 +215,10 @@ func decodeStringFromBCD(bcdBuf []byte) (string, error) {
 		bld.WriteByte('-')
 	}
 
-	// Add missing zeroes to the left side when scale is bigger than a
-	// number of digits and a single missed zero to the right side when
-	// equal.
-	if scale > ndigits {
-		bld.WriteByte('0')
-		bld.WriteByte('.')
-		for diff := scale - ndigits; diff > 0; diff-- {
-			bld.WriteByte('0')
-		}
-	} else if scale == ndigits {
-		bld.WriteByte('0')
-	}
-
 	const MaxDigit = 0x09
 	// Builds a buffer with symbols of decimal number (digits, dot and sign).
 	processNibble := func(nibble byte) {
 		if nibble <= MaxDigit {
-			if ndigits == scale {
-				bld.WriteByte('.')
-			}
 			bld.WriteByte(nibble + '0')
 			ndigits--
 		}
@@ -254,5 +234,8 @@ func decodeStringFromBCD(bcdBuf []byte) (string, error) {
 		processNibble(lowNibble)
 	}
 
-	return bld.String(), nil
+	if bld.Len() == 0 {
+		bld.WriteByte('0')
+	}
+	return bld.String(), -1 * scale, nil
 }

decimal/decimal.go

@@ -87,16 +87,20 @@ func (decNum *Decimal) UnmarshalMsgpack(b []byte) error {
 	//  +--------+-------------------+------------+===============+
 	//  | MP_EXT | length (optional) | MP_DECIMAL | PackedDecimal |
 	//  +--------+-------------------+------------+===============+
-	digits, err := decodeStringFromBCD(b)
+	digits, exp, err := decodeStringFromBCD(b)
 	if err != nil {
 		return fmt.Errorf("msgpack: can't decode string from BCD buffer (%x): %w", b, err)
 	}
+
 	dec, err := decimal.NewFromString(digits)
-	*decNum = *NewDecimal(dec)
 	if err != nil {
 		return fmt.Errorf("msgpack: can't encode string (%s) to a decimal number: %w", digits, err)
 	}
 
+	if exp != 0 {
+		dec = dec.Shift(int32(exp))
+	}
+	*decNum = *NewDecimal(dec)
 	return nil
 }
 

decimal/decimal_test.go

@@ -116,6 +116,17 @@ var correctnessSamples = []struct {
 	{"-1234567891234567890.0987654321987654321", "c7150113012345678912345678900987654321987654321d", false},
 }
 
+var correctnessDecodeSamples = []struct {
+	numString string
+	mpBuf     string
+	fixExt    bool
+}{
+	{"1e2", "d501fe1c", true},
+	{"1.1e31", "c70301e2011c", false},
+	{"13e-2", "c7030102013c", false},
+	{"-1e3", "d501fd1d", true},
+}
+
 // There is a difference between encoding result from a raw string and from
 // decimal.Decimal. It's expected because decimal.Decimal simplifies decimals:
 // 0.00010000 -> 0.0001
@@ -387,18 +398,23 @@ func TestEncodeStringToBCD(t *testing.T) {
 }
 
 func TestDecodeStringFromBCD(t *testing.T) {
-	samples := append(correctnessSamples, rawSamples...)
+	samples := correctnessSamples
+	samples = append(samples, correctnessDecodeSamples...)
+	samples = append(samples, rawSamples...)
 	samples = append(samples, benchmarkSamples...)
 	for _, testcase := range samples {
 		t.Run(testcase.numString, func(t *testing.T) {
 			b, _ := hex.DecodeString(testcase.mpBuf)
 			bcdBuf := trimMPHeader(b, testcase.fixExt)
-			s, err := DecodeStringFromBCD(bcdBuf)
+			s, exp, err := DecodeStringFromBCD(bcdBuf)
 			if err != nil {
 				t.Fatalf("Failed to decode BCD '%x' to decimal: %s", bcdBuf, err)
 			}
 
 			decActual, err := decimal.NewFromString(s)
+			if exp != 0 {
+				decActual = decActual.Shift(int32(exp))
+			}
 			if err != nil {
 				t.Fatalf("Failed to msgpack.Encoder string ('%s') to decimal", s)
 			}

decimal/export_test.go

@@ -4,7 +4,7 @@ func EncodeStringToBCD(buf string) ([]byte, error) {
 	return encodeStringToBCD(buf)
 }
 
-func DecodeStringFromBCD(bcdBuf []byte) (string, error) {
+func DecodeStringFromBCD(bcdBuf []byte) (string, int, error) {
 	return decodeStringFromBCD(bcdBuf)
 }
 

decimal/fuzzing_test.go

@@ -10,11 +10,14 @@ import (
 	. "github.com/tarantool/go-tarantool/v2/decimal"
 )
 
-func strToDecimal(t *testing.T, buf string) decimal.Decimal {
+func strToDecimal(t *testing.T, buf string, exp int) decimal.Decimal {
 	decNum, err := decimal.NewFromString(buf)
 	if err != nil {
 		t.Fatal(err)
 	}
+	if exp != 0 {
+		decNum = decNum.Shift(int32(exp))
+	}
 	return decNum
 }
 
@@ -33,13 +36,13 @@ func FuzzEncodeDecodeBCD(f *testing.F) {
 		if err != nil {
 			t.Skip("Only correct requests are interesting: %w", err)
 		}
-		var dec string
-		dec, err = DecodeStringFromBCD(bcdBuf)
+
+		dec, exp, err := DecodeStringFromBCD(bcdBuf)
 		if err != nil {
 			t.Fatalf("Failed to decode encoded value ('%s')", orig)
 		}
 
-		if !strToDecimal(t, dec).Equal(strToDecimal(t, orig)) {
+		if !strToDecimal(t, dec, exp).Equal(strToDecimal(t, orig, 0)) {
 			t.Fatal("Decimal numbers are not equal")
 		}
 	})