From 4b8b2141084721ef87698dc4fb85e8491c5f8d7a Mon Sep 17 00:00:00 2001 From: zeripath Date: Mon, 27 Sep 2021 18:30:11 +0100 Subject: [PATCH] Create doctor command to fix repo_units broken by dumps from 1.14.3-1.14.6 (#17136) (#17137) Backport #17136 There was a serious issue with the `gitea dump` command in 1.14.3-1.14.6 which led to corruption of the `config` field of the `repo_unit` table. This PR adds a doctor command to attempt to fix the broken repo_units. Users affected by #16961 should run: ``` gitea doctor --fix --run fix-broken-repo-units ``` Fix #16961 Signed-off-by: Andrew Thornton --- models/login_source.go | 16 +- models/repo_unit.go | 16 +- modules/doctor/fix16961.go | 317 ++++++++++++++++++++++++++++++++ modules/doctor/fix16961_test.go | 271 +++++++++++++++++++++++++++ 4 files changed, 607 insertions(+), 13 deletions(-) create mode 100644 modules/doctor/fix16961.go create mode 100644 modules/doctor/fix16961_test.go diff --git a/models/login_source.go b/models/login_source.go index 5674196e0c..5897743269 100644 --- a/models/login_source.go +++ b/models/login_source.go @@ -71,9 +71,9 @@ var ( _ convert.Conversion = &SSPIConfig{} ) -// jsonUnmarshalHandleDoubleEncode - due to a bug in xorm (see https://gitea.com/xorm/xorm/pulls/1957) - it's +// JSONUnmarshalHandleDoubleEncode - due to a bug in xorm (see https://gitea.com/xorm/xorm/pulls/1957) - it's // possible that a Blob may be double encoded or gain an unwanted prefix of 0xff 0xfe. -func jsonUnmarshalHandleDoubleEncode(bs []byte, v interface{}) error { +func JSONUnmarshalHandleDoubleEncode(bs []byte, v interface{}) error { json := jsoniter.ConfigCompatibleWithStandardLibrary err := json.Unmarshal(bs, v) if err != nil { @@ -89,7 +89,7 @@ func jsonUnmarshalHandleDoubleEncode(bs []byte, v interface{}) error { rs = append(rs, temp...) } if ok { - if rs[0] == 0xff && rs[1] == 0xfe { + if len(rs) > 1 && rs[0] == 0xff && rs[1] == 0xfe { rs = rs[2:] } err = json.Unmarshal(rs, v) @@ -108,7 +108,7 @@ type LDAPConfig struct { // FromDB fills up a LDAPConfig from serialized format. func (cfg *LDAPConfig) FromDB(bs []byte) error { - err := jsonUnmarshalHandleDoubleEncode(bs, &cfg) + err := JSONUnmarshalHandleDoubleEncode(bs, &cfg) if err != nil { return err } @@ -149,7 +149,7 @@ type SMTPConfig struct { // FromDB fills up an SMTPConfig from serialized format. func (cfg *SMTPConfig) FromDB(bs []byte) error { - return jsonUnmarshalHandleDoubleEncode(bs, cfg) + return JSONUnmarshalHandleDoubleEncode(bs, cfg) } // ToDB exports an SMTPConfig to a serialized format. @@ -166,7 +166,7 @@ type PAMConfig struct { // FromDB fills up a PAMConfig from serialized format. func (cfg *PAMConfig) FromDB(bs []byte) error { - return jsonUnmarshalHandleDoubleEncode(bs, cfg) + return JSONUnmarshalHandleDoubleEncode(bs, cfg) } // ToDB exports a PAMConfig to a serialized format. @@ -187,7 +187,7 @@ type OAuth2Config struct { // FromDB fills up an OAuth2Config from serialized format. func (cfg *OAuth2Config) FromDB(bs []byte) error { - return jsonUnmarshalHandleDoubleEncode(bs, cfg) + return JSONUnmarshalHandleDoubleEncode(bs, cfg) } // ToDB exports an SMTPConfig to a serialized format. @@ -207,7 +207,7 @@ type SSPIConfig struct { // FromDB fills up an SSPIConfig from serialized format. func (cfg *SSPIConfig) FromDB(bs []byte) error { - return jsonUnmarshalHandleDoubleEncode(bs, cfg) + return JSONUnmarshalHandleDoubleEncode(bs, cfg) } // ToDB exports an SSPIConfig to a serialized format. diff --git a/models/repo_unit.go b/models/repo_unit.go index f430e4f7f3..143ba1a42d 100644 --- a/models/repo_unit.go +++ b/models/repo_unit.go @@ -28,7 +28,7 @@ type UnitConfig struct{} // FromDB fills up a UnitConfig from serialized format. func (cfg *UnitConfig) FromDB(bs []byte) error { - return jsonUnmarshalHandleDoubleEncode(bs, &cfg) + return JSONUnmarshalHandleDoubleEncode(bs, &cfg) } // ToDB exports a UnitConfig to a serialized format. @@ -44,7 +44,7 @@ type ExternalWikiConfig struct { // FromDB fills up a ExternalWikiConfig from serialized format. func (cfg *ExternalWikiConfig) FromDB(bs []byte) error { - return jsonUnmarshalHandleDoubleEncode(bs, &cfg) + return JSONUnmarshalHandleDoubleEncode(bs, &cfg) } // ToDB exports a ExternalWikiConfig to a serialized format. @@ -62,7 +62,7 @@ type ExternalTrackerConfig struct { // FromDB fills up a ExternalTrackerConfig from serialized format. func (cfg *ExternalTrackerConfig) FromDB(bs []byte) error { - return jsonUnmarshalHandleDoubleEncode(bs, &cfg) + return JSONUnmarshalHandleDoubleEncode(bs, &cfg) } // ToDB exports a ExternalTrackerConfig to a serialized format. @@ -80,7 +80,7 @@ type IssuesConfig struct { // FromDB fills up a IssuesConfig from serialized format. func (cfg *IssuesConfig) FromDB(bs []byte) error { - return jsonUnmarshalHandleDoubleEncode(bs, &cfg) + return JSONUnmarshalHandleDoubleEncode(bs, &cfg) } // ToDB exports a IssuesConfig to a serialized format. @@ -104,7 +104,7 @@ type PullRequestsConfig struct { // FromDB fills up a PullRequestsConfig from serialized format. func (cfg *PullRequestsConfig) FromDB(bs []byte) error { - return jsonUnmarshalHandleDoubleEncode(bs, &cfg) + return JSONUnmarshalHandleDoubleEncode(bs, &cfg) } // ToDB exports a PullRequestsConfig to a serialized format. @@ -219,3 +219,9 @@ func getUnitsByRepoID(e Engine, repoID int64) (units []*RepoUnit, err error) { return units, nil } + +// UpdateRepoUnit updates the provided repo unit +func UpdateRepoUnit(unit *RepoUnit) error { + _, err := x.ID(unit.ID).Update(unit) + return err +} diff --git a/modules/doctor/fix16961.go b/modules/doctor/fix16961.go new file mode 100644 index 0000000000..07cfc4633a --- /dev/null +++ b/modules/doctor/fix16961.go @@ -0,0 +1,317 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package doctor + +import ( + "bytes" + "fmt" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/timeutil" + "xorm.io/builder" +) + +// #16831 revealed that the dump command that was broken in 1.14.3-1.14.6 and 1.15.0 (#15885). +// This led to repo_unit and login_source cfg not being converted to JSON in the dump +// Unfortunately although it was hoped that there were only a few users affected it +// appears that many users are affected. + +// We therefore need to provide a doctor command to fix this repeated issue #16961 + +func parseBool16961(bs []byte) (bool, error) { + if bytes.EqualFold(bs, []byte("%!s(bool=false)")) { + return false, nil + } + + if bytes.EqualFold(bs, []byte("%!s(bool=true)")) { + return true, nil + } + + return false, fmt.Errorf("unexpected bool format: %s", string(bs)) +} + +func fixUnitConfig16961(bs []byte, cfg *models.UnitConfig) (fixed bool, err error) { + err = models.JSONUnmarshalHandleDoubleEncode(bs, &cfg) + if err == nil { + return + } + + // Handle #16961 + if string(bs) != "&{}" && len(bs) != 0 { + return + } + + return true, nil +} + +func fixExternalWikiConfig16961(bs []byte, cfg *models.ExternalWikiConfig) (fixed bool, err error) { + err = models.JSONUnmarshalHandleDoubleEncode(bs, &cfg) + if err == nil { + return + } + + if len(bs) < 3 { + return + } + if bs[0] != '&' || bs[1] != '{' || bs[len(bs)-1] != '}' { + return + } + cfg.ExternalWikiURL = string(bs[2 : len(bs)-1]) + return true, nil +} + +func fixExternalTrackerConfig16961(bs []byte, cfg *models.ExternalTrackerConfig) (fixed bool, err error) { + err = models.JSONUnmarshalHandleDoubleEncode(bs, &cfg) + if err == nil { + return + } + // Handle #16961 + if len(bs) < 3 { + return + } + + if bs[0] != '&' || bs[1] != '{' || bs[len(bs)-1] != '}' { + return + } + + parts := bytes.Split(bs[2:len(bs)-1], []byte{' '}) + if len(parts) != 3 { + return + } + + cfg.ExternalTrackerURL = string(bytes.Join(parts[:len(parts)-2], []byte{' '})) + cfg.ExternalTrackerFormat = string(parts[len(parts)-2]) + cfg.ExternalTrackerStyle = string(parts[len(parts)-1]) + return true, nil +} + +func fixPullRequestsConfig16961(bs []byte, cfg *models.PullRequestsConfig) (fixed bool, err error) { + err = models.JSONUnmarshalHandleDoubleEncode(bs, &cfg) + if err == nil { + return + } + + // Handle #16961 + if len(bs) < 3 { + return + } + + if bs[0] != '&' || bs[1] != '{' || bs[len(bs)-1] != '}' { + return + } + + // PullRequestsConfig was the following in 1.14 + // type PullRequestsConfig struct { + // IgnoreWhitespaceConflicts bool + // AllowMerge bool + // AllowRebase bool + // AllowRebaseMerge bool + // AllowSquash bool + // AllowManualMerge bool + // AutodetectManualMerge bool + // } + // + // 1.15 added in addition: + // DefaultDeleteBranchAfterMerge bool + // DefaultMergeStyle MergeStyle + parts := bytes.Split(bs[2:len(bs)-1], []byte{' '}) + if len(parts) < 7 { + return + } + + var parseErr error + cfg.IgnoreWhitespaceConflicts, parseErr = parseBool16961(parts[0]) + if parseErr != nil { + return + } + cfg.AllowMerge, parseErr = parseBool16961(parts[1]) + if parseErr != nil { + return + } + cfg.AllowRebase, parseErr = parseBool16961(parts[2]) + if parseErr != nil { + return + } + cfg.AllowRebaseMerge, parseErr = parseBool16961(parts[3]) + if parseErr != nil { + return + } + cfg.AllowSquash, parseErr = parseBool16961(parts[4]) + if parseErr != nil { + return + } + cfg.AllowManualMerge, parseErr = parseBool16961(parts[5]) + if parseErr != nil { + return + } + cfg.AutodetectManualMerge, parseErr = parseBool16961(parts[6]) + if parseErr != nil { + return + } + + // 1.14 unit + if len(parts) == 7 { + return true, nil + } + + if len(parts) < 9 { + return + } + + cfg.DefaultDeleteBranchAfterMerge, parseErr = parseBool16961(parts[7]) + if parseErr != nil { + return + } + + cfg.DefaultMergeStyle = models.MergeStyle(string(bytes.Join(parts[8:], []byte{' '}))) + return true, nil +} + +func fixIssuesConfig16961(bs []byte, cfg *models.IssuesConfig) (fixed bool, err error) { + err = models.JSONUnmarshalHandleDoubleEncode(bs, &cfg) + if err == nil { + return + } + + // Handle #16961 + if len(bs) < 3 { + return + } + + if bs[0] != '&' || bs[1] != '{' || bs[len(bs)-1] != '}' { + return + } + + parts := bytes.Split(bs[2:len(bs)-1], []byte{' '}) + if len(parts) != 3 { + return + } + var parseErr error + cfg.EnableTimetracker, parseErr = parseBool16961(parts[0]) + if parseErr != nil { + return + } + cfg.AllowOnlyContributorsToTrackTime, parseErr = parseBool16961(parts[1]) + if parseErr != nil { + return + } + cfg.EnableDependencies, parseErr = parseBool16961(parts[2]) + if parseErr != nil { + return + } + return true, nil +} + +func fixBrokenRepoUnit16961(repoUnit *models.RepoUnit, bs []byte) (fixed bool, err error) { + // Shortcut empty or null values + if len(bs) == 0 { + return false, nil + } + + switch models.UnitType(repoUnit.Type) { + case models.UnitTypeCode, models.UnitTypeReleases, models.UnitTypeWiki, models.UnitTypeProjects: + cfg := &models.UnitConfig{} + repoUnit.Config = cfg + if fixed, err := fixUnitConfig16961(bs, cfg); !fixed { + return false, err + } + case models.UnitTypeExternalWiki: + cfg := &models.ExternalWikiConfig{} + repoUnit.Config = cfg + + if fixed, err := fixExternalWikiConfig16961(bs, cfg); !fixed { + return false, err + } + case models.UnitTypeExternalTracker: + cfg := &models.ExternalTrackerConfig{} + repoUnit.Config = cfg + if fixed, err := fixExternalTrackerConfig16961(bs, cfg); !fixed { + return false, err + } + case models.UnitTypePullRequests: + cfg := &models.PullRequestsConfig{} + repoUnit.Config = cfg + + if fixed, err := fixPullRequestsConfig16961(bs, cfg); !fixed { + return false, err + } + case models.UnitTypeIssues: + cfg := &models.IssuesConfig{} + repoUnit.Config = cfg + if fixed, err := fixIssuesConfig16961(bs, cfg); !fixed { + return false, err + } + default: + panic(fmt.Sprintf("unrecognized repo unit type: %v", repoUnit.Type)) + } + return true, nil +} + +func fixBrokenRepoUnits16961(logger log.Logger, autofix bool) error { + // RepoUnit describes all units of a repository + type RepoUnit struct { + ID int64 + RepoID int64 + Type models.UnitType + Config []byte + CreatedUnix timeutil.TimeStamp `xorm:"INDEX CREATED"` + } + + count := 0 + + err := models.Iterate( + models.DefaultDBContext(), + new(RepoUnit), + builder.Gt{ + "id": 0, + }, + func(idx int, bean interface{}) error { + unit := bean.(*RepoUnit) + + bs := unit.Config + repoUnit := &models.RepoUnit{ + ID: unit.ID, + RepoID: unit.RepoID, + Type: unit.Type, + CreatedUnix: unit.CreatedUnix, + } + + if fixed, err := fixBrokenRepoUnit16961(repoUnit, bs); !fixed { + return err + } + + count++ + if !autofix { + return nil + } + + return models.UpdateRepoUnit(repoUnit) + }, + ) + + if err != nil { + logger.Critical("Unable to iterate acrosss repounits to fix the broken units: Error %v", err) + return err + } + + if !autofix { + logger.Warn("Found %d broken repo_units", count) + return nil + } + logger.Info("Fixed %d broken repo_units", count) + + return nil +} + +func init() { + Register(&Check{ + Title: "Check for incorrectly dumped repo_units (See #16961)", + Name: "fix-broken-repo-units", + IsDefault: false, + Run: fixBrokenRepoUnits16961, + Priority: 7, + }) +} diff --git a/modules/doctor/fix16961_test.go b/modules/doctor/fix16961_test.go new file mode 100644 index 0000000000..017f585335 --- /dev/null +++ b/modules/doctor/fix16961_test.go @@ -0,0 +1,271 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package doctor + +import ( + "testing" + + "code.gitea.io/gitea/models" + "github.com/stretchr/testify/assert" +) + +func Test_fixUnitConfig_16961(t *testing.T) { + tests := []struct { + name string + bs string + wantFixed bool + wantErr bool + }{ + { + name: "empty", + bs: "", + wantFixed: true, + wantErr: false, + }, + { + name: "normal: {}", + bs: "{}", + wantFixed: false, + wantErr: false, + }, + { + name: "broken but fixable: &{}", + bs: "&{}", + wantFixed: true, + wantErr: false, + }, + { + name: "broken but unfixable: &{asdasd}", + bs: "&{asdasd}", + wantFixed: false, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotFixed, err := fixUnitConfig16961([]byte(tt.bs), &models.UnitConfig{}) + if (err != nil) != tt.wantErr { + t.Errorf("fixUnitConfig_16961() error = %v, wantErr %v", err, tt.wantErr) + return + } + if gotFixed != tt.wantFixed { + t.Errorf("fixUnitConfig_16961() = %v, want %v", gotFixed, tt.wantFixed) + } + }) + } +} + +func Test_fixExternalWikiConfig_16961(t *testing.T) { + tests := []struct { + name string + bs string + expected string + wantFixed bool + wantErr bool + }{ + { + name: "normal: {\"ExternalWikiURL\":\"http://someurl\"}", + bs: "{\"ExternalWikiURL\":\"http://someurl\"}", + expected: "http://someurl", + wantFixed: false, + wantErr: false, + }, + { + name: "broken: &{http://someurl}", + bs: "&{http://someurl}", + expected: "http://someurl", + wantFixed: true, + wantErr: false, + }, + { + name: "broken but unfixable: http://someurl", + bs: "http://someurl", + wantFixed: false, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := &models.ExternalWikiConfig{} + gotFixed, err := fixExternalWikiConfig16961([]byte(tt.bs), cfg) + if (err != nil) != tt.wantErr { + t.Errorf("fixExternalWikiConfig_16961() error = %v, wantErr %v", err, tt.wantErr) + return + } + if gotFixed != tt.wantFixed { + t.Errorf("fixExternalWikiConfig_16961() = %v, want %v", gotFixed, tt.wantFixed) + } + if cfg.ExternalWikiURL != tt.expected { + t.Errorf("fixExternalWikiConfig_16961().ExternalWikiURL = %v, want %v", cfg.ExternalWikiURL, tt.expected) + } + }) + } +} + +func Test_fixExternalTrackerConfig_16961(t *testing.T) { + tests := []struct { + name string + bs string + expected models.ExternalTrackerConfig + wantFixed bool + wantErr bool + }{ + { + name: "normal", + bs: `{"ExternalTrackerURL":"a","ExternalTrackerFormat":"b","ExternalTrackerStyle":"c"}`, + expected: models.ExternalTrackerConfig{ + ExternalTrackerURL: "a", + ExternalTrackerFormat: "b", + ExternalTrackerStyle: "c", + }, + wantFixed: false, + wantErr: false, + }, + { + name: "broken", + bs: "&{a b c}", + expected: models.ExternalTrackerConfig{ + ExternalTrackerURL: "a", + ExternalTrackerFormat: "b", + ExternalTrackerStyle: "c", + }, + wantFixed: true, + wantErr: false, + }, + { + name: "broken - too many fields", + bs: "&{a b c d}", + wantFixed: false, + wantErr: true, + }, + { + name: "broken - wrong format", + bs: "a b c d}", + wantFixed: false, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := &models.ExternalTrackerConfig{} + gotFixed, err := fixExternalTrackerConfig16961([]byte(tt.bs), cfg) + if (err != nil) != tt.wantErr { + t.Errorf("fixExternalTrackerConfig_16961() error = %v, wantErr %v", err, tt.wantErr) + return + } + if gotFixed != tt.wantFixed { + t.Errorf("fixExternalTrackerConfig_16961() = %v, want %v", gotFixed, tt.wantFixed) + } + if cfg.ExternalTrackerFormat != tt.expected.ExternalTrackerFormat { + t.Errorf("fixExternalTrackerConfig_16961().ExternalTrackerFormat = %v, want %v", tt.expected.ExternalTrackerFormat, cfg.ExternalTrackerFormat) + } + if cfg.ExternalTrackerStyle != tt.expected.ExternalTrackerStyle { + t.Errorf("fixExternalTrackerConfig_16961().ExternalTrackerStyle = %v, want %v", tt.expected.ExternalTrackerStyle, cfg.ExternalTrackerStyle) + } + if cfg.ExternalTrackerURL != tt.expected.ExternalTrackerURL { + t.Errorf("fixExternalTrackerConfig_16961().ExternalTrackerURL = %v, want %v", tt.expected.ExternalTrackerURL, cfg.ExternalTrackerURL) + } + }) + } +} + +func Test_fixPullRequestsConfig_16961(t *testing.T) { + tests := []struct { + name string + bs string + expected models.PullRequestsConfig + wantFixed bool + wantErr bool + }{ + { + name: "normal", + bs: `{"IgnoreWhitespaceConflicts":false,"AllowMerge":false,"AllowRebase":false,"AllowRebaseMerge":false,"AllowSquash":false,"AllowManualMerge":false,"AutodetectManualMerge":false,"DefaultDeleteBranchAfterMerge":false,"DefaultMergeStyle":""}`, + }, + { + name: "broken - 1.14", + bs: `&{%!s(bool=false) %!s(bool=true) %!s(bool=true) %!s(bool=true) %!s(bool=true) %!s(bool=false) %!s(bool=false)}`, + expected: models.PullRequestsConfig{ + IgnoreWhitespaceConflicts: false, + AllowMerge: true, + AllowRebase: true, + AllowRebaseMerge: true, + AllowSquash: true, + AllowManualMerge: false, + AutodetectManualMerge: false, + }, + wantFixed: true, + }, + { + name: "broken - 1.15", + bs: `&{%!s(bool=false) %!s(bool=true) %!s(bool=true) %!s(bool=true) %!s(bool=true) %!s(bool=false) %!s(bool=false) %!s(bool=false) merge}`, + expected: models.PullRequestsConfig{ + AllowMerge: true, + AllowRebase: true, + AllowRebaseMerge: true, + AllowSquash: true, + DefaultMergeStyle: models.MergeStyleMerge, + }, + wantFixed: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := &models.PullRequestsConfig{} + gotFixed, err := fixPullRequestsConfig16961([]byte(tt.bs), cfg) + if (err != nil) != tt.wantErr { + t.Errorf("fixPullRequestsConfig_16961() error = %v, wantErr %v", err, tt.wantErr) + return + } + if gotFixed != tt.wantFixed { + t.Errorf("fixPullRequestsConfig_16961() = %v, want %v", gotFixed, tt.wantFixed) + } + assert.EqualValues(t, &tt.expected, cfg) + }) + } +} + +func Test_fixIssuesConfig_16961(t *testing.T) { + tests := []struct { + name string + bs string + expected models.IssuesConfig + wantFixed bool + wantErr bool + }{ + { + name: "normal", + bs: `{"EnableTimetracker":true,"AllowOnlyContributorsToTrackTime":true,"EnableDependencies":true}`, + expected: models.IssuesConfig{ + EnableTimetracker: true, + AllowOnlyContributorsToTrackTime: true, + EnableDependencies: true, + }, + }, + { + name: "broken", + bs: `&{%!s(bool=true) %!s(bool=true) %!s(bool=true)}`, + expected: models.IssuesConfig{ + EnableTimetracker: true, + AllowOnlyContributorsToTrackTime: true, + EnableDependencies: true, + }, + wantFixed: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := &models.IssuesConfig{} + gotFixed, err := fixIssuesConfig16961([]byte(tt.bs), cfg) + if (err != nil) != tt.wantErr { + t.Errorf("fixIssuesConfig_16961() error = %v, wantErr %v", err, tt.wantErr) + return + } + if gotFixed != tt.wantFixed { + t.Errorf("fixIssuesConfig_16961() = %v, want %v", gotFixed, tt.wantFixed) + } + assert.EqualValues(t, &tt.expected, cfg) + }) + } +}