go-acme / lego

Let's Encrypt/ACME client and library written in Go
https://go-acme.github.io/lego/
MIT License
7.91k stars 1.02k forks source link

websupport: include tzdata on windows #2274

Closed r0b0 closed 2 weeks ago

r0b0 commented 2 weeks ago

fixes #2273

r0b0 commented 2 weeks ago

see https://github.com/golang/go/issues/55899 for explanation - if golang itself is not installed on the windows machine, time.LoadLocation() will fail unless time/tzdata is imported

ldez commented 2 weeks ago

In fact, we don't need "GMT" timezone, we can use UTC.

--- i/providers/dns/websupport/internal/client.go
+++ w/providers/dns/websupport/internal/client.go
@@ -150,12 +150,7 @@ func (c *Client) DeleteRecord(ctx context.Context, domainName string, recordID i
 func (c *Client) do(req *http.Request, result any) error {
        req.Header.Set("Accept-Language", "en_us")

-       location, err := time.LoadLocation("GMT")
-       if err != nil {
-               return fmt.Errorf("time location: %w", err)
-       }
-
-       err = c.sign(req, time.Now().In(location))
+       err := c.sign(req, time.Now().UTC())
        if err != nil {
                return fmt.Errorf("signature: %w", err)
        }

The problem is related to the usage of time.LoadLocation which related on the zip but .UTC doesn't need this file. And globally GMT is equal to UTC, it's just the old name.

ldez commented 2 weeks ago

FYI, this is not exactly true:

if golang itself is not installed on the windows machine

The problem is related to the missing time zone data on your computer, not related to Go.

The system time zone data are used if the zip is missing.

demo ```go package main import ( "fmt" "log" "time" ) func main() { now := time.Now() fmt.Println(now.UTC()) location, err := time.LoadLocation("GMT") if err != nil { log.Fatalf("time location: %s", err) } fmt.Println(now.In(location)) } ``` ```console $ docker run --rm -v $(pwd):/app -w /app -it alpine:3.20 sh /app # ./foo 2024-09-16 12:44:35.630864645 +0000 UTC 2024/09/16 12:44:35 time location: unknown time zone GMT /app # /app # apk add tzdata fetch https://dl-cdn.alpinelinux.org/alpine/v3.20/main/x86_64/APKINDEX.tar.gz fetch https://dl-cdn.alpinelinux.org/alpine/v3.20/community/x86_64/APKINDEX.tar.gz (1/1) Installing tzdata (2024b-r0) OK: 9 MiB in 15 packages /app # ./foo 2024-09-16 12:45:02.058793991 +0000 UTC 2024-09-16 12:45:02.058793991 +0000 GMT /app # ```
r0b0 commented 2 weeks ago

OK, there are multiple instances of time.LoadLocation() throughout the code - if you have a fix, you probably want to apply it everywhere:

$ grep -ri time.LoadLocation .
./providers/dns/brandit/internal/client.go:     location, err := time.LoadLocation("GMT")
./providers/dns/nifcloud/internal/client.go:            location, err := time.LoadLocation("GMT")
./providers/dns/websupport/internal/client.go:  location, err := time.LoadLocation("GMT")
./providers/dns/wedos/internal/token.go:                loc, err := time.LoadLocation(zoneName)
ldez commented 2 weeks ago

Technically, there is nothing to fix because it relies on system elements by default.

Your problem is related to the time zones available on your system, the time/tzdata package is just a kind of workaround.

https://github.com/golang/go/blob/3d33437c450aa74014ea1d41cd986b6ee6266984/src/time/tzdata/tzdata.go#L5-L8

The time/tzdata package comes with some constraints:

https://github.com/golang/go/blob/3d33437c450aa74014ea1d41cd986b6ee6266984/src/time/tzdata/tzdata.go#L10-L11

So, I will change "GMT" to UTC in some cases but it's not possible everywhere, and it's not required, as the problem is the availability of the time zone information on your system.

To avoid any ambiguity, the time zones are available by default on Windows without the time/tzdata package.

https://github.com/golang/go/blob/master/src/time/zoneinfo_windows.go