From 0cfae2a5b1974eec8c9f2dc495a24a133b30c194 Mon Sep 17 00:00:00 2001 From: Lea Anthony Date: Thu, 15 Aug 2024 08:44:37 +1000 Subject: [PATCH 1/2] Fix Timezone issues with UNTIL and DTSTART --- str.go | 79 +++++++++++++++++++++++++++++++++++++++++++++-------- str_test.go | 59 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 125 insertions(+), 13 deletions(-) diff --git a/str.go b/str.go index 76f70f3..5d65d6e 100644 --- a/str.go +++ b/str.go @@ -23,6 +23,10 @@ func timeToStr(time time.Time) string { return time.UTC().Format(DateTimeFormat) } +func localTimeToStr(time time.Time) string { + return time.Format(DateTimeFormat) +} + func strToTimeInLoc(str string, loc *time.Location) (time.Time, error) { if len(str) == len(DateFormat) { return time.ParseInLocation(DateFormat, str, loc) @@ -144,7 +148,18 @@ func (option *ROption) RRuleString() string { result = append(result, fmt.Sprintf("COUNT=%v", option.Count)) } if !option.Until.IsZero() { - result = append(result, fmt.Sprintf("UNTIL=%v", timeToStr(option.Until))) + var timeString string + loc := option.Until.Location() + if loc == nil || loc == time.UTC { + timeString = fmt.Sprintf("UNTIL=%v", timeToStr(option.Until)) + } else { + timeString = fmt.Sprintf("UNTIL=%s", localTimeToStr(option.Until)) + if strings.HasSuffix(timeString, "Z") { + timeString = timeString[:len(timeString)-1] + } + } + + result = append(result, timeString) } result = appendIntsOption(result, "BYSETPOS", option.Bysetpos) result = appendIntsOption(result, "BYMONTH", option.Bymonth) @@ -190,7 +205,17 @@ func StrToROptionInLocation(rfcString string, loc *time.Location) (*ROption, err result := ROption{} freqSet := false + // Keep track of if DTSTART and UNTIL are given and if they are in the same format + var dtstartGiven bool + var dtstartTimeDefinedAsUTC bool + var untilGiven bool + var untilTimeDefinedAsUTC bool + if dtstartStr != "" { + // Save dstart time format + dtstartGiven = true + dtstartTimeDefinedAsUTC = strings.HasSuffix(dtstartStr, "Z") + firstName, err := processRRuleName(dtstartStr) if err != nil { return nil, fmt.Errorf("expect DTSTART but: %s", err) @@ -203,6 +228,9 @@ func StrToROptionInLocation(rfcString string, loc *time.Location) (*ROption, err if err != nil { return nil, fmt.Errorf("StrToDtStart failed: %s", err) } + if dtstartTimeDefinedAsUTC && result.Dtstart.Location() != time.UTC { + return nil, fmt.Errorf("DTSTART time format mismatch: TZID='%s' but time given in UTC", result.Dtstart.Location().String()) + } } rruleStr = strings.TrimPrefix(rruleStr, "RRULE:") @@ -221,6 +249,8 @@ func StrToROptionInLocation(rfcString string, loc *time.Location) (*ROption, err result.Freq, e = StrToFreq(value) freqSet = true case "DTSTART": + dtstartGiven = true + dtstartTimeDefinedAsUTC = strings.HasSuffix(value, "Z") result.Dtstart, e = strToTimeInLoc(value, loc) case "INTERVAL": result.Interval, e = strconv.Atoi(value) @@ -229,7 +259,14 @@ func StrToROptionInLocation(rfcString string, loc *time.Location) (*ROption, err case "COUNT": result.Count, e = strconv.Atoi(value) case "UNTIL": - result.Until, e = strToTimeInLoc(value, loc) + untilGiven = true + untilTimeDefinedAsUTC = strings.HasSuffix(value, "Z") + location := loc + // If UNTIL time is LOCAL time, we need to parse it in the same location as DTSTART + if !untilTimeDefinedAsUTC && result.Dtstart.Location() != nil { + location = result.Dtstart.Location() + } + result.Until, e = strToTimeInLoc(value, location) case "BYSETPOS": result.Bysetpos, e = strToInts(value) case "BYMONTH": @@ -264,6 +301,13 @@ func StrToROptionInLocation(rfcString string, loc *time.Location) (*ROption, err // a value from the options this returns. return nil, errors.New("RRULE property FREQ is required") } + + // If dtstart isn't in the same format as until, return an error + if dtstartGiven && untilGiven { + if dtstartTimeDefinedAsUTC != untilTimeDefinedAsUTC { + return nil, errors.New("DTSTART and UNTIL time format mismatch") + } + } return &result, nil } @@ -441,21 +485,34 @@ func processRRuleName(line string) (string, error) { // StrToDtStart accepts string with format: "(TZID={timezone}:)?{time}" and parses it to a date // may be used to parse DTSTART rules, without the DTSTART; part. func StrToDtStart(str string, defaultLoc *time.Location) (time.Time, error) { - tmp := strings.Split(str, ":") - if len(tmp) > 2 || len(tmp) == 0 { - return time.Time{}, fmt.Errorf("bad format") + parts := strings.Split(str, ":") + if len(parts) < 0 || len(parts) > 2 { + return time.Time{}, errors.New("bad format") } + var location = defaultLoc + var err error - if len(tmp) == 2 { - // tzid - loc, err := parseTZID(tmp[0]) + // If we have a time zone, parse it + if len(parts) == 2 { + location, err = parseTZID(parts[0]) if err != nil { return time.Time{}, err } - return strToTimeInLoc(tmp[1], loc) + parts = parts[1:] + } + + // Parse time + dtstartTime, err := strToTimeInLoc(parts[0], location) + if err != nil { + return time.Time{}, err } - // no tzid, len == 1 - return strToTimeInLoc(tmp[0], defaultLoc) + + // Ensure that we don't mismatch time zones + if strings.HasSuffix(parts[0], "Z") && location != time.UTC { + return time.Time{}, errors.New("DTSTART time format in UTC but location is specified") + } + + return dtstartTime, nil } func parseTZID(s string) (*time.Location, error) { diff --git a/str_test.go b/str_test.go index 80b7c3f..b1621da 100644 --- a/str_test.go +++ b/str_test.go @@ -3,6 +3,7 @@ package rrule import ( + "reflect" "testing" "time" ) @@ -40,12 +41,18 @@ func TestRFCSetToString(t *testing.T) { func TestCompatibility(t *testing.T) { str := "FREQ=WEEKLY;DTSTART=20120201T093000Z;INTERVAL=5;WKST=TU;COUNT=2;UNTIL=20130130T230000Z;BYSETPOS=2;BYMONTH=3;BYYEARDAY=95;BYWEEKNO=1;BYDAY=MO,+2FR;BYHOUR=9;BYMINUTE=30;BYSECOND=0;BYEASTER=-1" - r, _ := StrToRRule(str) + r, err := StrToRRule(str) + if err != nil { + t.Fatalf(err.Error()) + } want := "DTSTART:20120201T093000Z\nRRULE:FREQ=WEEKLY;INTERVAL=5;WKST=TU;COUNT=2;UNTIL=20130130T230000Z;BYSETPOS=2;BYMONTH=3;BYYEARDAY=95;BYWEEKNO=1;BYDAY=MO,+2FR;BYHOUR=9;BYMINUTE=30;BYSECOND=0;BYEASTER=-1" if s := r.String(); s != want { t.Errorf("StrToRRule(%q).String() = %q, want %q", str, s, want) } - r, _ = StrToRRule(want) + r, err = StrToRRule(want) + if err != nil { + t.Fatalf(err.Error()) + } if s := r.String(); s != want { t.Errorf("StrToRRule(%q).String() = %q, want %q", want, want, want) } @@ -431,3 +438,51 @@ func TestStrSetParseErrors(t *testing.T) { } } } + +func TestLocalTime(t *testing.T) { + localTimeRRULE := "DTSTART;TZID=Australia/Sydney:19980101T090000\nRRULE:FREQ=WEEKLY;UNTIL=20201230T220000" + r, err := StrToRRule(localTimeRRULE) + if err != nil { + t.Errorf("Error parsing rrule: %v", err) + } + localTimeRRULE2 := r.String() + if localTimeRRULE != localTimeRRULE2 { + t.Errorf("Expected:\n%v\ngot\n%v\n", localTimeRRULE, localTimeRRULE2) + } +} + +func TestLocalTime2(t *testing.T) { + sydney, _ := time.LoadLocation("Australia/Sydney") + localTimeRRULE := "DTSTART;TZID=Australia/Sydney:19980101T090000\nRRULE:FREQ=WEEKLY;UNTIL=20201230T220000" + r, err := StrToRRule(localTimeRRULE) + if err != nil { + t.Errorf("Error parsing rrule: %v", err) + } + until := r.GetUntil() + if !reflect.DeepEqual(sydney, until.Location()) { + t.Errorf("Expected:\n%v\ngot\n%v\n", sydney.String(), until.Location().String()) + } +} + +func TestDateTimeMismatchErrors1(t *testing.T) { + mismatchedTimeFormats := "DTSTART;19980101T090000Z\nRRULE:FREQ=WEEKLY;UNTIL=20201230T220000" + _, err := StrToRRule(mismatchedTimeFormats) + if err == nil { + t.Errorf("Expected error due to mismatched time formats") + } +} + +func TestDateTimeMismatchErrors2(t *testing.T) { + mismatchedTimeFormats := "DTSTART;19980101T090000\nRRULE:FREQ=WEEKLY;UNTIL=20201230T220000Z" + _, err := StrToRRule(mismatchedTimeFormats) + if err == nil { + t.Errorf("Expected error due to mismatched time formats") + } +} +func TestDateTimeMismatchErrors3(t *testing.T) { + mismatchedTimeFormats := "DTSTART;TZID=Australia/Sydney:19980101T090000Z\nRRULE:FREQ=WEEKLY;UNTIL=20201230T220000Z" + _, err := StrToRRule(mismatchedTimeFormats) + if err == nil { + t.Errorf("Expected error due to both TZID and UTC DATETIME given") + } +} From d898b57a072d5e3afa6760955097421fdb981c08 Mon Sep 17 00:00:00 2001 From: Lea Anthony Date: Thu, 15 Aug 2024 09:58:25 +1000 Subject: [PATCH 2/2] Update test names --- str_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/str_test.go b/str_test.go index b1621da..d3fc4f1 100644 --- a/str_test.go +++ b/str_test.go @@ -439,7 +439,7 @@ func TestStrSetParseErrors(t *testing.T) { } } -func TestLocalTime(t *testing.T) { +func TestRRULEStringKeepsLocalTime(t *testing.T) { localTimeRRULE := "DTSTART;TZID=Australia/Sydney:19980101T090000\nRRULE:FREQ=WEEKLY;UNTIL=20201230T220000" r, err := StrToRRule(localTimeRRULE) if err != nil { @@ -451,7 +451,7 @@ func TestLocalTime(t *testing.T) { } } -func TestLocalTime2(t *testing.T) { +func TestLocalTimeIsParsedCorrectly(t *testing.T) { sydney, _ := time.LoadLocation("Australia/Sydney") localTimeRRULE := "DTSTART;TZID=Australia/Sydney:19980101T090000\nRRULE:FREQ=WEEKLY;UNTIL=20201230T220000" r, err := StrToRRule(localTimeRRULE) @@ -464,7 +464,7 @@ func TestLocalTime2(t *testing.T) { } } -func TestDateTimeMismatchErrors1(t *testing.T) { +func Test_DTStartInUTC_UNTILInLocalTime(t *testing.T) { mismatchedTimeFormats := "DTSTART;19980101T090000Z\nRRULE:FREQ=WEEKLY;UNTIL=20201230T220000" _, err := StrToRRule(mismatchedTimeFormats) if err == nil { @@ -472,14 +472,14 @@ func TestDateTimeMismatchErrors1(t *testing.T) { } } -func TestDateTimeMismatchErrors2(t *testing.T) { +func Test_DTStartInLocalTime_UNTILInUTC(t *testing.T) { mismatchedTimeFormats := "DTSTART;19980101T090000\nRRULE:FREQ=WEEKLY;UNTIL=20201230T220000Z" _, err := StrToRRule(mismatchedTimeFormats) if err == nil { t.Errorf("Expected error due to mismatched time formats") } } -func TestDateTimeMismatchErrors3(t *testing.T) { +func Test_TZGiven_DTStartInUTC(t *testing.T) { mismatchedTimeFormats := "DTSTART;TZID=Australia/Sydney:19980101T090000Z\nRRULE:FREQ=WEEKLY;UNTIL=20201230T220000Z" _, err := StrToRRule(mismatchedTimeFormats) if err == nil {