From 45f1ca4946a53b74e392d51897315f70d896feea Mon Sep 17 00:00:00 2001
From: Milan Pavlik <milan@gitpod.io>
Date: Tue, 31 May 2022 20:08:56 +0000
Subject: [PATCH] [usage] Harden parsing of time from VarChar field

---
 components/usage/pkg/db/types.go      | 55 ++++++++++++++++++------
 components/usage/pkg/db/types_test.go | 62 +++++++++++++++++++--------
 2 files changed, 86 insertions(+), 31 deletions(-)

diff --git a/components/usage/pkg/db/types.go b/components/usage/pkg/db/types.go
index ae82286c653333..c710959f8a8a8f 100644
--- a/components/usage/pkg/db/types.go
+++ b/components/usage/pkg/db/types.go
@@ -6,54 +6,83 @@ package db
 
 import (
 	"database/sql/driver"
-	"errors"
 	"fmt"
 	"github.com/relvacode/iso8601"
 	"time"
 )
 
 func NewVarcharTime(t time.Time) VarcharTime {
-	return VarcharTime(t.UTC())
+	return VarcharTime{
+		t:     t,
+		valid: true,
+	}
 }
 
 func NewVarcharTimeFromStr(s string) (VarcharTime, error) {
-	parsed, err := iso8601.ParseString(string(s))
-	if err != nil {
-		return VarcharTime{}, fmt.Errorf("failed to parse as ISO 8601: %w", err)
-	}
-	return VarcharTime(parsed), nil
+	var vt VarcharTime
+	err := vt.Scan(s)
+	return vt, err
 }
 
 // VarcharTime exists for cases where records are inserted into the DB as VARCHAR but actually contain a timestamp which is time.RFC3339
-type VarcharTime time.Time
+type VarcharTime struct {
+	t     time.Time
+	valid bool
+}
 
 // Scan implements the Scanner interface.
 func (n *VarcharTime) Scan(value interface{}) error {
 	if value == nil {
-		return fmt.Errorf("nil value")
+		n.valid = false
+		return nil
 	}
 
 	switch s := value.(type) {
 	case []uint8:
+		// Null value - empty string mean value is not set
 		if len(s) == 0 {
-			return errors.New("failed to parse empty varchar time")
+			n.valid = false
+			return nil
 		}
 
 		parsed, err := iso8601.ParseString(string(s))
 		if err != nil {
 			return fmt.Errorf("failed to parse %v into ISO8601: %w", string(s), err)
 		}
-		*n = VarcharTime(parsed.UTC())
+		n.valid = true
+		n.t = parsed.UTC()
+		return nil
+	case string:
+		if len(s) == 0 {
+			n.valid = false
+			return nil
+		}
+
+		parsed, err := iso8601.ParseString(s)
+		if err != nil {
+			return fmt.Errorf("failed to parse %v into ISO8601: %w", s, err)
+		}
+
+		n.valid = true
+		n.t = parsed.UTC()
 		return nil
 	}
 	return fmt.Errorf("unknown scan value for VarcharTime with value: %v", value)
 }
 
+func (n VarcharTime) Time() time.Time {
+	return n.t
+}
+
+func (n VarcharTime) IsSet() bool {
+	return n.valid
+}
+
 // Value implements the driver Valuer interface.
 func (n VarcharTime) Value() (driver.Value, error) {
-	return time.Time(n).UTC().Format(time.RFC3339Nano), nil
+	return n.t.UTC().Format(time.RFC3339Nano), nil
 }
 
 func (n VarcharTime) String() string {
-	return time.Time(n).Format(time.RFC3339Nano)
+	return n.t.Format(time.RFC3339Nano)
 }
diff --git a/components/usage/pkg/db/types_test.go b/components/usage/pkg/db/types_test.go
index 20e5da2d791b1d..50fadfb3539982 100644
--- a/components/usage/pkg/db/types_test.go
+++ b/components/usage/pkg/db/types_test.go
@@ -11,43 +11,69 @@ import (
 )
 
 func TestVarcharTime_Scan(t *testing.T) {
+	type Expectation struct {
+		Time  VarcharTime
+		Error bool
+	}
+
 	for _, scenario := range []struct {
 		Name string
 
 		Input    interface{}
-		Expected time.Time
-		Error    bool
+		Expected Expectation
 	}{
 		{
-			Name:  "nil value errors",
+			Name:  "nil value does not error and sets invalid",
 			Input: nil,
-			Error: true,
+			Expected: Expectation{
+				Error: false,
+			},
 		},
 		{
-			Name:  "empty uint8 slice errors",
+			Name:  "empty uint8 slice does not error and sets invalid",
 			Input: []uint8{},
-			Error: true,
+			Expected: Expectation{
+				Error: false,
+			},
 		},
 		{
-			Name:  "fails with string",
-			Input: "2019-05-10T09:54:28.185Z",
-			Error: true,
+			Name:  "parses valid ISO 8601 from TypeScript from []uint8",
+			Input: []uint8("2019-05-10T09:54:28.185Z"),
+			Expected: Expectation{
+				Time: VarcharTime{
+					t:     time.Date(2019, 05, 10, 9, 54, 28, 185000000, time.UTC),
+					valid: true,
+				},
+				Error: false,
+			},
 		},
 		{
-			Name:     "parses valid ISO 8601 from TypeScript",
-			Input:    []uint8("2019-05-10T09:54:28.185Z"),
-			Expected: time.Date(2019, 05, 10, 9, 54, 28, 185000000, time.UTC),
+			Name:  "invalid string errors",
+			Input: "2019-05-10T09:54:28.185Z-not-a-datetime",
+			Expected: Expectation{
+				Error: true,
+			},
+		},
+		{
+			Name:  "string is parsed",
+			Input: "2019-05-10T09:54:28.185Z",
+			Expected: Expectation{
+				Time: VarcharTime{
+					t:     time.Date(2019, 05, 10, 9, 54, 28, 185000000, time.UTC),
+					valid: true,
+				},
+				Error: false,
+			},
 		},
 	} {
 		t.Run(scenario.Name, func(t *testing.T) {
 			var vt VarcharTime
 			err := vt.Scan(scenario.Input)
-			if scenario.Error {
-				require.Error(t, err)
-			} else {
-				require.NoError(t, err)
-				require.Equal(t, scenario.Expected, time.Time(vt))
-			}
+
+			require.Equal(t, scenario.Expected, Expectation{
+				Time:  vt,
+				Error: err != nil,
+			})
 		})
 	}
 }