diff options
| -rw-r--r-- | models/helper.go | 2 | ||||
| -rw-r--r-- | models/repo_unit.go | 6 | ||||
| -rw-r--r-- | modules/doctor/fix16961.go | 318 | ||||
| -rw-r--r-- | modules/doctor/fix16961_test.go | 271 |
4 files changed, 596 insertions, 1 deletions
diff --git a/models/helper.go b/models/helper.go index c499b5512d..710c15a978 100644 --- a/models/helper.go +++ b/models/helper.go @@ -51,7 +51,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) diff --git a/models/repo_unit.go b/models/repo_unit.go index 7061119bd8..474f65bf03 100644 --- a/models/repo_unit.go +++ b/models/repo_unit.go @@ -220,3 +220,9 @@ func getUnitsByRepoID(e db.Engine, repoID int64) (units []*RepoUnit, err error) return units, nil } + +// UpdateRepoUnit updates the provided repo unit +func UpdateRepoUnit(unit *RepoUnit) error { + _, err := db.GetEngine(db.DefaultContext).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..60cc5ffe2f --- /dev/null +++ b/modules/doctor/fix16961.go @@ -0,0 +1,318 @@ +// 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/models/db" + "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 := db.Iterate( + db.DefaultContext, + 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) + }) + } +} |