Skip to content

Commit b867657

Browse files
committed
Fix timezone parsing
Signed-off-by: Paulo Gomes <pjbgf@linux.com>
1 parent 8b32350 commit b867657

3 files changed

Lines changed: 64 additions & 14 deletions

File tree

cmd/gogit/commit.go

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"errors"
55
"fmt"
66
"os"
7-
"strconv"
87
"time"
98

109
"github.com/go-git/go-git/v6"
@@ -33,6 +32,8 @@ var commitCmd = &cobra.Command{
3332
return fmt.Errorf("failed to open repository: %w", err)
3433
}
3534

35+
defer r.Close()
36+
3637
w, err := r.Worktree()
3738
if err != nil {
3839
return fmt.Errorf("failed to open worktree: %w", err)
@@ -98,20 +99,16 @@ func parseGitDate(s string) (time.Time, error) {
9899
return time.Time{}, err
99100
}
100101

101-
hours, err := strconv.Atoi(zone[:len(zone)-2])
102+
// Defer the sign/range parsing to time.Parse with the canonical "-0700"
103+
// layout: it validates length and digits and handles both positive and
104+
// negative offsets correctly. The reference value's date components are
105+
// irrelevant — we only consume the resulting zone offset.
106+
zt, err := time.Parse("-0700", zone)
102107
if err != nil {
103-
return time.Time{}, err
108+
return time.Time{}, fmt.Errorf("invalid zone %q: %w", zone, err)
104109
}
105110

106-
mins, err := strconv.Atoi(zone[len(zone)-2:])
107-
if err != nil {
108-
return time.Time{}, err
109-
}
110-
111-
offset := hours*3600 + mins*60
112-
if zone[0] == '-' {
113-
offset = -offset
114-
}
111+
_, offset := zt.Zone()
115112

116113
return time.Unix(secs, 0).In(time.FixedZone(zone, offset)), nil
117114
}

cmd/gogit/commit_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,56 @@ import (
44
"os"
55
"path/filepath"
66
"testing"
7+
"time"
78
)
89

10+
func TestParseGitDate(t *testing.T) {
11+
t.Parallel()
12+
13+
tests := []struct {
14+
name string
15+
in string
16+
wantSecs int64
17+
wantOffset int
18+
wantErr bool
19+
}{
20+
{name: "test_tick negative offset", in: "1112911993 -0700", wantSecs: 1112911993, wantOffset: -7 * 3600},
21+
{name: "positive offset", in: "1112911993 +0530", wantSecs: 1112911993, wantOffset: 5*3600 + 30*60},
22+
{name: "missing zone", in: "1112911993", wantErr: true},
23+
{name: "short zone panics in old impl", in: "1112911993 -7", wantErr: true},
24+
{name: "non-numeric zone", in: "1112911993 abcd", wantErr: true},
25+
{name: "non-numeric seconds", in: "abc -0700", wantErr: true},
26+
}
27+
28+
for _, tc := range tests {
29+
t.Run(tc.name, func(t *testing.T) {
30+
t.Parallel()
31+
32+
got, err := parseGitDate(tc.in)
33+
if tc.wantErr {
34+
if err == nil {
35+
t.Fatalf("parseGitDate(%q) expected error, got time %v", tc.in, got)
36+
}
37+
38+
return
39+
}
40+
41+
if err != nil {
42+
t.Fatalf("parseGitDate(%q): %v", tc.in, err)
43+
}
44+
45+
if got.Unix() != tc.wantSecs {
46+
t.Fatalf("seconds: got %d want %d", got.Unix(), tc.wantSecs)
47+
}
48+
49+
_, offset := got.Zone()
50+
if offset != tc.wantOffset {
51+
t.Fatalf("offset: got %d want %d (got time %s)", offset, tc.wantOffset, got.Format(time.RFC3339))
52+
}
53+
})
54+
}
55+
}
56+
957
func TestCommitWithEnvIdentity(t *testing.T) {
1058
t.Parallel()
1159

cmd/gogit/main_test.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,22 @@ func TestMain(m *testing.M) {
1717
if err != nil {
1818
panic(err)
1919
}
20-
defer os.RemoveAll(dir)
2120

2221
gogitBin = filepath.Join(dir, "gogit")
2322
build := exec.Command("go", "build", "-o", gogitBin, ".")
2423

2524
build.Stderr = os.Stderr
2625
if err := build.Run(); err != nil {
26+
_ = os.RemoveAll(dir)
27+
2728
panic(err)
2829
}
2930

30-
os.Exit(m.Run())
31+
// os.Exit skips deferred functions, so run cleanup explicitly before exit.
32+
code := m.Run()
33+
_ = os.RemoveAll(dir)
34+
35+
os.Exit(code)
3136
}
3237

3338
func runGogit(t *testing.T, dir string, args ...string) (string, string, error) {

0 commit comments

Comments
 (0)