From 98e56003418223162290ff63f228deeb228ea5d6 Mon Sep 17 00:00:00 2001 From: alhudz Date: Wed, 10 Jun 2026 11:01:59 +0530 Subject: [PATCH 1/5] reject non-rfc3339 timestamp strings in timestamp() conversion --- common/types/string.go | 11 +++++++---- common/types/string_test.go | 29 +++++++++++++++++++++++++++++ common/types/timestamp.go | 10 ++++++++++ 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/common/types/string.go b/common/types/string.go index 5f5a4335..5bdb294b 100644 --- a/common/types/string.go +++ b/common/types/string.go @@ -122,11 +122,14 @@ func (s String) ConvertToType(typeVal ref.Type) ref.Val { return durationOf(d) } case TimestampType: - if t, err := time.Parse(time.RFC3339, s.Value().(string)); err == nil { - if t.Unix() < minUnixTime || t.Unix() > maxUnixTime { - return celErrTimestampOverflow + str := s.Value().(string) + if strictRFC3339Pattern.MatchString(str) { + if t, err := time.Parse(time.RFC3339, str); err == nil { + if t.Unix() < minUnixTime || t.Unix() > maxUnixTime { + return celErrTimestampOverflow + } + return timestampOf(t) } - return timestampOf(t) } case StringType: return s diff --git a/common/types/string_test.go b/common/types/string_test.go index 158f2bb7..af2a9a06 100644 --- a/common/types/string_test.go +++ b/common/types/string_test.go @@ -163,6 +163,35 @@ func TestStringConvertToType(t *testing.T) { } } +func TestStringConvertToTimestampStrict(t *testing.T) { + valid := []string{ + "2025-01-17T01:00:00.001Z", + "2025-01-01T12:34:56Z", + "2025-01-01T12:34:56.123456789Z", + "2025-01-01T12:34:56+05:30", + "2025-01-01T12:34:56-08:00", + "2025-01-01T12:34:56+14:00", + } + for _, s := range valid { + if IsError(String(s).ConvertToType(TimestampType)) { + t.Errorf("String(%q).ConvertToType(TimestampType) errored, wanted a timestamp", s) + } + } + // RFC 3339 violations that time.Parse accepts loosely. + invalid := []string{ + "2025-01-17T01:00:00,001Z", // ',' fractional separator + "2025-01-17T1:00:00Z", // single-digit hour + "2025-01-17T01:5:00Z", // single-digit minute + "2025-01-18T01:01:01.001+24:01", // offset hour out of range + "2025-01-17T01:01:01.001+00:60", // offset minute out of range + } + for _, s := range invalid { + if !IsError(String(s).ConvertToType(TimestampType)) { + t.Errorf("String(%q).ConvertToType(TimestampType) succeeded, wanted an error", s) + } + } +} + func TestStringEqual(t *testing.T) { if !String("hello").Equal(String("hello")).(Bool) { t.Error("Two equivalent strings were not equal") diff --git a/common/types/timestamp.go b/common/types/timestamp.go index 060caf6b..9c6dfa1d 100644 --- a/common/types/timestamp.go +++ b/common/types/timestamp.go @@ -17,6 +17,7 @@ package types import ( "fmt" "reflect" + "regexp" "strconv" "strings" "time" @@ -52,6 +53,15 @@ const ( maxUnixTime int64 = 253402300799 ) +// strictRFC3339Pattern gates the strings accepted by the `timestamp()` overload. +// time.Parse accepts inputs that RFC 3339 forbids: a ',' fractional-second +// separator, single-digit time fields, and numeric offsets whose hours exceed +// 23 or minutes exceed 59. Those slip past unnoticed and shift the parsed +// instant, so they are rejected before time.Parse runs. The remaining calendar +// validation (month, day, leap year) is left to time.Parse. +var strictRFC3339Pattern = regexp.MustCompile( + `^\d{4}-\d{2}-\d{2}[Tt]([01]\d|2[0-3]):[0-5]\d:([0-5]\d|60)(\.\d+)?([Zz]|[+-]([01]\d|2[0-3]):[0-5]\d)$`) + // Add implements traits.Adder.Add. func (t Timestamp) Add(other ref.Val) ref.Val { switch other.Type() { From a6f83c32b6f685379b268d94c60b1a4f09a8181c Mon Sep 17 00:00:00 2001 From: alhudz Date: Fri, 12 Jun 2026 09:57:01 +0530 Subject: [PATCH 2/5] return early with a specific error when the timestamp format is invalid --- common/types/string.go | 13 +++++++------ common/types/string_test.go | 9 ++++++++- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/common/types/string.go b/common/types/string.go index 5bdb294b..c4847f7a 100644 --- a/common/types/string.go +++ b/common/types/string.go @@ -123,13 +123,14 @@ func (s String) ConvertToType(typeVal ref.Type) ref.Val { } case TimestampType: str := s.Value().(string) - if strictRFC3339Pattern.MatchString(str) { - if t, err := time.Parse(time.RFC3339, str); err == nil { - if t.Unix() < minUnixTime || t.Unix() > maxUnixTime { - return celErrTimestampOverflow - } - return timestampOf(t) + if !strictRFC3339Pattern.MatchString(str) { + return NewErr("invalid RFC 3339 timestamp %q", str) + } + if t, err := time.Parse(time.RFC3339, str); err == nil { + if t.Unix() < minUnixTime || t.Unix() > maxUnixTime { + return celErrTimestampOverflow } + return timestampOf(t) } case StringType: return s diff --git a/common/types/string_test.go b/common/types/string_test.go index af2a9a06..664e3990 100644 --- a/common/types/string_test.go +++ b/common/types/string_test.go @@ -15,6 +15,7 @@ package types import ( + "fmt" "reflect" "testing" "time" @@ -186,8 +187,14 @@ func TestStringConvertToTimestampStrict(t *testing.T) { "2025-01-17T01:01:01.001+00:60", // offset minute out of range } for _, s := range invalid { - if !IsError(String(s).ConvertToType(TimestampType)) { + out := String(s).ConvertToType(TimestampType) + if !IsError(out) { t.Errorf("String(%q).ConvertToType(TimestampType) succeeded, wanted an error", s) + continue + } + want := fmt.Sprintf("invalid RFC 3339 timestamp %q", s) + if got := out.(*Err).String(); got != want { + t.Errorf("String(%q).ConvertToType(TimestampType) errored with %q, wanted %q", s, got, want) } } } From ea57bf4e2939b3ccb3bcba5662df57dbd4a0561e Mon Sep 17 00:00:00 2001 From: alhudz Date: Thu, 18 Jun 2026 18:05:50 +0530 Subject: [PATCH 3/5] add benchmark for timestamp() string conversion --- common/types/string_test.go | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/common/types/string_test.go b/common/types/string_test.go index 664e3990..daf8fdc5 100644 --- a/common/types/string_test.go +++ b/common/types/string_test.go @@ -180,11 +180,11 @@ func TestStringConvertToTimestampStrict(t *testing.T) { } // RFC 3339 violations that time.Parse accepts loosely. invalid := []string{ - "2025-01-17T01:00:00,001Z", // ',' fractional separator - "2025-01-17T1:00:00Z", // single-digit hour - "2025-01-17T01:5:00Z", // single-digit minute - "2025-01-18T01:01:01.001+24:01", // offset hour out of range - "2025-01-17T01:01:01.001+00:60", // offset minute out of range + "2025-01-17T01:00:00,001Z", // ',' fractional separator + "2025-01-17T1:00:00Z", // single-digit hour + "2025-01-17T01:5:00Z", // single-digit minute + "2025-01-18T01:01:01.001+24:01", // offset hour out of range + "2025-01-17T01:01:01.001+00:60", // offset minute out of range } for _, s := range invalid { out := String(s).ConvertToType(TimestampType) @@ -199,6 +199,15 @@ func TestStringConvertToTimestampStrict(t *testing.T) { } } +func BenchmarkStringConvertToTimestamp(b *testing.B) { + s := String("2025-01-01T12:34:56.123456789Z") + for i := 0; i < b.N; i++ { + if IsError(s.ConvertToType(TimestampType)) { + b.Fatal("ConvertToType(TimestampType) errored, wanted a timestamp") + } + } +} + func TestStringEqual(t *testing.T) { if !String("hello").Equal(String("hello")).(Bool) { t.Error("Two equivalent strings were not equal") From 2e16e64134abd43cb0ab89b2d8b94067a12721e0 Mon Sep 17 00:00:00 2001 From: Alhuda Khan Date: Mon, 29 Jun 2026 13:20:22 +0530 Subject: [PATCH 4/5] replace RFC 3339 regex gate with a hand-rolled scan --- common/types/string.go | 2 +- common/types/timestamp.go | 74 ++++++++++++++++++++++++++++++++++ common/types/timestamp_test.go | 41 +++++++++++++++++++ 3 files changed, 116 insertions(+), 1 deletion(-) diff --git a/common/types/string.go b/common/types/string.go index c4847f7a..1335903a 100644 --- a/common/types/string.go +++ b/common/types/string.go @@ -123,7 +123,7 @@ func (s String) ConvertToType(typeVal ref.Type) ref.Val { } case TimestampType: str := s.Value().(string) - if !strictRFC3339Pattern.MatchString(str) { + if !isStrictRFC3339(str) { return NewErr("invalid RFC 3339 timestamp %q", str) } if t, err := time.Parse(time.RFC3339, str); err == nil { diff --git a/common/types/timestamp.go b/common/types/timestamp.go index 9c6dfa1d..aa1c5887 100644 --- a/common/types/timestamp.go +++ b/common/types/timestamp.go @@ -59,9 +59,83 @@ const ( // 23 or minutes exceed 59. Those slip past unnoticed and shift the parsed // instant, so they are rejected before time.Parse runs. The remaining calendar // validation (month, day, leap year) is left to time.Parse. +// +// isStrictRFC3339 is the implementation used on the conversion path; the pattern +// is retained as the reference the scan is conformance tested against. var strictRFC3339Pattern = regexp.MustCompile( `^\d{4}-\d{2}-\d{2}[Tt]([01]\d|2[0-3]):[0-5]\d:([0-5]\d|60)(\.\d+)?([Zz]|[+-]([01]\d|2[0-3]):[0-5]\d)$`) +// isStrictRFC3339 reports whether s matches strictRFC3339Pattern, hand-rolled to +// keep the conversion path off the regexp engine and its per-call cost. +func isStrictRFC3339(s string) bool { + // Shortest accepted form is "2006-01-02T15:04:05Z" (20 bytes): a 19-byte + // fixed-width date-time followed by at least a 'Z'/'z' zone. + if len(s) < 20 { + return false + } + // date: \d{4}-\d{2}-\d{2} + if !isDigit(s[0]) || !isDigit(s[1]) || !isDigit(s[2]) || !isDigit(s[3]) || s[4] != '-' || + !isDigit(s[5]) || !isDigit(s[6]) || s[7] != '-' || + !isDigit(s[8]) || !isDigit(s[9]) { + return false + } + // date/time separator [Tt] + if s[10] != 'T' && s[10] != 't' { + return false + } + // time: ([01]\d|2[0-3]):[0-5]\d:([0-5]\d|60) + if !isHour(s[11], s[12]) || s[13] != ':' || !isMinute(s[14], s[15]) || s[16] != ':' || !isSecond(s[17], s[18]) { + return false + } + rest := s[19:] + // optional fractional seconds (\.\d+) + if rest[0] == '.' { + rest = rest[1:] + n := 0 + for n < len(rest) && isDigit(rest[n]) { + n++ + } + if n == 0 { + return false + } + rest = rest[n:] + } + // zone: [Zz] | [+-]([01]\d|2[0-3]):[0-5]\d + if len(rest) == 1 { + return rest[0] == 'Z' || rest[0] == 'z' + } + if len(rest) == 6 && (rest[0] == '+' || rest[0] == '-') { + return isHour(rest[1], rest[2]) && rest[3] == ':' && isMinute(rest[4], rest[5]) + } + return false +} + +func isDigit(c byte) bool { return c >= '0' && c <= '9' } + +// isHour reports whether the two bytes form 00-23. +func isHour(hi, lo byte) bool { + switch hi { + case '0', '1': + return isDigit(lo) + case '2': + return lo >= '0' && lo <= '3' + } + return false +} + +// isMinute reports whether the two bytes form 00-59. +func isMinute(hi, lo byte) bool { + return hi >= '0' && hi <= '5' && isDigit(lo) +} + +// isSecond reports whether the two bytes form 00-60 (60 permits a leap second). +func isSecond(hi, lo byte) bool { + if hi == '6' { + return lo == '0' + } + return isMinute(hi, lo) +} + // Add implements traits.Adder.Add. func (t Timestamp) Add(other ref.Val) ref.Val { switch other.Type() { diff --git a/common/types/timestamp_test.go b/common/types/timestamp_test.go index a7ff89d4..53c557c6 100644 --- a/common/types/timestamp_test.go +++ b/common/types/timestamp_test.go @@ -485,3 +485,44 @@ func TestTimestampGetMilliseconds(t *testing.T) { t.Errorf("ts.getMilliseconds('America/Phoenix') got %v, wanted 1 ms", msTz) } } + +func TestIsStrictRFC3339MatchesPattern(t *testing.T) { + // Exercise the hand-rolled scan against strictRFC3339Pattern, including the + // boundary cases for each field and a few well-formed timestamps. The two + // must agree on every input. + cases := []string{ + "2025-01-01T12:34:56Z", + "2025-01-01T12:34:56z", + "2025-01-01t12:34:56Z", + "2025-01-01T12:34:56.123456789Z", + "2025-01-01T12:34:56.1Z", + "2025-01-01T00:00:00+00:00", + "2025-01-01T23:59:60-08:00", + "2025-01-01T12:34:56+14:00", + "2025-01-01T12:34:56+05:30", + "2025-01-01T20:00:00Z", + "2025-01-01T23:59:59Z", + // rejected forms + "2025-01-01T12:34:56,123Z", + "2025-01-01T1:34:56Z", + "2025-01-01T12:3:56Z", + "2025-01-01T12:34:5Z", + "2025-01-01T24:00:00Z", + "2025-01-01T12:60:00Z", + "2025-01-01T12:34:61Z", + "2025-01-01T12:34:56.Z", + "2025-01-01T12:34:56+24:00", + "2025-01-01T12:34:56+00:60", + "2025-01-01T12:34:56+0530", + "2025-01-01T12:34:56", + "2025-01-01 12:34:56Z", + "2025-1-01T12:34:56Z", + "", + "not-a-timestamp", + } + for _, s := range cases { + if got, want := isStrictRFC3339(s), strictRFC3339Pattern.MatchString(s); got != want { + t.Errorf("isStrictRFC3339(%q) = %v, strictRFC3339Pattern.MatchString = %v", s, got, want) + } + } +} From 9a9c717dd7a9002dc2a64a10d16b1d33a10c8a04 Mon Sep 17 00:00:00 2001 From: Alhuda Khan Date: Tue, 14 Jul 2026 13:43:29 +0530 Subject: [PATCH 5/5] restructure RFC 3339 scan around strconv.ParseUint field helpers --- common/types/timestamp.go | 67 +++++++++++++++------------------- common/types/timestamp_test.go | 5 +++ 2 files changed, 34 insertions(+), 38 deletions(-) diff --git a/common/types/timestamp.go b/common/types/timestamp.go index aa1c5887..8bdeab44 100644 --- a/common/types/timestamp.go +++ b/common/types/timestamp.go @@ -21,6 +21,7 @@ import ( "strconv" "strings" "time" + "unicode" "github.com/google/cel-go/common/overloads" "github.com/google/cel-go/common/types/ref" @@ -57,13 +58,14 @@ const ( // time.Parse accepts inputs that RFC 3339 forbids: a ',' fractional-second // separator, single-digit time fields, and numeric offsets whose hours exceed // 23 or minutes exceed 59. Those slip past unnoticed and shift the parsed -// instant, so they are rejected before time.Parse runs. The remaining calendar -// validation (month, day, leap year) is left to time.Parse. +// instant, so they are rejected before time.Parse runs. Month and day are held +// to the grammar ranges 01-12 and 01-31; the remaining calendar validation +// (day-of-month vs. month, leap years) is left to time.Parse. // // isStrictRFC3339 is the implementation used on the conversion path; the pattern // is retained as the reference the scan is conformance tested against. var strictRFC3339Pattern = regexp.MustCompile( - `^\d{4}-\d{2}-\d{2}[Tt]([01]\d|2[0-3]):[0-5]\d:([0-5]\d|60)(\.\d+)?([Zz]|[+-]([01]\d|2[0-3]):[0-5]\d)$`) + `^\d{4}-(0[1-9]|1[0-2])-(0[1-9]|[12]\d|3[01])[Tt]([01]\d|2[0-3]):[0-5]\d:([0-5]\d|60)(\.\d+)?([Zz]|[+-]([01]\d|2[0-3]):[0-5]\d)$`) // isStrictRFC3339 reports whether s matches strictRFC3339Pattern, hand-rolled to // keep the conversion path off the regexp engine and its per-call cost. @@ -73,22 +75,14 @@ func isStrictRFC3339(s string) bool { if len(s) < 20 { return false } - // date: \d{4}-\d{2}-\d{2} - if !isDigit(s[0]) || !isDigit(s[1]) || !isDigit(s[2]) || !isDigit(s[3]) || s[4] != '-' || - !isDigit(s[5]) || !isDigit(s[6]) || s[7] != '-' || - !isDigit(s[8]) || !isDigit(s[9]) { - return false - } - // date/time separator [Tt] - if s[10] != 'T' && s[10] != 't' { - return false - } - // time: ([01]\d|2[0-3]):[0-5]\d:([0-5]\d|60) - if !isHour(s[11], s[12]) || s[13] != ':' || !isMinute(s[14], s[15]) || s[16] != ':' || !isSecond(s[17], s[18]) { + // full-date "T" partial-time + if !isYear(s[0:4]) || !isChar(s[4], '-') || !isMonth(s[5:7]) || !isChar(s[7], '-') || !isDay(s[8:10]) || + !isChar(s[10], 't') || + !isHour(s[11:13]) || !isChar(s[13], ':') || !isMinute(s[14:16]) || !isChar(s[16], ':') || !isSecond(s[17:19]) { return false } rest := s[19:] - // optional fractional seconds (\.\d+) + // optional fractional seconds: "." 1*DIGIT if rest[0] == '.' { rest = rest[1:] n := 0 @@ -100,41 +94,38 @@ func isStrictRFC3339(s string) bool { } rest = rest[n:] } - // zone: [Zz] | [+-]([01]\d|2[0-3]):[0-5]\d + // time-offset: "Z" or ("+" / "-") time-hour ":" time-minute if len(rest) == 1 { - return rest[0] == 'Z' || rest[0] == 'z' + return isChar(rest[0], 'z') } if len(rest) == 6 && (rest[0] == '+' || rest[0] == '-') { - return isHour(rest[1], rest[2]) && rest[3] == ':' && isMinute(rest[4], rest[5]) + return isHour(rest[1:3]) && isChar(rest[3], ':') && isMinute(rest[4:6]) } return false } func isDigit(c byte) bool { return c >= '0' && c <= '9' } -// isHour reports whether the two bytes form 00-23. -func isHour(hi, lo byte) bool { - switch hi { - case '0', '1': - return isDigit(lo) - case '2': - return lo >= '0' && lo <= '3' - } - return false +// isChar reports whether got is want, case-insensitively; want must be lower case. +func isChar(got, want byte) bool { + g, w := rune(got), rune(want) + return g == w || unicode.ToLower(g) == w } -// isMinute reports whether the two bytes form 00-59. -func isMinute(hi, lo byte) bool { - return hi >= '0' && hi <= '5' && isDigit(lo) +// inRange reports whether s is all decimal digits and its value lies in [lo, hi]. +func inRange(s string, lo, hi uint64) bool { + u, err := strconv.ParseUint(s, 10, 64) + return err == nil && u >= lo && u <= hi } -// isSecond reports whether the two bytes form 00-60 (60 permits a leap second). -func isSecond(hi, lo byte) bool { - if hi == '6' { - return lo == '0' - } - return isMinute(hi, lo) -} +func isYear(s string) bool { return inRange(s, 0, 9999) } +func isMonth(s string) bool { return inRange(s, 1, 12) } +func isDay(s string) bool { return inRange(s, 1, 31) } +func isHour(s string) bool { return inRange(s, 0, 23) } +func isMinute(s string) bool { return inRange(s, 0, 59) } + +// isSecond permits 60 for a leap second. +func isSecond(s string) bool { return inRange(s, 0, 60) } // Add implements traits.Adder.Add. func (t Timestamp) Add(other ref.Val) ref.Val { diff --git a/common/types/timestamp_test.go b/common/types/timestamp_test.go index 53c557c6..91f219a2 100644 --- a/common/types/timestamp_test.go +++ b/common/types/timestamp_test.go @@ -502,7 +502,12 @@ func TestIsStrictRFC3339MatchesPattern(t *testing.T) { "2025-01-01T12:34:56+05:30", "2025-01-01T20:00:00Z", "2025-01-01T23:59:59Z", + "2025-12-31T12:34:56Z", // rejected forms + "2025-00-01T12:34:56Z", + "2025-13-01T12:34:56Z", + "2025-01-00T12:34:56Z", + "2025-01-32T12:34:56Z", "2025-01-01T12:34:56,123Z", "2025-01-01T1:34:56Z", "2025-01-01T12:3:56Z",