fix: prevent ParseString overflow and stabilize fuzz round-trip

Author: cagrico

.github/workflows/ci.yml

@@ -17,4 +17,12 @@ jobs:
           go-version: "1.22"
 
       - run: go test ./... -count=1
-      - run: go vet ./...
+      - run: go vet ./...
+      - run: go test ./... -count=1 -race
+      - run: go test ./... -count=1 -cover
+      - run: go test -bench=. -benchmem ./... # opsiyonel (PR'da biraz yavaşlatır)
+
+      - name: Fuzz (short)
+        run: go test ./... -fuzz=FuzzParseString -fuzztime=10s
+
+

alloc_property_test.go

@@ -0,0 +1,58 @@
+package money_test
+
+import (
+	"math/rand"
+	"testing"
+	"time"
+
+	"github.com/dahaiyiyimcom/money"
+)
+
+func TestAllocateProportional_Property_SumExact(t *testing.T) {
+	r := rand.New(rand.NewSource(time.Now().UnixNano()))
+
+	for iter := 0; iter < 5000; iter++ {
+		n := r.Intn(15) // 0..14
+		bases := make([]money.Amount, n)
+
+		var total int64
+		for i := 0; i < n; i++ {
+			// Keep bases mostly positive; include some zeros
+			v := int64(r.Intn(50000)) // up to 500.00
+			bases[i] = money.NewMinor(v)
+			total += v
+		}
+
+		// Choose discount in [0, total] (if total==0 => 0)
+		var d int64
+		if total > 0 {
+			d = int64(r.Intn(int(total + 1)))
+		}
+		discount := money.NewMinor(d)
+
+		out := money.AllocateProportional(bases, discount)
+
+		// Invariant 1: output length matches input length
+		if len(out) != len(bases) {
+			t.Fatalf("len(out)=%d len(bases)=%d", len(out), len(bases))
+		}
+
+		// Invariant 2: sum(out) == discount
+		var sum int64
+		for _, x := range out {
+			sum += x.Minor()
+		}
+		if sum != discount.Minor() {
+			t.Fatalf("sum(out)=%d discount=%d bases=%v out=%v", sum, discount.Minor(), bases, out)
+		}
+
+		// Optional invariant 3: if discount==0 -> all zeros
+		if discount.Minor() == 0 {
+			for i, x := range out {
+				if x.Minor() != 0 {
+					t.Fatalf("expected all zeros when discount=0; i=%d got=%d", i, x.Minor())
+				}
+			}
+		}
+	}
+}

alloc_test.go

@@ -71,3 +71,21 @@ func TestAllocateProportional_EmptyOrZero(t *testing.T) {
 		t.Fatalf("expected [0], got=%v", got)
 	}
 }
+
+func TestAllocateProportional_WithNegativeBase(t *testing.T) {
+	bases := []money.Amount{
+		money.NewMinor(-1000),
+		money.NewMinor(2000),
+	}
+	discount := money.NewMinor(100)
+
+	out := money.AllocateProportional(bases, discount)
+
+	var sum int64
+	for _, v := range out {
+		sum += v.Minor()
+	}
+	if sum != 100 {
+		t.Fatalf("sum got=%d want=100", sum)
+	}
+}

amount_test.go

@@ -43,3 +43,15 @@ func TestAmount_Arithmetic(t *testing.T) {
 		t.Fatalf("MulQty got=%d want=3702", got)
 	}
 }
+
+func TestAmount_IsNegative(t *testing.T) {
+	if money.NewMinor(0).IsNegative() {
+		t.Fatalf("0 should not be negative")
+	}
+	if money.NewMinor(1).IsNegative() {
+		t.Fatalf("positive should not be negative")
+	}
+	if !money.NewMinor(-1).IsNegative() {
+		t.Fatalf("negative should be negative")
+	}
+}

bench_test.go

@@ -0,0 +1,67 @@
+package money_test
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/dahaiyiyimcom/money"
+)
+
+func BenchmarkParseString(b *testing.B) {
+	in := "419.29"
+	b.ReportAllocs()
+	b.ResetTimer()
+
+	for i := 0; i < b.N; i++ {
+		_, _ = money.ParseString(in)
+	}
+}
+
+func BenchmarkStringFixed2(b *testing.B) {
+	a := money.NewMinor(41929)
+	b.ReportAllocs()
+	b.ResetTimer()
+
+	for i := 0; i < b.N; i++ {
+		_ = a.StringFixed2()
+	}
+}
+
+func BenchmarkJSONMarshalAmount(b *testing.B) {
+	type DTO struct {
+		Price money.Amount `json:"price"`
+	}
+	dto := DTO{Price: money.NewMinor(41929)}
+
+	b.ReportAllocs()
+	b.ResetTimer()
+
+	for i := 0; i < b.N; i++ {
+		_, _ = json.Marshal(dto)
+	}
+}
+
+func BenchmarkAllocateProportional(b *testing.B) {
+	bases := make([]money.Amount, 50)
+	for i := 0; i < len(bases); i++ {
+		bases[i] = money.NewMinor(int64(1000 + i*3))
+	}
+	discount := money.NewMinor(1234)
+
+	b.ReportAllocs()
+	b.ResetTimer()
+
+	for i := 0; i < b.N; i++ {
+		_ = money.AllocateProportional(bases, discount)
+	}
+}
+
+func BenchmarkMulRatio(b *testing.B) {
+	a := money.NewMinor(1234567) // 12,345.67
+	b.ReportAllocs()
+	b.ResetTimer()
+
+	for i := 0; i < b.N; i++ {
+		_ = a.MulRatio(18, 100, money.RoundHalfUp)
+	}
+}

