fix(dump): parse dumped time strings so restore works on MySQL and MariaDB

Dumps store datetime columns as RFC3339 strings (e.g. 2026-03-27T11:27:01Z). On restore these were passed verbatim to the database, which MySQL and MariaDB reject with 'Error 1292 Incorrect datetime value'. Parse them into time.Time so the driver formats them for the target database. Also guard against a nil pointer dereference when a dump contains a column that no longer exists in the current schema - such columns are now dropped with a warning instead of crashing. Fixes https://github.com/go-vikunja/vikunja/issues/3375
2026-08-01 18:54:36 -05:00 · 2026-07-30 21:15:19 +02:00
parent 4abda62a99
commit 0ad2e70b3a
2 changed files with 176 additions and 3 deletions
@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"regexp"
 	"strings"
+	"time"

 	"code.vikunja.io/api/pkg/log"

@@ -29,6 +30,27 @@ import (

 var validTableName = regexp.MustCompile(`^[a-zA-Z_][a-zA-Z0-9_]*$`)

+// Formats a dumped time value may be in, depending on the database and driver the dump was created with.
+var dumpTimeFormats = []string{
+	time.RFC3339Nano,
+	"2006-01-02 15:04:05.999999999Z07:00",
+	"2006-01-02 15:04:05.999999999",
+	"2006-01-02T15:04:05.999999999",
+	"2006-01-02",
+}
+
+// parseDumpTime parses a time string from a dump. Values without a timezone are
+// interpreted as UTC since that's what dumps are written in.
+func parseDumpTime(value string) (t time.Time, err error) {
+	for _, format := range dumpTimeFormats {
+		t, err = time.ParseInLocation(format, value, time.UTC)
+		if err == nil {
+			return t, nil
+		}
+	}
+	return t, err
+}
+
 func validateTableName(table string) error {
 	if !validTableName.MatchString(table) {
 		return fmt.Errorf("invalid table name: %q", table)
@@ -85,14 +107,33 @@ func Restore(table string, contents []map[string]interface{}) (err error) {

 	for _, content := range contents {
 		for colName, value := range content {
+			col := metaForCurrentTable.GetColumn(colName)
+			if col == nil {
+				log.Warningf("Column %s does not exist in table %s, dropping it from the restored data", colName, table)
+				delete(content, colName)
+				continue
+			}
+
+			strVal, is := value.(string)
+			if !is || !col.SQLType.IsTime() {
+				continue
+			}
+
 			// Date fields might get restored as 0001-01-01 from null dates. This can have unintended side-effects like
 			// users being scheduled for deletion after a restore.
 			// To avoid this, we set these dates to nil so that they'll end up as null in the db.
-			col := metaForCurrentTable.GetColumn(colName)
-			strVal, is := value.(string)
-			if is && col.SQLType.IsTime() && (strVal == "" || strings.HasPrefix(strVal, "0001-")) {
+			if strVal == "" || strings.HasPrefix(strVal, "0001-") {
 				content[colName] = nil
+				continue
 			}
+
+			// Dumps contain dates as RFC3339 strings, which MySQL and MariaDB reject ("Incorrect
+			// datetime value"). Convert to time.Time and let the driver format it for the target db.
+			t, err := parseDumpTime(strVal)
+			if err != nil {
+				return fmt.Errorf("could not parse time value %q for column %s in table %s: %w", strVal, colName, table, err)
+			}
+			content[colName] = t
 		}

 		if _, err := x.Table(table).Insert(content); err != nil {
@@ -0,0 +1,132 @@
+// Vikunja is a to-do list application to facilitate your life.
+// Copyright 2018-present Vikunja and contributors. All rights reserved.
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+package db
+
+import (
+	"testing"
+	"time"
+
+	"code.vikunja.io/api/pkg/config"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestParseDumpTime(t *testing.T) {
+	t.Run("rfc3339 utc", func(t *testing.T) {
+		parsed, err := parseDumpTime("2026-03-27T11:27:01Z")
+		require.NoError(t, err)
+		assert.Equal(t, time.Date(2026, 3, 27, 11, 27, 1, 0, time.UTC), parsed.UTC())
+	})
+
+	t.Run("rfc3339 with offset", func(t *testing.T) {
+		parsed, err := parseDumpTime("2024-02-06T08:32:51+01:00")
+		require.NoError(t, err)
+		assert.Equal(t, time.Date(2024, 2, 6, 7, 32, 51, 0, time.UTC), parsed.UTC())
+	})
+
+	t.Run("rfc3339 with nanoseconds", func(t *testing.T) {
+		parsed, err := parseDumpTime("2026-03-27T11:27:01.123456789Z")
+		require.NoError(t, err)
+		assert.Equal(t, time.Date(2026, 3, 27, 11, 27, 1, 123456789, time.UTC), parsed.UTC())
+	})
+
+	t.Run("space separated without timezone", func(t *testing.T) {
+		parsed, err := parseDumpTime("2026-03-27 11:27:01")
+		require.NoError(t, err)
+		assert.Equal(t, time.Date(2026, 3, 27, 11, 27, 1, 0, time.UTC), parsed.UTC())
+	})
+
+	t.Run("t separated without timezone", func(t *testing.T) {
+		parsed, err := parseDumpTime("2026-03-27T11:27:01")
+		require.NoError(t, err)
+		assert.Equal(t, time.Date(2026, 3, 27, 11, 27, 1, 0, time.UTC), parsed.UTC())
+	})
+
+	t.Run("date only", func(t *testing.T) {
+		parsed, err := parseDumpTime("2026-03-27")
+		require.NoError(t, err)
+		assert.Equal(t, time.Date(2026, 3, 27, 0, 0, 0, 0, time.UTC), parsed.UTC())
+	})
+
+	t.Run("invalid", func(t *testing.T) {
+		_, err := parseDumpTime("not-a-date")
+		require.Error(t, err)
+	})
+}
+
+type dumpRestoreTest struct {
+	ID      int64     `xorm:"autoincr not null unique pk"`
+	Title   string    `xorm:"varchar(250)"`
+	Created time.Time `xorm:"not null"`
+	Updated time.Time `xorm:""`
+}
+
+func TestRestore(t *testing.T) {
+	config.InitDefaultConfig()
+	engine, err := CreateTestEngine()
+	require.NoError(t, err)
+	require.NoError(t, engine.Sync(&dumpRestoreTest{}))
+
+	t.Run("time values from a dump", func(t *testing.T) {
+		err := RestoreAndTruncate("dump_restore_test", []map[string]interface{}{
+			{
+				"id":      int64(1),
+				"title":   "test",
+				"created": "2026-03-27T11:27:01Z",
+				"updated": "0001-01-01T00:00:00Z",
+			},
+		})
+		require.NoError(t, err)
+
+		row := &dumpRestoreTest{ID: 1}
+		has, err := engine.Get(row)
+		require.NoError(t, err)
+		require.True(t, has)
+		assert.Equal(t, time.Date(2026, 3, 27, 11, 27, 1, 0, time.UTC).Unix(), row.Created.Unix())
+		assert.True(t, row.Updated.IsZero())
+	})
+
+	t.Run("unknown columns are dropped", func(t *testing.T) {
+		err := RestoreAndTruncate("dump_restore_test", []map[string]interface{}{
+			{
+				"id":             int64(2),
+				"title":          "test2",
+				"created":        "2026-03-27 11:27:01",
+				"does_not_exist": "some value",
+			},
+		})
+		require.NoError(t, err)
+
+		row := &dumpRestoreTest{ID: 2}
+		has, err := engine.Get(row)
+		require.NoError(t, err)
+		require.True(t, has)
+		assert.Equal(t, "test2", row.Title)
+	})
+
+	t.Run("invalid time value", func(t *testing.T) {
+		err := RestoreAndTruncate("dump_restore_test", []map[string]interface{}{
+			{
+				"id":      int64(3),
+				"created": "definitely-not-a-date",
+			},
+		})
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "could not parse time value")
+	})
+}