From 8f8b23c729ad8839dec67270292d9a01aec02163 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 2 Mar 2026 16:10:34 +0100 Subject: [PATCH 1/9] daemon: fix some untyped "invalid parameter" errors Signed-off-by: Sebastiaan van Stijn --- daemon/containerd/image_list.go | 7 ++++--- daemon/images/image_list.go | 5 +++-- daemon/images/image_prune.go | 13 ++++++------- daemon/server/router/system/system_routes.go | 4 ++-- 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/daemon/containerd/image_list.go b/daemon/containerd/image_list.go index ae004cdca4..83578d54cb 100644 --- a/daemon/containerd/image_list.go +++ b/daemon/containerd/image_list.go @@ -3,6 +3,7 @@ package containerd import ( "context" "encoding/json" + "fmt" "runtime" "sort" "strings" @@ -603,11 +604,11 @@ func (i *ImageService) setupFilters(ctx context.Context, imageFilters filters.Ar err = imageFilters.WalkValues("until", func(value string) error { ts, err := timestamp.GetTimestamp(value, time.Now()) if err != nil { - return err + return errdefs.InvalidParameter(fmt.Errorf("invalid value for 'until' filter: %w", err)) } seconds, nanoseconds, err := timestamp.ParseTimestamps(ts, 0) if err != nil { - return err + return errdefs.InvalidParameter(fmt.Errorf("invalid value for 'until' filter: %w", err)) } until := time.Unix(seconds, nanoseconds) @@ -615,7 +616,7 @@ func (i *ImageService) setupFilters(ctx context.Context, imageFilters filters.Ar created := image.CreatedAt return created.Before(until) }) - return err + return nil }) if err != nil { return nil, err diff --git a/daemon/images/image_list.go b/daemon/images/image_list.go index f1571041dc..d750aebf67 100644 --- a/daemon/images/image_list.go +++ b/daemon/images/image_list.go @@ -14,6 +14,7 @@ import ( "github.com/moby/moby/v2/daemon/internal/layer" "github.com/moby/moby/v2/daemon/internal/timestamp" "github.com/moby/moby/v2/daemon/server/imagebackend" + "github.com/moby/moby/v2/errdefs" ) var acceptedImageFilterTags = map[string]bool{ @@ -64,11 +65,11 @@ func (i *ImageService) Images(ctx context.Context, opts imagebackend.ListOptions err = opts.Filters.WalkValues("until", func(value string) error { ts, err := timestamp.GetTimestamp(value, time.Now()) if err != nil { - return err + return errdefs.InvalidParameter(fmt.Errorf("invalid value for 'until' filter: %w", err)) } seconds, nanoseconds, err := timestamp.ParseTimestamps(ts, 0) if err != nil { - return err + return errdefs.InvalidParameter(fmt.Errorf("invalid value for 'until' filter: %w", err)) } if tsUnix := time.Unix(seconds, nanoseconds); beforeFilter.IsZero() || beforeFilter.After(tsUnix) { beforeFilter = tsUnix diff --git a/daemon/images/image_prune.go b/daemon/images/image_prune.go index a21c063132..102b33e3cb 100644 --- a/daemon/images/image_prune.go +++ b/daemon/images/image_prune.go @@ -2,6 +2,7 @@ package images import ( "context" + "fmt" "strconv" "time" @@ -185,22 +186,20 @@ func matchLabels(pruneFilters filters.Args, labels map[string]string) bool { } func getUntilFromPruneFilters(pruneFilters filters.Args) (time.Time, error) { - until := time.Time{} if !pruneFilters.Contains("until") { - return until, nil + return time.Time{}, nil } untilFilters := pruneFilters.Get("until") if len(untilFilters) > 1 { - return until, errors.New("more than one until filter specified") + return time.Time{}, errdefs.InvalidParameter(errors.New("more than one until filter specified")) } ts, err := timestamp.GetTimestamp(untilFilters[0], time.Now()) if err != nil { - return until, err + return time.Time{}, errdefs.InvalidParameter(fmt.Errorf("invalid value for 'until' filter: %w", err)) } seconds, nanoseconds, err := timestamp.ParseTimestamps(ts, 0) if err != nil { - return until, err + return time.Time{}, errdefs.InvalidParameter(fmt.Errorf("invalid value for 'until' filter: %w", err)) } - until = time.Unix(seconds, nanoseconds) - return until, nil + return time.Unix(seconds, nanoseconds), nil } diff --git a/daemon/server/router/system/system_routes.go b/daemon/server/router/system/system_routes.go index 62517e2b67..e43bc5cfa1 100644 --- a/daemon/server/router/system/system_routes.go +++ b/daemon/server/router/system/system_routes.go @@ -297,11 +297,11 @@ func (s *systemRouter) getEvents(ctx context.Context, w http.ResponseWriter, r * since, err := eventTime(r.Form.Get("since")) if err != nil { - return err + return invalidRequestError{fmt.Errorf("invalid value for 'since': %w", err)} } until, err := eventTime(r.Form.Get("until")) if err != nil { - return err + return invalidRequestError{fmt.Errorf("invalid value for 'until': %w", err)} } var ( From 282297aa0c693d34fa0c131033f7fc4f551d920e Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 26 Feb 2026 01:20:04 +0100 Subject: [PATCH 2/9] daemon/internal: GetTimestamp: don't discard nanosecs, and trim zero - The function would discard nanoseconds when parsing durations - Some code-paths would omit zero nanoseconds; consistently discard "zero" nanoseconds Signed-off-by: Sebastiaan van Stijn --- daemon/internal/timestamp/timestamp.go | 11 ++++- daemon/internal/timestamp/timestamp_test.go | 47 ++++++++++----------- 2 files changed, 32 insertions(+), 26 deletions(-) diff --git a/daemon/internal/timestamp/timestamp.go b/daemon/internal/timestamp/timestamp.go index 7b175f0c93..9e7441e51e 100644 --- a/daemon/internal/timestamp/timestamp.go +++ b/daemon/internal/timestamp/timestamp.go @@ -25,7 +25,11 @@ const ( // as the given reference time minus the amount of the duration. func GetTimestamp(value string, reference time.Time) (string, error) { if d, err := time.ParseDuration(value); value != "0" && err == nil { - return strconv.FormatInt(reference.Add(-d).Unix(), 10), nil + t := reference.Add(-d) + if t.Nanosecond() == 0 { + return strconv.FormatInt(t.Unix(), 10), nil + } + return fmt.Sprintf("%d.%09d", t.Unix(), t.Nanosecond()), nil } var format string @@ -92,7 +96,10 @@ func GetTimestamp(value string, reference time.Time) (string, error) { return value, nil // unix timestamp in and out case (meaning: the value passed at the command line is already in the right format for passing to the server) } - return fmt.Sprintf("%d.%09d", t.Unix(), int64(t.Nanosecond())), nil + if t.Nanosecond() == 0 { + return strconv.FormatInt(t.Unix(), 10), nil + } + return fmt.Sprintf("%d.%09d", t.Unix(), t.Nanosecond()), nil } // ParseTimestamps returns seconds and nanoseconds from a timestamp that has diff --git a/daemon/internal/timestamp/timestamp_test.go b/daemon/internal/timestamp/timestamp_test.go index 56aab2ee3a..8dc6db79dc 100644 --- a/daemon/internal/timestamp/timestamp_test.go +++ b/daemon/internal/timestamp/timestamp_test.go @@ -1,13 +1,12 @@ package timestamp import ( - "fmt" "testing" "time" ) func TestGetTimestamp(t *testing.T) { - now := time.Now().In(time.UTC) + now := time.Date(2020, 1, 2, 3, 4, 5, 123456789, time.UTC) cases := []struct { in, expected string expectedErr bool @@ -16,38 +15,38 @@ func TestGetTimestamp(t *testing.T) { {"2006-01-02T15:04:05.999999999+07:00", "1136189045.999999999", false}, {"2006-01-02T15:04:05.999999999Z", "1136214245.999999999", false}, {"2006-01-02T15:04:05.999999999", "1136214245.999999999", false}, - {"2006-01-02T15:04:05Z", "1136214245.000000000", false}, - {"2006-01-02T15:04:05", "1136214245.000000000", false}, + {"2006-01-02T15:04:05Z", "1136214245", false}, + {"2006-01-02T15:04:05", "1136214245", false}, {"2006-01-02T15:04:0Z", "", true}, {"2006-01-02T15:04:0", "", true}, - {"2006-01-02T15:04Z", "1136214240.000000000", false}, - {"2006-01-02T15:04+00:00", "1136214240.000000000", false}, - {"2006-01-02T15:04-00:00", "1136214240.000000000", false}, - {"2006-01-02T15:04", "1136214240.000000000", false}, + {"2006-01-02T15:04Z", "1136214240", false}, + {"2006-01-02T15:04+00:00", "1136214240", false}, + {"2006-01-02T15:04-00:00", "1136214240", false}, + {"2006-01-02T15:04", "1136214240", false}, {"2006-01-02T15:0Z", "", true}, {"2006-01-02T15:0", "", true}, - {"2006-01-02T15Z", "1136214000.000000000", false}, - {"2006-01-02T15+00:00", "1136214000.000000000", false}, - {"2006-01-02T15-00:00", "1136214000.000000000", false}, - {"2006-01-02T15", "1136214000.000000000", false}, - {"2006-01-02T1Z", "1136163600.000000000", false}, - {"2006-01-02T1", "1136163600.000000000", false}, + {"2006-01-02T15Z", "1136214000", false}, + {"2006-01-02T15+00:00", "1136214000", false}, + {"2006-01-02T15-00:00", "1136214000", false}, + {"2006-01-02T15", "1136214000", false}, + {"2006-01-02T1Z", "1136163600", false}, + {"2006-01-02T1", "1136163600", false}, {"2006-01-02TZ", "", true}, {"2006-01-02T", "", true}, - {"2006-01-02+00:00", "1136160000.000000000", false}, - {"2006-01-02-00:00", "1136160000.000000000", false}, - {"2006-01-02-00:01", "1136160060.000000000", false}, - {"2006-01-02Z", "1136160000.000000000", false}, - {"2006-01-02", "1136160000.000000000", false}, - {"2015-05-13T20:39:09Z", "1431549549.000000000", false}, + {"2006-01-02+00:00", "1136160000", false}, + {"2006-01-02-00:00", "1136160000", false}, + {"2006-01-02-00:01", "1136160060", false}, + {"2006-01-02Z", "1136160000", false}, + {"2006-01-02", "1136160000", false}, + {"2015-05-13T20:39:09Z", "1431549549", false}, // unix timestamps returned as is {"1136073600", "1136073600", false}, {"1136073600.000000001", "1136073600.000000001", false}, // Durations - {"1m", fmt.Sprintf("%d", now.Add(-1*time.Minute).Unix()), false}, - {"1.5h", fmt.Sprintf("%d", now.Add(-90*time.Minute).Unix()), false}, - {"1h30m", fmt.Sprintf("%d", now.Add(-90*time.Minute).Unix()), false}, + {"1m", "1577934185.123456789", false}, + {"1.5h", "1577928845.123456789", false}, + {"1h30m", "1577928845.123456789", false}, {"invalid", "", true}, {"", "", true}, @@ -58,7 +57,7 @@ func TestGetTimestamp(t *testing.T) { if o != c.expected || (err == nil && c.expectedErr) || (err != nil && !c.expectedErr) { - t.Errorf("wrong value for '%s'. expected:'%s' got:'%s' with error: `%s`", c.in, c.expected, o, err) + t.Errorf("wrong value for '%s'. expected:'%s' got:'%s' with error: `%v`", c.in, c.expected, o, err) t.Fail() } } From f37fc9fb639c69ed7cb9080e0140f45dcc5d4cc5 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 26 Feb 2026 00:18:15 +0100 Subject: [PATCH 3/9] daemon/internal/timestamp: use table-tests Also use RFC3339Nano for "expected" values in preparation of switching to strong typed results. Signed-off-by: Sebastiaan van Stijn --- daemon/internal/timestamp/timestamp_test.go | 187 +++++++++++++------- 1 file changed, 119 insertions(+), 68 deletions(-) diff --git a/daemon/internal/timestamp/timestamp_test.go b/daemon/internal/timestamp/timestamp_test.go index 8dc6db79dc..457d8983b7 100644 --- a/daemon/internal/timestamp/timestamp_test.go +++ b/daemon/internal/timestamp/timestamp_test.go @@ -1,94 +1,145 @@ -package timestamp +package timestamp_test import ( + "fmt" + "strconv" "testing" "time" + + "github.com/moby/moby/v2/daemon/internal/timestamp" + "gotest.tools/v3/assert" + is "gotest.tools/v3/assert/cmp" ) func TestGetTimestamp(t *testing.T) { now := time.Date(2020, 1, 2, 3, 4, 5, 123456789, time.UTC) - cases := []struct { - in, expected string - expectedErr bool + tests := []struct { + in string + expected string // RFC3339Nano in UTC + expectedErr bool }{ // Partial RFC3339 strings get parsed with second precision - {"2006-01-02T15:04:05.999999999+07:00", "1136189045.999999999", false}, - {"2006-01-02T15:04:05.999999999Z", "1136214245.999999999", false}, - {"2006-01-02T15:04:05.999999999", "1136214245.999999999", false}, - {"2006-01-02T15:04:05Z", "1136214245", false}, - {"2006-01-02T15:04:05", "1136214245", false}, - {"2006-01-02T15:04:0Z", "", true}, - {"2006-01-02T15:04:0", "", true}, - {"2006-01-02T15:04Z", "1136214240", false}, - {"2006-01-02T15:04+00:00", "1136214240", false}, - {"2006-01-02T15:04-00:00", "1136214240", false}, - {"2006-01-02T15:04", "1136214240", false}, - {"2006-01-02T15:0Z", "", true}, - {"2006-01-02T15:0", "", true}, - {"2006-01-02T15Z", "1136214000", false}, - {"2006-01-02T15+00:00", "1136214000", false}, - {"2006-01-02T15-00:00", "1136214000", false}, - {"2006-01-02T15", "1136214000", false}, - {"2006-01-02T1Z", "1136163600", false}, - {"2006-01-02T1", "1136163600", false}, - {"2006-01-02TZ", "", true}, - {"2006-01-02T", "", true}, - {"2006-01-02+00:00", "1136160000", false}, - {"2006-01-02-00:00", "1136160000", false}, - {"2006-01-02-00:01", "1136160060", false}, - {"2006-01-02Z", "1136160000", false}, - {"2006-01-02", "1136160000", false}, - {"2015-05-13T20:39:09Z", "1431549549", false}, + {in: "2006-01-02T15:04:05.999999999+07:00", expected: "2006-01-02T08:04:05.999999999Z"}, + {in: "2006-01-02T15:04:05.999999999Z", expected: "2006-01-02T15:04:05.999999999Z"}, + {in: "2006-01-02T15:04:05.999999999", expected: "2006-01-02T15:04:05.999999999Z"}, + {in: "2006-01-02T15:04:05Z", expected: "2006-01-02T15:04:05Z"}, + {in: "2006-01-02T15:04:05", expected: "2006-01-02T15:04:05Z"}, + {in: "2006-01-02T15:04:0Z", expectedErr: true}, + {in: "2006-01-02T15:04:0", expectedErr: true}, + {in: "2006-01-02T15:04Z", expected: "2006-01-02T15:04:00Z"}, + {in: "2006-01-02T15:04+00:00", expected: "2006-01-02T15:04:00Z"}, + {in: "2006-01-02T15:04-00:00", expected: "2006-01-02T15:04:00Z"}, + {in: "2006-01-02T15:04", expected: "2006-01-02T15:04:00Z"}, + {in: "2006-01-02T15:0Z", expectedErr: true}, + {in: "2006-01-02T15:0", expectedErr: true}, + {in: "2006-01-02T15Z", expected: "2006-01-02T15:00:00Z"}, + {in: "2006-01-02T15+00:00", expected: "2006-01-02T15:00:00Z"}, + {in: "2006-01-02T15-00:00", expected: "2006-01-02T15:00:00Z"}, + {in: "2006-01-02T15", expected: "2006-01-02T15:00:00Z"}, + {in: "2006-01-02T1Z", expected: "2006-01-02T01:00:00Z"}, + {in: "2006-01-02T1", expected: "2006-01-02T01:00:00Z"}, + {in: "2006-01-02TZ", expectedErr: true}, + {in: "2006-01-02T", expectedErr: true}, + {in: "2006-01-02+00:00", expected: "2006-01-02T00:00:00Z"}, + {in: "2006-01-02-00:00", expected: "2006-01-02T00:00:00Z"}, + {in: "2006-01-02-00:01", expected: "2006-01-02T00:01:00Z"}, + {in: "2006-01-02Z", expected: "2006-01-02T00:00:00Z"}, + {in: "2006-01-02", expected: "2006-01-02T00:00:00Z"}, + {in: "2015-05-13T20:39:09Z", expected: "2015-05-13T20:39:09Z"}, - // unix timestamps returned as is - {"1136073600", "1136073600", false}, - {"1136073600.000000001", "1136073600.000000001", false}, - // Durations - {"1m", "1577934185.123456789", false}, - {"1.5h", "1577928845.123456789", false}, - {"1h30m", "1577928845.123456789", false}, + // Unix timestamps + {in: "1136073600", expected: "2006-01-01T00:00:00Z"}, + {in: "1136073600.000000001", expected: "2006-01-01T00:00:00.000000001Z"}, - {"invalid", "", true}, - {"", "", true}, + // Durations (relative to `now`) + {in: "-1m", expected: "2020-01-02T03:05:05.123456789Z"}, // welcome to the future ¯\_(ツ)_/¯ + {in: "0m", expected: "2020-01-02T03:04:05.123456789Z"}, + {in: "1m", expected: "2020-01-02T03:03:05.123456789Z"}, + {in: "1.5h", expected: "2020-01-02T01:34:05.123456789Z"}, + {in: "1h30m", expected: "2020-01-02T01:34:05.123456789Z"}, + + // invalid values + {in: " 1136073600 \t", expectedErr: true}, + {in: "foo.bar", expectedErr: true}, + {in: "1136073600.bar", expectedErr: true}, + {in: "1136073600.000000001bar", expectedErr: true}, + {in: "invalid", expectedErr: true}, + {in: "", expectedErr: true}, } - for _, c := range cases { - o, err := GetTimestamp(c.in, now) - if o != c.expected || - (err == nil && c.expectedErr) || - (err != nil && !c.expectedErr) { - t.Errorf("wrong value for '%s'. expected:'%s' got:'%s' with error: `%v`", c.in, c.expected, o, err) - t.Fail() + for _, tc := range tests { + name := tc.in + if name == "" { + name = "" } + t.Run(name, func(t *testing.T) { + out, err := timestamp.GetTimestamp(tc.in, now) + if tc.expectedErr { + assert.Assert(t, err != nil, "expected error for %q, got none", tc.in) + return + } + assert.NilError(t, err) + + exp, err := time.Parse(time.RFC3339Nano, tc.expected) + assert.NilError(t, err, "invalid expected value") + + var want string + if exp.Nanosecond() == 0 { + want = strconv.FormatInt(exp.Unix(), 10) + } else { + want = fmt.Sprintf("%d.%09d", exp.Unix(), exp.Nanosecond()) + } + assert.Assert(t, out == want, + "expected: %s\ngot: %s", + want, + out, + ) + }) } } func TestParseTimestamps(t *testing.T) { - cases := []struct { - in string - def, expectedS, expectedN int64 - expectedErr bool + tests := []struct { + in string + def int64 + expectedS int64 + expectedN int64 + expectedErr bool }{ // unix timestamps - {"1136073600", 0, 1136073600, 0, false}, - {"1136073600.000000001", 0, 1136073600, 1, false}, - {"1136073600.0000000010", 0, 1136073600, 1, false}, - {"1136073600.0000000001", 0, 1136073600, 0, false}, - {"1136073600.0000000009", 0, 1136073600, 0, false}, - {"1136073600.00000001", 0, 1136073600, 10, false}, - {"foo.bar", 0, 0, 0, true}, - {"1136073600.bar", 0, 1136073600, 0, true}, - {"", -1, -1, 0, false}, + {in: "1136073600", expectedS: 1136073600, expectedN: 0}, + {in: "1136073600.0", expectedS: 1136073600, expectedN: 0}, + {in: "1136073600.000000001", expectedS: 1136073600, expectedN: 1}, + {in: "1136073600.0000000010", expectedS: 1136073600, expectedN: 1}, // truncates + {in: "1136073600.0000000001", expectedS: 1136073600, expectedN: 0}, // truncates + {in: "1136073600.0000000009", expectedS: 1136073600, expectedN: 0}, // truncates + {in: "1136073600.00000001", expectedS: 1136073600, expectedN: 10}, // pads + {in: "1136073600.1", expectedS: 1136073600, expectedN: 100000000}, // pads + + // invalid values + {in: " 1136073600 \t", expectedErr: true}, + {in: "foo.bar", expectedErr: true}, + {in: ".0000000009", expectedErr: true}, + {in: "1136073600.bar", expectedErr: true}, + + // default value + {in: "", def: -1, expectedS: -1, expectedN: 0}, } - for _, c := range cases { - s, n, err := ParseTimestamps(c.in, c.def) - if s != c.expectedS || - n != c.expectedN || - (err == nil && c.expectedErr) || - (err != nil && !c.expectedErr) { - t.Errorf("wrong values for input `%s` with default `%d` expected:'%d'seconds and `%d`nanosecond got:'%d'seconds and `%d`nanoseconds with error: `%s`", c.in, c.def, c.expectedS, c.expectedN, s, n, err) - t.Fail() + for _, tc := range tests { + name := tc.in + if name == "" { + name = "" } + t.Run(name, func(t *testing.T) { + s, n, err := timestamp.ParseTimestamps(tc.in, tc.def) + if tc.expectedErr { + assert.Assert(t, err != nil, "expected error for %q, got none", tc.in) + return + } + assert.NilError(t, err) + assert.Check(t, is.Equal(s, tc.expectedS)) + assert.Check(t, is.Equal(n, tc.expectedN)) + }) } } From fc3cb003986c2ff4e3c3893b7df2a46ee9b0324e Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 25 Feb 2026 15:02:20 +0100 Subject: [PATCH 4/9] daemon/internal/timestamp: don't return value on err These functions would return partial values when invalid; update it to return zero values. Signed-off-by: Sebastiaan van Stijn --- daemon/internal/timestamp/timestamp.go | 12 ++++++++---- daemon/internal/timestamp/timestamp_test.go | 1 + 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/daemon/internal/timestamp/timestamp.go b/daemon/internal/timestamp/timestamp.go index 9e7441e51e..2496f68483 100644 --- a/daemon/internal/timestamp/timestamp.go +++ b/daemon/internal/timestamp/timestamp.go @@ -116,21 +116,25 @@ func ParseTimestamps(value string, defaultSeconds int64) (seconds int64, nanosec if value == "" { return defaultSeconds, 0, nil } - return parseTimestamp(value) + s, n, err := parseTimestamp(value) + if err != nil { + return 0, 0, fmt.Errorf("invalid timestamp %q: %w", value, err) + } + return s, n, nil } func parseTimestamp(value string) (seconds int64, nanoseconds int64, _ error) { s, n, ok := strings.Cut(value, ".") sec, err := strconv.ParseInt(s, 10, 64) if err != nil { - return sec, 0, err + return 0, 0, err } - if !ok { + if !ok || n == "0" || n == "" { return sec, 0, nil } nsec, err := strconv.ParseInt(n, 10, 64) if err != nil { - return sec, nsec, err + return 0, 0, err } // should already be in nanoseconds but just in case convert n to nanoseconds nsec = int64(float64(nsec) * math.Pow(float64(10), float64(9-len(n)))) diff --git a/daemon/internal/timestamp/timestamp_test.go b/daemon/internal/timestamp/timestamp_test.go index 457d8983b7..ae71cf834b 100644 --- a/daemon/internal/timestamp/timestamp_test.go +++ b/daemon/internal/timestamp/timestamp_test.go @@ -108,6 +108,7 @@ func TestParseTimestamps(t *testing.T) { }{ // unix timestamps {in: "1136073600", expectedS: 1136073600, expectedN: 0}, + {in: "1136073600.", expectedS: 1136073600, expectedN: 0}, // allow empty nanoseconds {in: "1136073600.0", expectedS: 1136073600, expectedN: 0}, {in: "1136073600.000000001", expectedS: 1136073600, expectedN: 1}, {in: "1136073600.0000000010", expectedS: 1136073600, expectedN: 1}, // truncates From 553420b8f3162b40b30b3ff456f7ff21834adfdc Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 25 Feb 2026 15:14:09 +0100 Subject: [PATCH 5/9] daemon/internal/timestamp: replace GetTimestamp for Parse All code in the daemon using `GetTimestamp` called `ParseTimestamp` afterwards to convert the string value to a time.Time. Replace it with a `Parse` function that immediately returns a time.Time (in UTC) so that we don't have to "fuzzy" parse twice. Signed-off-by: Sebastiaan van Stijn --- daemon/containerd/image_list.go | 11 ++----- daemon/events/events_test.go | 20 +++--------- daemon/events/testutils/testutils.go | 11 ++----- daemon/images/image_list.go | 11 +++---- daemon/images/image_prune.go | 8 ++--- daemon/internal/builder-next/builder.go | 11 ++----- daemon/internal/timestamp/timestamp.go | 36 +++++++++------------ daemon/internal/timestamp/timestamp_test.go | 20 ++++-------- daemon/network/filter.go | 10 ++---- daemon/prune.go | 16 +++------ 10 files changed, 48 insertions(+), 106 deletions(-) diff --git a/daemon/containerd/image_list.go b/daemon/containerd/image_list.go index 83578d54cb..5bac143cf4 100644 --- a/daemon/containerd/image_list.go +++ b/daemon/containerd/image_list.go @@ -601,20 +601,15 @@ func (i *ImageService) setupFilters(ctx context.Context, imageFilters filters.Ar return nil, err } + now := time.Now() err = imageFilters.WalkValues("until", func(value string) error { - ts, err := timestamp.GetTimestamp(value, time.Now()) + until, err := timestamp.Parse(value, now) if err != nil { return errdefs.InvalidParameter(fmt.Errorf("invalid value for 'until' filter: %w", err)) } - seconds, nanoseconds, err := timestamp.ParseTimestamps(ts, 0) - if err != nil { - return errdefs.InvalidParameter(fmt.Errorf("invalid value for 'until' filter: %w", err)) - } - until := time.Unix(seconds, nanoseconds) fltrs = append(fltrs, func(image c8dimages.Image) bool { - created := image.CreatedAt - return created.Before(until) + return image.CreatedAt.Before(until) }) return nil }) diff --git a/daemon/events/events_test.go b/daemon/events/events_test.go index e88ee1f997..bdf6e671a2 100644 --- a/daemon/events/events_test.go +++ b/daemon/events/events_test.go @@ -126,10 +126,7 @@ func TestLogEvents(t *testing.T) { // 2016-03-07T17:28:03.129014751+02:00 container destroy 0b863f2a26c18557fc6cdadda007c459f9ec81b874780808138aea78a3595079 (image=ubuntu, name=small_hoover) func TestLoadBufferedEvents(t *testing.T) { now := time.Now() - f, err := timestamp.GetTimestamp("2016-03-07T17:28:03.100000000+02:00", now) - assert.NilError(t, err) - - s, sNano, err := timestamp.ParseTimestamps(f, -1) + since, err := timestamp.Parse("2016-03-07T17:28:03.100000000+02:00", now) assert.NilError(t, err) m1, err := eventstestutils.Scan("2016-03-07T17:28:03.022433271+02:00 container die 0b863f2a26c18557fc6cdadda007c459f9ec81b874780808138aea78a3595079 (image=ubuntu, name=small_hoover)") @@ -145,7 +142,6 @@ func TestLoadBufferedEvents(t *testing.T) { events: []events.Message{*m1, *m2, *m3}, } - since := time.Unix(s, sNano) until := time.Time{} messages := evts.loadBufferedEvents(since, until, nil) @@ -154,16 +150,11 @@ func TestLoadBufferedEvents(t *testing.T) { func TestLoadBufferedEventsOnlyFromPast(t *testing.T) { now := time.Now() - f, err := timestamp.GetTimestamp("2016-03-07T17:28:03.090000000+02:00", now) + + since, err := timestamp.Parse("2016-03-07T17:28:03.090000000+02:00", now) assert.NilError(t, err) - s, sNano, err := timestamp.ParseTimestamps(f, 0) - assert.NilError(t, err) - - f, err = timestamp.GetTimestamp("2016-03-07T17:28:03.100000000+02:00", now) - assert.NilError(t, err) - - u, uNano, err := timestamp.ParseTimestamps(f, 0) + until, err := timestamp.Parse("2016-03-07T17:28:03.100000000+02:00", now) assert.NilError(t, err) m1, err := eventstestutils.Scan("2016-03-07T17:28:03.022433271+02:00 container die 0b863f2a26c18557fc6cdadda007c459f9ec81b874780808138aea78a3595079 (image=ubuntu, name=small_hoover)") @@ -179,9 +170,6 @@ func TestLoadBufferedEventsOnlyFromPast(t *testing.T) { events: []events.Message{*m1, *m2, *m3}, } - since := time.Unix(s, sNano) - until := time.Unix(u, uNano) - messages := evts.loadBufferedEvents(since, until, nil) assert.Assert(t, is.Len(messages, 1)) assert.Check(t, is.Equal(messages[0].Type, events.NetworkEventType)) diff --git a/daemon/events/testutils/testutils.go b/daemon/events/testutils/testutils.go index 282cdc3274..d392669eca 100644 --- a/daemon/events/testutils/testutils.go +++ b/daemon/events/testutils/testutils.go @@ -45,12 +45,7 @@ func Scan(text string) (*events.Message, error) { return nil, fmt.Errorf("text is not an event: %s", text) } - f, err := timestamp.GetTimestamp(md["timestamp"], time.Now()) - if err != nil { - return nil, err - } - - t, tn, err := timestamp.ParseTimestamps(f, -1) + created, err := timestamp.Parse(md["timestamp"], time.Now()) if err != nil { return nil, err } @@ -62,8 +57,8 @@ func Scan(text string) (*events.Message, error) { } return &events.Message{ - Time: t, - TimeNano: time.Unix(t, tn).UnixNano(), + Time: created.Unix(), + TimeNano: created.UnixNano(), Type: events.Type(md["eventType"]), Action: events.Action(md["action"]), Actor: events.Actor{ diff --git a/daemon/images/image_list.go b/daemon/images/image_list.go index d750aebf67..e8e3919505 100644 --- a/daemon/images/image_list.go +++ b/daemon/images/image_list.go @@ -62,17 +62,14 @@ func (i *ImageService) Images(ctx context.Context, opts imagebackend.ListOptions return nil, err } + now := time.Now() err = opts.Filters.WalkValues("until", func(value string) error { - ts, err := timestamp.GetTimestamp(value, time.Now()) + ts, err := timestamp.Parse(value, now) if err != nil { return errdefs.InvalidParameter(fmt.Errorf("invalid value for 'until' filter: %w", err)) } - seconds, nanoseconds, err := timestamp.ParseTimestamps(ts, 0) - if err != nil { - return errdefs.InvalidParameter(fmt.Errorf("invalid value for 'until' filter: %w", err)) - } - if tsUnix := time.Unix(seconds, nanoseconds); beforeFilter.IsZero() || beforeFilter.After(tsUnix) { - beforeFilter = tsUnix + if beforeFilter.IsZero() || beforeFilter.After(ts) { + beforeFilter = ts } return nil }) diff --git a/daemon/images/image_prune.go b/daemon/images/image_prune.go index 102b33e3cb..d56cab634b 100644 --- a/daemon/images/image_prune.go +++ b/daemon/images/image_prune.go @@ -193,13 +193,9 @@ func getUntilFromPruneFilters(pruneFilters filters.Args) (time.Time, error) { if len(untilFilters) > 1 { return time.Time{}, errdefs.InvalidParameter(errors.New("more than one until filter specified")) } - ts, err := timestamp.GetTimestamp(untilFilters[0], time.Now()) + t, err := timestamp.Parse(untilFilters[0], time.Now()) if err != nil { return time.Time{}, errdefs.InvalidParameter(fmt.Errorf("invalid value for 'until' filter: %w", err)) } - seconds, nanoseconds, err := timestamp.ParseTimestamps(ts, 0) - if err != nil { - return time.Time{}, errdefs.InvalidParameter(fmt.Errorf("invalid value for 'until' filter: %w", err)) - } - return time.Unix(seconds, nanoseconds), nil + return t, nil } diff --git a/daemon/internal/builder-next/builder.go b/daemon/internal/builder-next/builder.go index 58a616026c..30f2b80467 100644 --- a/daemon/internal/builder-next/builder.go +++ b/daemon/internal/builder-next/builder.go @@ -674,20 +674,13 @@ func toBuildkitPruneInfo(opts buildbackend.CachePruneOptions) (client.PruneInfo, case 0: // nothing to do case 1: - ts, err := timestamp.GetTimestamp(untilValues[0], time.Now()) + parsed, err := timestamp.Parse(untilValues[0], time.Now()) if err != nil { return client.PruneInfo{}, errInvalidFilterValue{ errors.Wrapf(err, "%q filter expects a duration (e.g., '24h') or a timestamp", filterKey), } } - seconds, nanoseconds, err := timestamp.ParseTimestamps(ts, 0) - if err != nil { - return client.PruneInfo{}, errInvalidFilterValue{ - errors.Wrapf(err, "failed to parse timestamp %q", ts), - } - } - - until = time.Since(time.Unix(seconds, nanoseconds)) + until = time.Since(parsed) default: return client.PruneInfo{}, errMultipleFilterValues{} } diff --git a/daemon/internal/timestamp/timestamp.go b/daemon/internal/timestamp/timestamp.go index 2496f68483..be6848022a 100644 --- a/daemon/internal/timestamp/timestamp.go +++ b/daemon/internal/timestamp/timestamp.go @@ -17,19 +17,17 @@ const ( dateLocal = "2006-01-02" // RFC3339 with local timezone and time at 00:00:00 ) -// GetTimestamp tries to parse given string as golang duration, -// then RFC3339 time and finally as a Unix timestamp. If -// any of these were successful, it returns a Unix timestamp -// as string otherwise returns the given value back. -// In case of duration input, the returned timestamp is computed -// as the given reference time minus the amount of the duration. -func GetTimestamp(value string, reference time.Time) (string, error) { +// Parse tries to parse given string as golang duration, then RFC3339 time and +// finally as a Unix timestamp. The returned time is normalized to UTC. +// +// In case of duration input, the returned timestamp is computed as the given +// reference time minus the amount of the duration. +func Parse(value string, reference time.Time) (time.Time, error) { + if strings.TrimSpace(value) == "" { + return time.Time{}, errors.New("failed to parse value as time or duration: value is empty") + } if d, err := time.ParseDuration(value); value != "0" && err == nil { - t := reference.Add(-d) - if t.Nanosecond() == 0 { - return strconv.FormatInt(t.Unix(), 10), nil - } - return fmt.Sprintf("%d.%09d", t.Unix(), t.Nanosecond()), nil + return reference.Add(-d).UTC(), nil } var format string @@ -88,18 +86,16 @@ func GetTimestamp(value string, reference time.Time) (string, error) { if err != nil { // if there is a `-` then it's an RFC3339 like timestamp if strings.Contains(value, "-") { - return "", err // was probably an RFC3339 like timestamp but the parser failed with an error + return time.Time{}, err // was probably an RFC3339 like timestamp but the parser failed with an error } - if _, _, err := parseTimestamp(value); err != nil { - return "", fmt.Errorf("failed to parse value as time or duration: %q", value) + sec, nsec, err := parseTimestamp(value) + if err != nil { + return time.Time{}, fmt.Errorf("failed to parse value as time or duration: %q", value) } - return value, nil // unix timestamp in and out case (meaning: the value passed at the command line is already in the right format for passing to the server) + return time.Unix(sec, nsec), nil } - if t.Nanosecond() == 0 { - return strconv.FormatInt(t.Unix(), 10), nil - } - return fmt.Sprintf("%d.%09d", t.Unix(), t.Nanosecond()), nil + return t.UTC(), nil } // ParseTimestamps returns seconds and nanoseconds from a timestamp that has diff --git a/daemon/internal/timestamp/timestamp_test.go b/daemon/internal/timestamp/timestamp_test.go index ae71cf834b..2b009a4d3c 100644 --- a/daemon/internal/timestamp/timestamp_test.go +++ b/daemon/internal/timestamp/timestamp_test.go @@ -1,8 +1,6 @@ package timestamp_test import ( - "fmt" - "strconv" "testing" "time" @@ -11,7 +9,7 @@ import ( is "gotest.tools/v3/assert/cmp" ) -func TestGetTimestamp(t *testing.T) { +func TestParse(t *testing.T) { now := time.Date(2020, 1, 2, 3, 4, 5, 123456789, time.UTC) tests := []struct { in string @@ -73,26 +71,20 @@ func TestGetTimestamp(t *testing.T) { name = "" } t.Run(name, func(t *testing.T) { - out, err := timestamp.GetTimestamp(tc.in, now) + out, err := timestamp.Parse(tc.in, now) if tc.expectedErr { assert.Assert(t, err != nil, "expected error for %q, got none", tc.in) return } assert.NilError(t, err) - exp, err := time.Parse(time.RFC3339Nano, tc.expected) + want, err := time.Parse(time.RFC3339Nano, tc.expected) assert.NilError(t, err, "invalid expected value") - var want string - if exp.Nanosecond() == 0 { - want = strconv.FormatInt(exp.Unix(), 10) - } else { - want = fmt.Sprintf("%d.%09d", exp.Unix(), exp.Nanosecond()) - } - assert.Assert(t, out == want, + assert.Assert(t, out.Equal(want), "expected: %s\ngot: %s", - want, - out, + want.Format(time.RFC3339Nano), + out.Format(time.RFC3339Nano), ) }) } diff --git a/daemon/network/filter.go b/daemon/network/filter.go index 5cca048b90..66015f8bf6 100644 --- a/daemon/network/filter.go +++ b/daemon/network/filter.go @@ -96,20 +96,16 @@ func newFilter(args filters.Args) (Filter, error) { if err := args.WalkValues("type", validateNetworkTypeFilter); err != nil { return Filter{}, err } - until := time.Time{} + var until time.Time if untilFilters := args.Get("until"); len(untilFilters) > 0 { if len(untilFilters) > 1 { return Filter{}, errdefs.InvalidParameter(errors.New("more than one until filter specified")) } - ts, err := timestamp.GetTimestamp(untilFilters[0], time.Now()) + var err error + until, err = timestamp.Parse(untilFilters[0], time.Now()) if err != nil { return Filter{}, errdefs.InvalidParameter(err) } - seconds, nanoseconds, err := timestamp.ParseTimestamps(ts, 0) - if err != nil { - return Filter{}, errdefs.InvalidParameter(err) - } - until = time.Unix(seconds, nanoseconds) } return Filter{ args: args, diff --git a/daemon/prune.go b/daemon/prune.go index e2336f5326..7dd21d5a79 100644 --- a/daemon/prune.go +++ b/daemon/prune.go @@ -205,24 +205,18 @@ func (daemon *Daemon) NetworkPrune(ctx context.Context, filterArgs filters.Args) } func getUntilFromPruneFilters(pruneFilters filters.Args) (time.Time, error) { - until := time.Time{} if !pruneFilters.Contains("until") { - return until, nil + return time.Time{}, nil } untilFilters := pruneFilters.Get("until") if len(untilFilters) > 1 { - return until, errdefs.InvalidParameter(errors.New("more than one until filter specified")) + return time.Time{}, errdefs.InvalidParameter(errors.New("more than one until filter specified")) } - ts, err := timestamp.GetTimestamp(untilFilters[0], time.Now()) + t, err := timestamp.Parse(untilFilters[0], time.Now()) if err != nil { - return until, errdefs.InvalidParameter(err) + return time.Time{}, errdefs.InvalidParameter(err) } - seconds, nanoseconds, err := timestamp.ParseTimestamps(ts, 0) - if err != nil { - return until, errdefs.InvalidParameter(err) - } - until = time.Unix(seconds, nanoseconds) - return until, nil + return t, nil } func matchLabels(pruneFilters filters.Args, labels map[string]string) bool { From cfeceed9c09a4d89d8ee0aa68dc76eca35222df8 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 25 Feb 2026 15:50:00 +0100 Subject: [PATCH 6/9] daemon/internal/timestamp: improve unix timestamp parsing Simplify the logic to avoid `math.Pow` and float as intermediate; truncation/pad to 9 digits for nanoseconds and parse as integer. Also return the result as UTC time.Time, in preparation of using strong types for the exported functions. Signed-off-by: Sebastiaan van Stijn --- daemon/internal/timestamp/timestamp.go | 53 +++++++++++++++------ daemon/internal/timestamp/timestamp_test.go | 2 + 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/daemon/internal/timestamp/timestamp.go b/daemon/internal/timestamp/timestamp.go index be6848022a..a52603b950 100644 --- a/daemon/internal/timestamp/timestamp.go +++ b/daemon/internal/timestamp/timestamp.go @@ -1,8 +1,8 @@ package timestamp import ( + "errors" "fmt" - "math" "strconv" "strings" "time" @@ -88,11 +88,10 @@ func Parse(value string, reference time.Time) (time.Time, error) { if strings.Contains(value, "-") { return time.Time{}, err // was probably an RFC3339 like timestamp but the parser failed with an error } - sec, nsec, err := parseTimestamp(value) + t, err = parseTimestamp(value) if err != nil { - return time.Time{}, fmt.Errorf("failed to parse value as time or duration: %q", value) + return time.Time{}, fmt.Errorf("failed to parse value as time or duration: %w", err) } - return time.Unix(sec, nsec), nil } return t.UTC(), nil @@ -112,27 +111,53 @@ func ParseTimestamps(value string, defaultSeconds int64) (seconds int64, nanosec if value == "" { return defaultSeconds, 0, nil } - s, n, err := parseTimestamp(value) + t, err := parseTimestamp(value) if err != nil { return 0, 0, fmt.Errorf("invalid timestamp %q: %w", value, err) } - return s, n, nil + return t.Unix(), int64(t.Nanosecond()), nil } -func parseTimestamp(value string) (seconds int64, nanoseconds int64, _ error) { +func parseTimestamp(value string) (time.Time, error) { s, n, ok := strings.Cut(value, ".") sec, err := strconv.ParseInt(s, 10, 64) if err != nil { - return 0, 0, err + var numErr *strconv.NumError + if errors.As(err, &numErr) { + err = numErr.Err + } + return time.Time{}, fmt.Errorf("invalid seconds %q: %w", s, err) } - if !ok || n == "0" || n == "" { - return sec, 0, nil + if !ok || n == "" || n == "000000000" || n == "0" { + // Fast path for "zero" nanoseconds + return time.Unix(sec, 0).UTC(), nil + } + + // The documented format is 9 digits. Historically we allowed more and + // truncated to nanoseconds precision. Let's add some sensible limit; + // 20 digits would be a full uint64 value. + const maxFracDigits = 20 + if len(n) > maxFracDigits { + return time.Time{}, fmt.Errorf("invalid nanoseconds: length %d exceeds maximum %d", len(n), maxFracDigits) + } + + // Nanoseconds must be digits only (reject '+' / '-' and other junk). + for i := range len(n) { + if c := n[i]; c < '0' || c > '9' { + return time.Time{}, fmt.Errorf("invalid nanoseconds: invalid character %q at position %d", c, i) + } + } + + // cap at 9 digits, or pad to 9 digits + if len(n) > 9 { + n = n[:9] + } else if len(n) < 9 { + n += strings.Repeat("0", 9-len(n)) } nsec, err := strconv.ParseInt(n, 10, 64) if err != nil { - return 0, 0, err + // this should never fail, but just in case + return time.Time{}, fmt.Errorf("invalid nanoseconds %q: %w", n, err) } - // should already be in nanoseconds but just in case convert n to nanoseconds - nsec = int64(float64(nsec) * math.Pow(float64(10), float64(9-len(n)))) - return sec, nsec, nil + return time.Unix(sec, nsec).UTC(), nil } diff --git a/daemon/internal/timestamp/timestamp_test.go b/daemon/internal/timestamp/timestamp_test.go index 2b009a4d3c..4504300811 100644 --- a/daemon/internal/timestamp/timestamp_test.go +++ b/daemon/internal/timestamp/timestamp_test.go @@ -48,6 +48,8 @@ func TestParse(t *testing.T) { // Unix timestamps {in: "1136073600", expected: "2006-01-01T00:00:00Z"}, {in: "1136073600.000000001", expected: "2006-01-01T00:00:00.000000001Z"}, + {in: "1136073600.00000000000000000001", expected: "2006-01-01T00:00:00Z"}, // max length + {in: "1136073600.000000000000000000001", expectedErr: true}, // too long // Durations (relative to `now`) {in: "-1m", expected: "2020-01-02T03:05:05.123456789Z"}, // welcome to the future ¯\_(ツ)_/¯ From 568312cb85e224755e57a00b20150459823c84ca Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 25 Feb 2026 16:22:31 +0100 Subject: [PATCH 7/9] daemon/internal/timestamp: replace ParseTimestamps with ParseUnixTimestamp Return a time.Time instead of splitting to seconds/nanoseconds. Signed-off-by: Sebastiaan van Stijn --- daemon/cluster/services.go | 4 +-- daemon/internal/timestamp/timestamp.go | 29 ++++++++++++-------- daemon/internal/timestamp/timestamp_test.go | 17 +++++++----- daemon/logs.go | 6 ++-- daemon/server/router/system/system_routes.go | 15 ++-------- 5 files changed, 32 insertions(+), 39 deletions(-) diff --git a/daemon/cluster/services.go b/daemon/cluster/services.go index 7e996b127b..cd34fb02b2 100644 --- a/daemon/cluster/services.go +++ b/daemon/cluster/services.go @@ -6,7 +6,6 @@ import ( "io" "os" "strconv" - "time" "github.com/containerd/log" "github.com/distribution/reference" @@ -461,11 +460,10 @@ func (c *Cluster) ServiceLogs(ctx context.Context, selector *backend.LogSelector // get the since value - the time in the past we're looking at logs starting from var sinceProto *gogotypes.Timestamp if config.Since != "" { - s, n, err := timestamp.ParseTimestamps(config.Since, 0) + since, err := timestamp.ParseUnixTimestamp(config.Since) if err != nil { return nil, errors.Wrap(err, "could not parse since timestamp") } - since := time.Unix(s, n) sinceProto, err = gogotypes.TimestampProto(since) if err != nil { return nil, errors.Wrap(err, "could not parse timestamp to proto") diff --git a/daemon/internal/timestamp/timestamp.go b/daemon/internal/timestamp/timestamp.go index a52603b950..4f69c8aaae 100644 --- a/daemon/internal/timestamp/timestamp.go +++ b/daemon/internal/timestamp/timestamp.go @@ -97,25 +97,30 @@ func Parse(value string, reference time.Time) (time.Time, error) { return t.UTC(), nil } -// ParseTimestamps returns seconds and nanoseconds from a timestamp that has -// the format ("%d.%09d", time.Unix(), int64(time.Nanosecond())). -// If the incoming nanosecond portion is longer than 9 digits it is truncated. -// The expectation is that the seconds and nanoseconds will be used to create a -// time variable. For example: +// ParseUnixTimestamp parses a UNIX timestamp with optional nanoseconds +// and returns it as a [time.Time]. // -// seconds, nanoseconds, _ := ParseTimestamp("1136073600.000000001",0) -// since := time.Unix(seconds, nanoseconds) +// Value should be formatted as // -// returns seconds as defaultSeconds if value == "" -func ParseTimestamps(value string, defaultSeconds int64) (seconds int64, nanoseconds int64, _ error) { +// "%d.%09d" (seconds.nanoseconds) +// +// It accepts either: +// - "seconds" +// - "seconds.nanoseconds" +// +// If the nanoseconds has less than 9 digits it is right-padded with zeros. +// If it has more than 9 digits it is truncated. +// +// Empty values ("") produce a zero-value, with no error. +func ParseUnixTimestamp(value string) (time.Time, error) { if value == "" { - return defaultSeconds, 0, nil + return time.Time{}, nil } t, err := parseTimestamp(value) if err != nil { - return 0, 0, fmt.Errorf("invalid timestamp %q: %w", value, err) + return time.Time{}, fmt.Errorf("invalid timestamp %q: %w", value, err) } - return t.Unix(), int64(t.Nanosecond()), nil + return t, nil } func parseTimestamp(value string) (time.Time, error) { diff --git a/daemon/internal/timestamp/timestamp_test.go b/daemon/internal/timestamp/timestamp_test.go index 4504300811..7453d058e2 100644 --- a/daemon/internal/timestamp/timestamp_test.go +++ b/daemon/internal/timestamp/timestamp_test.go @@ -92,10 +92,9 @@ func TestParse(t *testing.T) { } } -func TestParseTimestamps(t *testing.T) { +func TestParseUnixTimestamp(t *testing.T) { tests := []struct { in string - def int64 expectedS int64 expectedN int64 expectedErr bool @@ -117,8 +116,8 @@ func TestParseTimestamps(t *testing.T) { {in: ".0000000009", expectedErr: true}, {in: "1136073600.bar", expectedErr: true}, - // default value - {in: "", def: -1, expectedS: -1, expectedN: 0}, + // empty value + {in: ""}, } for _, tc := range tests { @@ -127,14 +126,18 @@ func TestParseTimestamps(t *testing.T) { name = "" } t.Run(name, func(t *testing.T) { - s, n, err := timestamp.ParseTimestamps(tc.in, tc.def) + out, err := timestamp.ParseUnixTimestamp(tc.in) if tc.expectedErr { assert.Assert(t, err != nil, "expected error for %q, got none", tc.in) return } assert.NilError(t, err) - assert.Check(t, is.Equal(s, tc.expectedS)) - assert.Check(t, is.Equal(n, tc.expectedN)) + if tc.in == "" { + assert.Assert(t, out.IsZero()) + return + } + assert.Check(t, is.Equal(out.Unix(), tc.expectedS)) + assert.Check(t, is.Equal(int64(out.Nanosecond()), tc.expectedN)) }) } } diff --git a/daemon/logs.go b/daemon/logs.go index fa5d9c639f..b56fe90d38 100644 --- a/daemon/logs.go +++ b/daemon/logs.go @@ -79,20 +79,18 @@ func (daemon *Daemon) ContainerLogs(ctx context.Context, containerName string, c var since time.Time if config.Since != "" { - s, n, err := timestamp.ParseTimestamps(config.Since, 0) + since, err = timestamp.ParseUnixTimestamp(config.Since) if err != nil { return nil, false, err } - since = time.Unix(s, n) } var until time.Time if config.Until != "" && config.Until != "0" { - s, n, err := timestamp.ParseTimestamps(config.Until, 0) + until, err = timestamp.ParseUnixTimestamp(config.Until) if err != nil { return nil, false, err } - until = time.Unix(s, n) } follow := config.Follow && !cLogCreated diff --git a/daemon/server/router/system/system_routes.go b/daemon/server/router/system/system_routes.go index e43bc5cfa1..0ed371b612 100644 --- a/daemon/server/router/system/system_routes.go +++ b/daemon/server/router/system/system_routes.go @@ -295,11 +295,11 @@ func (s *systemRouter) getEvents(ctx context.Context, w http.ResponseWriter, r * return err } - since, err := eventTime(r.Form.Get("since")) + since, err := timestamp.ParseUnixTimestamp(r.Form.Get("since")) if err != nil { return invalidRequestError{fmt.Errorf("invalid value for 'since': %w", err)} } - until, err := eventTime(r.Form.Get("until")) + until, err := timestamp.ParseUnixTimestamp(r.Form.Get("until")) if err != nil { return invalidRequestError{fmt.Errorf("invalid value for 'until': %w", err)} } @@ -430,17 +430,6 @@ func (s *systemRouter) postAuth(ctx context.Context, w http.ResponseWriter, r *h }) } -func eventTime(formTime string) (time.Time, error) { - t, tNano, err := timestamp.ParseTimestamps(formTime, -1) - if err != nil { - return time.Time{}, err - } - if t == -1 { - return time.Time{}, nil - } - return time.Unix(t, tNano), nil -} - // These fields were deprecated in docker v1.10, API v1.22, but not removed // from the API responses. Unfortunately, the Docker CLI (and compose indirectly), // continued using these fields up until v25.0.0, and panic if the fields are From 210cdcbd465a7022e043cd9603b74b1b87ac888d Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 25 Feb 2026 17:00:35 +0100 Subject: [PATCH 8/9] daemon/server/router: fix "no stream selected" error and status Tweak the wording, and make sure we return a 400 status, not a 500 Signed-off-by: Sebastiaan van Stijn --- daemon/server/router/container/container_routes.go | 2 +- daemon/server/router/swarm/helpers.go | 3 ++- integration-cli/docker_api_logs_test.go | 4 +++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/daemon/server/router/container/container_routes.go b/daemon/server/router/container/container_routes.go index b6eea329e8..a4cd242310 100644 --- a/daemon/server/router/container/container_routes.go +++ b/daemon/server/router/container/container_routes.go @@ -192,7 +192,7 @@ func (c *containerRouter) getContainersLogs(ctx context.Context, w http.Response // with the appropriate status code. stdout, stderr := httputils.BoolValue(r, "stdout"), httputils.BoolValue(r, "stderr") if !stdout && !stderr { - return errdefs.InvalidParameter(errors.New("Bad parameters: you must choose at least one stream")) + return errdefs.InvalidParameter(errors.New("must specify at least one of 'stdout' or 'stderr'")) } containerName := vars["name"] diff --git a/daemon/server/router/swarm/helpers.go b/daemon/server/router/swarm/helpers.go index ceeea17489..a52ffdfaf8 100644 --- a/daemon/server/router/swarm/helpers.go +++ b/daemon/server/router/swarm/helpers.go @@ -11,6 +11,7 @@ import ( "github.com/moby/moby/v2/daemon/server/backend" "github.com/moby/moby/v2/daemon/server/httputils" "github.com/moby/moby/v2/daemon/server/httputils/logstream" + "github.com/moby/moby/v2/errdefs" ) // swarmLogs takes an http response, request, and selector, and writes the logs @@ -23,7 +24,7 @@ func (sr *swarmRouter) swarmLogs(ctx context.Context, w http.ResponseWriter, r * // with the appropriate status code. stdout, stderr := httputils.BoolValue(r, "stdout"), httputils.BoolValue(r, "stderr") if !stdout && !stderr { - return errors.New("Bad parameters: you must choose at least one stream") + return errdefs.InvalidParameter(errors.New("must specify at least one of 'stdout' or 'stderr'")) } // there is probably a neater way to manufacture the LogsOptions diff --git a/integration-cli/docker_api_logs_test.go b/integration-cli/docker_api_logs_test.go index ff23250ec2..e9e47734cb 100644 --- a/integration-cli/docker_api_logs_test.go +++ b/integration-cli/docker_api_logs_test.go @@ -11,6 +11,7 @@ import ( "testing" "time" + cerrdefs "github.com/containerd/errdefs" "github.com/moby/moby/api/pkg/stdcopy" "github.com/moby/moby/client" "github.com/moby/moby/v2/integration-cli/cli" @@ -63,7 +64,8 @@ func (s *DockerAPISuite) TestLogsAPINoStdoutNorStderr(c *testing.T) { defer apiClient.Close() _, err = apiClient.ContainerLogs(testutil.GetContext(c), name, client.ContainerLogsOptions{}) - assert.ErrorContains(c, err, "Bad parameters: you must choose at least one stream") + assert.ErrorType(c, err, cerrdefs.IsInvalidArgument) + assert.ErrorContains(c, err, "must specify at least one of 'stdout' or 'stderr'") } // Regression test for #12704 From f401a9678bc0a16adb4819093eb10cf95f6b4e24 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 25 Feb 2026 17:02:47 +0100 Subject: [PATCH 9/9] daemon/server/backend: ContainerLogsOptions: use time.Time for since, until Move parsing the wire format to the router instead of in the daemon, so that a strong type can be used internally. Signed-off-by: Sebastiaan van Stijn --- daemon/cluster/executor/container/adapter.go | 5 +--- daemon/cluster/services.go | 10 +++----- daemon/logs.go | 22 ++--------------- daemon/server/backend/backend.go | 4 ++-- .../router/container/container_routes.go | 24 +++++++++++++++++-- daemon/server/router/swarm/helpers.go | 14 ++++++++++- 6 files changed, 43 insertions(+), 36 deletions(-) diff --git a/daemon/cluster/executor/container/adapter.go b/daemon/cluster/executor/container/adapter.go index 3964284446..81911c308e 100644 --- a/daemon/cluster/executor/container/adapter.go +++ b/daemon/cluster/executor/container/adapter.go @@ -526,10 +526,7 @@ func (c *containerAdapter) logs(ctx context.Context, options api.LogSubscription if err != nil { return nil, err } - // print since as this formatted string because the docker container - // logs interface expects it like this. - // see [github.com/moby/moby/v2/daemon/internal/timestamp.ParseTimestamps] - apiOptions.Since = fmt.Sprintf("%d.%09d", since.Unix(), int64(since.Nanosecond())) + apiOptions.Since = since } if options.Tail < 0 { diff --git a/daemon/cluster/services.go b/daemon/cluster/services.go index cd34fb02b2..c1d0f42274 100644 --- a/daemon/cluster/services.go +++ b/daemon/cluster/services.go @@ -14,7 +14,6 @@ import ( "github.com/moby/moby/api/types/registry" "github.com/moby/moby/api/types/swarm" "github.com/moby/moby/v2/daemon/cluster/convert" - "github.com/moby/moby/v2/daemon/internal/timestamp" "github.com/moby/moby/v2/daemon/server/backend" "github.com/moby/moby/v2/daemon/server/swarmbackend" "github.com/moby/moby/v2/errdefs" @@ -459,12 +458,9 @@ func (c *Cluster) ServiceLogs(ctx context.Context, selector *backend.LogSelector // get the since value - the time in the past we're looking at logs starting from var sinceProto *gogotypes.Timestamp - if config.Since != "" { - since, err := timestamp.ParseUnixTimestamp(config.Since) - if err != nil { - return nil, errors.Wrap(err, "could not parse since timestamp") - } - sinceProto, err = gogotypes.TimestampProto(since) + if !config.Since.IsZero() { + var err error + sinceProto, err = gogotypes.TimestampProto(config.Since) if err != nil { return nil, errors.Wrap(err, "could not parse timestamp to proto") } diff --git a/daemon/logs.go b/daemon/logs.go index b56fe90d38..051cb45c19 100644 --- a/daemon/logs.go +++ b/daemon/logs.go @@ -3,14 +3,12 @@ package daemon import ( "context" "strconv" - "time" "github.com/containerd/containerd/v2/pkg/tracing" "github.com/containerd/log" containertypes "github.com/moby/moby/api/types/container" "github.com/moby/moby/v2/daemon/config" "github.com/moby/moby/v2/daemon/container" - "github.com/moby/moby/v2/daemon/internal/timestamp" "github.com/moby/moby/v2/daemon/logger" logcache "github.com/moby/moby/v2/daemon/logger/loggerutils/cache" "github.com/moby/moby/v2/daemon/server/backend" @@ -77,26 +75,10 @@ func (daemon *Daemon) ContainerLogs(ctx context.Context, containerName string, c tailLines = -1 } - var since time.Time - if config.Since != "" { - since, err = timestamp.ParseUnixTimestamp(config.Since) - if err != nil { - return nil, false, err - } - } - - var until time.Time - if config.Until != "" && config.Until != "0" { - until, err = timestamp.ParseUnixTimestamp(config.Until) - if err != nil { - return nil, false, err - } - } - follow := config.Follow && !cLogCreated logs := logReader.ReadLogs(ctx, logger.ReadConfig{ - Since: since, - Until: until, + Since: config.Since, + Until: config.Until, Tail: tailLines, Follow: follow, }) diff --git a/daemon/server/backend/backend.go b/daemon/server/backend/backend.go index a6cb5e0c75..f99bc958ca 100644 --- a/daemon/server/backend/backend.go +++ b/daemon/server/backend/backend.go @@ -112,8 +112,8 @@ type ContainerListOptions struct { type ContainerLogsOptions struct { ShowStdout bool ShowStderr bool - Since string - Until string + Since time.Time + Until time.Time Timestamps bool Follow bool Tail string diff --git a/daemon/server/router/container/container_routes.go b/daemon/server/router/container/container_routes.go index a4cd242310..1d8145faf8 100644 --- a/daemon/server/router/container/container_routes.go +++ b/daemon/server/router/container/container_routes.go @@ -11,6 +11,7 @@ import ( "slices" "strconv" "strings" + "time" "github.com/containerd/log" "github.com/containerd/platforms" @@ -20,6 +21,7 @@ import ( "github.com/moby/moby/api/types/network" "github.com/moby/moby/v2/daemon/internal/filters" "github.com/moby/moby/v2/daemon/internal/runconfig" + "github.com/moby/moby/v2/daemon/internal/timestamp" "github.com/moby/moby/v2/daemon/internal/versions" "github.com/moby/moby/v2/daemon/libnetwork/netlabel" networkSettings "github.com/moby/moby/v2/daemon/network" @@ -195,12 +197,30 @@ func (c *containerRouter) getContainersLogs(ctx context.Context, w http.Response return errdefs.InvalidParameter(errors.New("must specify at least one of 'stdout' or 'stderr'")) } + var since time.Time + if v := r.Form.Get("since"); v != "" { + var err error + since, err = timestamp.ParseUnixTimestamp(v) + if err != nil { + return errdefs.InvalidParameter(fmt.Errorf(`invalid value for "since": %w`, err)) + } + } + + var until time.Time + if v := r.Form.Get("until"); v != "" && v != "0" { + var err error + until, err = timestamp.ParseUnixTimestamp(v) + if err != nil { + return errdefs.InvalidParameter(fmt.Errorf(`invalid value for "until": %w`, err)) + } + } + containerName := vars["name"] logsConfig := &backend.ContainerLogsOptions{ Follow: httputils.BoolValue(r, "follow"), Timestamps: httputils.BoolValue(r, "timestamps"), - Since: r.Form.Get("since"), - Until: r.Form.Get("until"), + Since: since, + Until: until, Tail: r.Form.Get("tail"), ShowStdout: stdout, ShowStderr: stderr, diff --git a/daemon/server/router/swarm/helpers.go b/daemon/server/router/swarm/helpers.go index a52ffdfaf8..a10c0ec412 100644 --- a/daemon/server/router/swarm/helpers.go +++ b/daemon/server/router/swarm/helpers.go @@ -3,10 +3,13 @@ package swarm import ( "context" "errors" + "fmt" "net/http" + "time" basictypes "github.com/moby/moby/api/types" "github.com/moby/moby/api/types/swarm" + "github.com/moby/moby/v2/daemon/internal/timestamp" "github.com/moby/moby/v2/daemon/internal/versions" "github.com/moby/moby/v2/daemon/server/backend" "github.com/moby/moby/v2/daemon/server/httputils" @@ -27,12 +30,21 @@ func (sr *swarmRouter) swarmLogs(ctx context.Context, w http.ResponseWriter, r * return errdefs.InvalidParameter(errors.New("must specify at least one of 'stdout' or 'stderr'")) } + var since time.Time + if s := r.Form.Get("since"); s != "" { + var err error + since, err = timestamp.ParseUnixTimestamp(s) + if err != nil { + return errdefs.InvalidParameter(fmt.Errorf(`invalid value for "since": %w`, err)) + } + } + // there is probably a neater way to manufacture the LogsOptions // struct, probably in the caller, to eliminate the dependency on net/http logsConfig := &backend.ContainerLogsOptions{ Follow: httputils.BoolValue(r, "follow"), Timestamps: httputils.BoolValue(r, "timestamps"), - Since: r.Form.Get("since"), + Since: since, Tail: r.Form.Get("tail"), ShowStdout: stdout, ShowStderr: stderr,