db.go

@@ -2,13 +2,10 @@ package money
 
 import (
 	"database/sql/driver"
-	"errors"
 	"fmt"
-	"strings"
+	"strconv"
 )
 
-// DBAmount is a DB-facing wrapper for DECIMAL(10,2) columns.
-// Use it in structs scanned from sql rows.
 type DBAmount struct {
 	A Amount
 }
@@ -19,100 +16,53 @@ func (m *DBAmount) Scan(value any) error {
 		return nil
 	}
 
-	var s string
 	switch v := value.(type) {
 	case []byte:
-		s = string(v)
-	case string:
-		s = v
-	default:
-		return fmt.Errorf("money: unsupported scan type %T", value)
-	}
-
-	a, err := ParseString(s)
-	if err != nil {
-		return err
-	}
-	m.A = a
-	return nil
-}
-
-func (m DBAmount) Value() (driver.Value, error) {
-	// write back as "12.34" for DECIMAL(10,2)
-	return m.A.StringFixed2(), nil
-}
+		a, err := ParseString(string(v))
+		if err != nil {
+			return err
+		}
+		m.A = a
+		return nil
 
-// ParseString parses decimal string with optional sign, 2 decimals max.
-// Accepts: "12.34", "12", "-0.50", "  12.30 "
-func ParseString(s string) (Amount, error) {
-	s = strings.TrimSpace(s)
-	if s == "" {
-		return 0, errors.New("money: empty string")
-	}
+	case string:
+		a, err := ParseString(v)
+		if err != nil {
+			return err
+		}
+		m.A = a
+		return nil
 
-	sign := int64(1)
-	if s[0] == '-' {
-		sign = -1
-		s = s[1:]
-	} else if s[0] == '+' {
-		s = s[1:]
-	}
+	case float64:
+		// Force 2 decimals, then parse strictly.
+		s := strconv.FormatFloat(v, 'f', 2, 64)
+		a, err := ParseString(s)
+		if err != nil {
+			return err
+		}
+		m.A = a
+		return nil
 
-	parts := strings.Split(s, ".")
-	if len(parts) > 2 {
-		return 0, fmt.Errorf("money: invalid format: %q", s)
-	}
+	case float32:
+		s := strconv.FormatFloat(float64(v), 'f', 2, 64)
+		a, err := ParseString(s)
+		if err != nil {
+			return err
+		}
+		m.A = a
+		return nil
 
-	wholeStr := parts[0]
-	if wholeStr == "" {
-		wholeStr = "0"
-	}
-	var fracStr string
-	if len(parts) == 2 {
-		fracStr = parts[1]
-	}
+	case int64:
+		// If driver returns integer, assume it's already minor? (ambiguous)
+		// Better to treat as major units without decimals is dangerous.
+		// If you WANT to support this, define it clearly. For safety, reject:
+		return fmt.Errorf("money: unsupported scan int64=%d (ambiguous units)", v)
 
-	// normalize fraction to 2 digits
-	switch len(fracStr) {
-	case 0:
-		fracStr = "00"
-	case 1:
-		fracStr = fracStr + "0"
-	case 2:
-		// ok
 	default:
-		// If more than 2 decimals exist, you can either reject or round.
-		// For strict DECIMAL(10,2), reject is safest.
-		return 0, fmt.Errorf("money: too many decimal places: %q", s)
-	}
-
-	whole, err := parseUint(wholeStr)
-	if err != nil {
-		return 0, fmt.Errorf("money: invalid whole part: %w", err)
-	}
-	frac, err := parseUint(fracStr)
-	if err != nil {
-		return 0, fmt.Errorf("money: invalid fractional part: %w", err)
-	}
-	if frac > 99 {
-		return 0, fmt.Errorf("money: invalid fractional part: %q", fracStr)
+		return fmt.Errorf("money: unsupported scan type %T", value)
 	}
-
-	minor := int64(whole)*100 + int64(frac)
-	return Amount(sign * minor), nil
 }
 
-func parseUint(s string) (uint64, error) {
-	if s == "" {
-		return 0, errors.New("empty")
-	}
-	var n uint64
-	for i := 0; i < len(s); i++ {
-		c := s[i]
-		if c < '0' || c > '9' {
-			return 0, fmt.Errorf("non-digit %q", c)
-		}
-		n = n*10 + uint64(c-'0')
-	}
-	return n, nil
+func (m DBAmount) Value() (driver.Value, error) {
+	return m.A.StringFixed2(), nil
 }