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.

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

Unmarshal an empty time object provides a problematic timezone #332

Closed
aviadl opened this issue Feb 16, 2022 · 3 comments · Fixed by #333
Closed

Unmarshal an empty time object provides a problematic timezone #332

aviadl opened this issue Feb 16, 2022 · 3 comments · Fixed by #333

Comments

@aviadl
Copy link

aviadl commented Feb 16, 2022

Unmarshal a byte array that was generated by marshaling an empty time struct
If the unmarshal will receive an interface, this interface will eventually contain an empty time struct but with LMT timezone (that doesnt work properly in many cases)
If the unmarshal will receive a time struct, this will work properly

Expected Behavior

empty time object in utc timezone

Current Behavior

empty time object in LMT timezone

Possible Solution

add tm.isZero case to the flow of unmarshaling from interface
It does exists if a typed time struct is provided

Steps to Reproduce

See the following unit test:

func TestEmptyTimeMarshalWithInterface(t *testing.T) {
	a := time.Time{}
	b, err := msgpack.Marshal(a)
	if err != nil {
		t.Fatal(err)
	}
	var out interface{}
	err = msgpack.Unmarshal(b, &out)
	if err != nil {
		t.Fatal(err)
	}
	name, _ := out.(time.Time).Zone()
	if name != "UTC" {
		t.Fatal("Got wrong timezone")
	}

	var out2 time.Time
	err = msgpack.Unmarshal(b, &out2)
	if err != nil {
		t.Fatal(err)
	}
	name, _ = out2.Zone()
	if name != "UTC" {
		t.Fatal("Got wrong timezone")
	}
}
@pascal-zarrad
Copy link

Experiencing the same issue when using go-redis/cache.
Had to switch over to an alternative (JSON) as a workaround for now due to the issue.

@aviadl
Copy link
Author

aviadl commented Mar 3, 2023

@pascal-zarrad you can check my PR above

@jaimem88
Copy link

jaimem88 commented Nov 1, 2023

https://github.com/eko/gocache recently upgraded to v5, and we are trying to upgrade the dependency as well in https://gitlab.com/gitlab-org/container-registry/-/merge_requests/1442.

Now we are encountering a similar issue where the time.Location is set as time.Local instead of UTC -> https://gitlab.com/gitlab-renovate-forks/container-registry/-/jobs/5419691095, see extract from the test output

expected:  DeletedAt:sql.NullTime{Time:time.Date(1, time.January, 1, 0, 0, 0, 0, time.UTC)
actual:       DeletedAt:sql.NullTime{Time:time.Date(1, time.January, 1, 0, 0, 0, 0, time.Local)

It's not the worst as the time is still kept as zero but our test fail when checking for equality.

cc @aviadl @vmihailenco

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants