Ensure NewForHLC returns an error if the incoming decimal is invalid

This shouldn't be possible, but there are some indications it might be
authzed · Jan 29, 2024 · 37085c4 · 37085c4
1 parent 58df026
commit 37085c4
Show file tree

Hide file tree

Showing 5 changed files with 23 additions and 11 deletions.
diff --git a/internal/datastore/crdb/crdb.go b/internal/datastore/crdb/crdb.go
@@ -478,7 +478,7 @@ func readCRDBNow(ctx context.Context, reader pgxcommon.DBFuncQuerier) (datastore
 		return datastore.NoRevision, fmt.Errorf("unable to read timestamp: %w", err)
 	}
 
-	return revisions.NewForHLC(hlcNow), nil
+	return revisions.NewForHLC(hlcNow)
 }
 
 func readClusterTTLNanos(ctx context.Context, conn pgxcommon.DBFuncQuerier) (int64, error) {

diff --git a/internal/datastore/crdb/stats.go b/internal/datastore/crdb/stats.go
@@ -105,5 +105,5 @@ func updateCounter(ctx context.Context, tx pgx.Tx, change int64) (datastore.Revi
 		return datastore.NoRevision, fmt.Errorf("unable to executed upsert counter query: %w", err)
 	}
 
-	return revisions.NewForHLC(timestamp), nil
+	return revisions.NewForHLC(timestamp)
 }
diff --git a/internal/datastore/revisions/hlcrevision.go b/internal/datastore/revisions/hlcrevision.go
@@ -73,9 +73,8 @@ func HLCRevisionFromString(revisionStr string) (HLCRevision, error) {
 }
 
 // NewForHLC creates a new revision for the given hybrid logical clock.
-func NewForHLC(decimal decimal.Decimal) HLCRevision {
-	rev, _ := HLCRevisionFromString(decimal.String())
-	return rev
+func NewForHLC(decimal decimal.Decimal) (HLCRevision, error) {
+	return HLCRevisionFromString(decimal.String())
 }
 
 // NewHLCForTime creates a new revision for the given time.

diff --git a/internal/datastore/revisions/hlcrevision_test.go b/internal/datastore/revisions/hlcrevision_test.go
@@ -24,7 +24,8 @@ func TestNewForHLC(t *testing.T) {
 			d, err := decimal.NewFromString(tc)
 			require.NoError(t, err)
 
-			rev := NewForHLC(d)
+			rev, err := NewForHLC(d)
+			require.NoError(t, err)
 			require.Equal(t, tc, rev.String())
 		})
 	}

diff --git a/pkg/zedtoken/zedtoken_test.go b/pkg/zedtoken/zedtoken_test.go
@@ -166,10 +166,16 @@ var hlcDecodeTests = []struct {
 	expectError      bool
 }{
 	{
-		format:           "V1 ZedToken",
-		token:            "CAIaFQoTMTYyMTUzODE4OTAyODkyODAwMA==",
-		expectedRevision: revisions.NewForHLC(decimal.NewFromInt(1621538189028928000)),
-		expectError:      false,
+		format: "V1 ZedToken",
+		token:  "CAIaFQoTMTYyMTUzODE4OTAyODkyODAwMA==",
+		expectedRevision: func() datastore.Revision {
+			r, err := revisions.NewForHLC(decimal.NewFromInt(1621538189028928000))
+			if err != nil {
+				panic(err)
+			}
+			return r
+		}(),
+		expectError: false,
 	},
 	{
 		format: "V1 ZedToken",
@@ -179,7 +185,13 @@ var hlcDecodeTests = []struct {
 			if err != nil {
 				panic(err)
 			}
-			return revisions.NewForHLC(v)
+
+			r, err := revisions.NewForHLC(v)
+			if err != nil {
+				panic(err)
+			}
+
+			return r
 		})(),
 		expectError: false,
 	},