Skip to content

fix: off-by-one in timestamp parsing#2127

Merged
dearchap merged 1 commit intourfave:mainfrom
nickajacks1:urfavcli-date-off-by-one
May 10, 2025
Merged

fix: off-by-one in timestamp parsing#2127
dearchap merged 1 commit intourfave:mainfrom
nickajacks1:urfavcli-date-off-by-one

Conversation

@nickajacks1
Copy link
Contributor

Timestamps without dates would infer the correct date based on the local time. This can lead to bugs where the current local date is different than the date in the timezone of the original value. Fix this by using time.Now().In() to adjust the timezone.

What type of PR is this?

  • bug

What this PR does / why we need it:

  • Update timestamp flags to respect the timezone of the original value when inferring missing dates/years.

Which issue(s) this PR fixes:

None.

Special notes for your reviewer:

None in particular.

Testing

Tested with minimal reproduction:

func main() {
	x := &cli.Command{
		Flags: []cli.Flag{
			&cli.TimestampFlag{
				Name: "time",
				Config: cli.TimestampConfig{
					Layouts: []string{
						"15:04Z07:00",
					},
				},
			},
		},
		Action: func(ctx context.Context, c *cli.Command) error {
			y := c.Timestamp("time")
			fmt.Println(y)
			return nil
		},
	}

	x.Run(context.Background(), os.Args)
}

Without fix, running on May 9 6:00 PM UTC-7. Note the date is incorrectly set to the 9th:

$ go run . --time "10:30+00:00"
2025-05-09 10:30:00 +0000 UTC
$ date
Fri May  9 06:26:09 PM PDT 2025

With fix:

$ go run . --time "10:30+00:00"
2025-05-10 10:30:00 +0000 UTC
$ date
Fri May  9 06:26:56 PM PDT 2025

This can also be observed when running the unit tests of this repository:

        	Error:      	Not equal: 
        	            	expected: time.Date(2025, time.May, 10, 0, 33, 0, 0, time.UTC)
        	            	actual  : time.Date(2025, time.May, 9, 0, 33, 0, 0, time.UTC)

Release Notes

Fix an issue where timestamp values without dates failed to respect timezones.

Timestamps without dates would infer the correct date based on the local
time. This can lead to bugs where the current local date is different
than the date in the timezone of the original value. Fix this by using
time.Now().In() to adjust the timezone.
@nickajacks1 nickajacks1 requested a review from a team as a code owner May 10, 2025 01:31
@dearchap dearchap merged commit 72e00f8 into urfave:main May 10, 2025
9 checks passed
@nickajacks1 nickajacks1 deleted the urfavcli-date-off-by-one branch May 10, 2025 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants