Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix Timezone issues with UNTIL and DTSTART #68

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix Timezone issues with UNTIL and DTSTART #68

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0cfae2a
Fix Timezone issues with UNTIL and DTSTART
leaanthony-sc Aug 14, 2024
d898b57
Update test names
leaanthony-sc Aug 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 68 additions & 11 deletions str.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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:")
Expand All @@ -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)
Expand All @@ -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":
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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) {
Expand Down
59 changes: 57 additions & 2 deletions str_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package rrule

import (
"reflect"
"testing"
"time"
)
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -431,3 +438,51 @@ func TestStrSetParseErrors(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 {
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 TestLocalTimeIsParsedCorrectly(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 Test_DTStartInUTC_UNTILInLocalTime(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 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 Test_TZGiven_DTStartInUTC(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")
}
}