mirror of
https://github.com/go-gitea/gitea
synced 2024-12-23 00:54:28 +00:00
Avoid polluting config file when "save" (#25395)
That's a longstanding INI package problem: the "MustXxx" calls change the option values, and the following "Save" will save a lot of garbage options into the user's config file. Ideally we should refactor the INI package to a clear solution, but it's a huge work. A clear workaround is what this PR does: when "Save", load a clear INI instance and save it. Partially fix #25377, the "install" page needs more fine tunes.
This commit is contained in:
parent
831db53c21
commit
df5cf5ddbd
12
cmd/web.go
12
cmd/web.go
@ -217,9 +217,15 @@ func setPort(port string) error {
|
|||||||
defaultLocalURL += ":" + setting.HTTPPort + "/"
|
defaultLocalURL += ":" + setting.HTTPPort + "/"
|
||||||
|
|
||||||
// Save LOCAL_ROOT_URL if port changed
|
// Save LOCAL_ROOT_URL if port changed
|
||||||
setting.CfgProvider.Section("server").Key("LOCAL_ROOT_URL").SetValue(defaultLocalURL)
|
rootCfg := setting.CfgProvider
|
||||||
if err := setting.CfgProvider.Save(); err != nil {
|
saveCfg, err := rootCfg.PrepareSaving()
|
||||||
return fmt.Errorf("Failed to save config file: %v", err)
|
if err != nil {
|
||||||
|
return fmt.Errorf("failed to save config file: %v", err)
|
||||||
|
}
|
||||||
|
rootCfg.Section("server").Key("LOCAL_ROOT_URL").SetValue(defaultLocalURL)
|
||||||
|
saveCfg.Section("server").Key("LOCAL_ROOT_URL").SetValue(defaultLocalURL)
|
||||||
|
if err = saveCfg.Save(); err != nil {
|
||||||
|
return fmt.Errorf("failed to save config file: %v", err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return nil
|
return nil
|
||||||
|
@ -4,6 +4,7 @@
|
|||||||
package setting
|
package setting
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
@ -51,11 +52,17 @@ type ConfigProvider interface {
|
|||||||
GetSection(name string) (ConfigSection, error)
|
GetSection(name string) (ConfigSection, error)
|
||||||
Save() error
|
Save() error
|
||||||
SaveTo(filename string) error
|
SaveTo(filename string) error
|
||||||
|
|
||||||
|
DisableSaving()
|
||||||
|
PrepareSaving() (ConfigProvider, error)
|
||||||
}
|
}
|
||||||
|
|
||||||
type iniConfigProvider struct {
|
type iniConfigProvider struct {
|
||||||
opts *Options
|
opts *Options
|
||||||
ini *ini.File
|
ini *ini.File
|
||||||
|
|
||||||
|
disableSaving bool
|
||||||
|
|
||||||
newFile bool // whether the file has not existed previously
|
newFile bool // whether the file has not existed previously
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -191,7 +198,7 @@ type Options struct {
|
|||||||
// NewConfigProviderFromFile load configuration from file.
|
// NewConfigProviderFromFile load configuration from file.
|
||||||
// NOTE: do not print any log except error.
|
// NOTE: do not print any log except error.
|
||||||
func NewConfigProviderFromFile(opts *Options) (ConfigProvider, error) {
|
func NewConfigProviderFromFile(opts *Options) (ConfigProvider, error) {
|
||||||
cfg := ini.Empty()
|
cfg := ini.Empty(ini.LoadOptions{KeyValueDelimiterOnWrite: " = "})
|
||||||
newFile := true
|
newFile := true
|
||||||
|
|
||||||
if opts.CustomConf != "" {
|
if opts.CustomConf != "" {
|
||||||
@ -252,8 +259,13 @@ func (p *iniConfigProvider) GetSection(name string) (ConfigSection, error) {
|
|||||||
return &iniConfigSection{sec: sec}, nil
|
return &iniConfigSection{sec: sec}, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
var errDisableSaving = errors.New("this config can't be saved, developers should prepare a new config to save")
|
||||||
|
|
||||||
// Save saves the content into file
|
// Save saves the content into file
|
||||||
func (p *iniConfigProvider) Save() error {
|
func (p *iniConfigProvider) Save() error {
|
||||||
|
if p.disableSaving {
|
||||||
|
return errDisableSaving
|
||||||
|
}
|
||||||
filename := p.opts.CustomConf
|
filename := p.opts.CustomConf
|
||||||
if filename == "" {
|
if filename == "" {
|
||||||
if !p.opts.AllowEmpty {
|
if !p.opts.AllowEmpty {
|
||||||
@ -285,9 +297,29 @@ func (p *iniConfigProvider) Save() error {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func (p *iniConfigProvider) SaveTo(filename string) error {
|
func (p *iniConfigProvider) SaveTo(filename string) error {
|
||||||
|
if p.disableSaving {
|
||||||
|
return errDisableSaving
|
||||||
|
}
|
||||||
return p.ini.SaveTo(filename)
|
return p.ini.SaveTo(filename)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// DisableSaving disables the saving function, use PrepareSaving to get clear config options.
|
||||||
|
func (p *iniConfigProvider) DisableSaving() {
|
||||||
|
p.disableSaving = true
|
||||||
|
}
|
||||||
|
|
||||||
|
// PrepareSaving loads the ini from file again to get clear config options.
|
||||||
|
// Otherwise, the "MustXxx" calls would have polluted the current config provider,
|
||||||
|
// it makes the "Save" outputs a lot of garbage options
|
||||||
|
// After the INI package gets refactored, no "MustXxx" pollution, this workaround can be dropped.
|
||||||
|
func (p *iniConfigProvider) PrepareSaving() (ConfigProvider, error) {
|
||||||
|
cfgFile := p.opts.CustomConf
|
||||||
|
if cfgFile == "" {
|
||||||
|
return nil, errors.New("no config file to save")
|
||||||
|
}
|
||||||
|
return NewConfigProviderFromFile(p.opts)
|
||||||
|
}
|
||||||
|
|
||||||
func mustMapSetting(rootCfg ConfigProvider, sectionName string, setting any) {
|
func mustMapSetting(rootCfg ConfigProvider, sectionName string, setting any) {
|
||||||
if err := rootCfg.Section(sectionName).MapTo(setting); err != nil {
|
if err := rootCfg.Section(sectionName).MapTo(setting); err != nil {
|
||||||
log.Fatal("Failed to map %s settings: %v", sectionName, err)
|
log.Fatal("Failed to map %s settings: %v", sectionName, err)
|
||||||
|
@ -84,11 +84,11 @@ func TestNewConfigProviderFromFile(t *testing.T) {
|
|||||||
|
|
||||||
bs, err := os.ReadFile(testFile)
|
bs, err := os.ReadFile(testFile)
|
||||||
assert.NoError(t, err)
|
assert.NoError(t, err)
|
||||||
assert.Equal(t, "[foo]\nk1=a\n", string(bs))
|
assert.Equal(t, "[foo]\nk1 = a\n", string(bs))
|
||||||
|
|
||||||
bs, err = os.ReadFile(testFile1)
|
bs, err = os.ReadFile(testFile1)
|
||||||
assert.NoError(t, err)
|
assert.NoError(t, err)
|
||||||
assert.Equal(t, "[foo]\nk1=a\nk2=b\n", string(bs))
|
assert.Equal(t, "[foo]\nk1 = a\nk2 = b\n", string(bs))
|
||||||
|
|
||||||
// load existing file and save
|
// load existing file and save
|
||||||
cfg, err = NewConfigProviderFromFile(&Options{CustomConf: testFile, AllowEmpty: true})
|
cfg, err = NewConfigProviderFromFile(&Options{CustomConf: testFile, AllowEmpty: true})
|
||||||
@ -99,7 +99,7 @@ func TestNewConfigProviderFromFile(t *testing.T) {
|
|||||||
assert.NoError(t, cfg.Save())
|
assert.NoError(t, cfg.Save())
|
||||||
bs, err = os.ReadFile(testFile)
|
bs, err = os.ReadFile(testFile)
|
||||||
assert.NoError(t, err)
|
assert.NoError(t, err)
|
||||||
assert.Equal(t, "[foo]\nk1=a\n\n[bar]\nk1=b\n", string(bs))
|
assert.Equal(t, "[foo]\nk1 = a\n\n[bar]\nk1 = b\n", string(bs))
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestNewConfigProviderForLocale(t *testing.T) {
|
func TestNewConfigProviderForLocale(t *testing.T) {
|
||||||
@ -119,3 +119,27 @@ func TestNewConfigProviderForLocale(t *testing.T) {
|
|||||||
assert.Equal(t, "foo", cfg.Section("").Key("k1").String())
|
assert.Equal(t, "foo", cfg.Section("").Key("k1").String())
|
||||||
assert.Equal(t, "xxx", cfg.Section("").Key("k2").String())
|
assert.Equal(t, "xxx", cfg.Section("").Key("k2").String())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestDisableSaving(t *testing.T) {
|
||||||
|
testFile := t.TempDir() + "/test.ini"
|
||||||
|
_ = os.WriteFile(testFile, []byte("k1=a\nk2=b"), 0o644)
|
||||||
|
cfg, err := NewConfigProviderFromFile(&Options{CustomConf: testFile, AllowEmpty: true})
|
||||||
|
assert.NoError(t, err)
|
||||||
|
|
||||||
|
cfg.DisableSaving()
|
||||||
|
err = cfg.Save()
|
||||||
|
assert.ErrorIs(t, err, errDisableSaving)
|
||||||
|
|
||||||
|
saveCfg, err := cfg.PrepareSaving()
|
||||||
|
assert.NoError(t, err)
|
||||||
|
|
||||||
|
saveCfg.Section("").Key("k1").MustString("x")
|
||||||
|
saveCfg.Section("").Key("k2").SetValue("y")
|
||||||
|
saveCfg.Section("").Key("k3").SetValue("z")
|
||||||
|
err = saveCfg.Save()
|
||||||
|
assert.NoError(t, err)
|
||||||
|
|
||||||
|
bs, err := os.ReadFile(testFile)
|
||||||
|
assert.NoError(t, err)
|
||||||
|
assert.Equal(t, "k1 = a\nk2 = y\nk3 = z\n", string(bs))
|
||||||
|
}
|
||||||
|
@ -59,13 +59,18 @@ func loadLFSFrom(rootCfg ConfigProvider) error {
|
|||||||
if err != nil || n != 32 {
|
if err != nil || n != 32 {
|
||||||
LFS.JWTSecretBase64, err = generate.NewJwtSecretBase64()
|
LFS.JWTSecretBase64, err = generate.NewJwtSecretBase64()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return fmt.Errorf("Error generating JWT Secret for custom config: %v", err)
|
return fmt.Errorf("error generating JWT Secret for custom config: %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Save secret
|
// Save secret
|
||||||
sec.Key("LFS_JWT_SECRET").SetValue(LFS.JWTSecretBase64)
|
saveCfg, err := rootCfg.PrepareSaving()
|
||||||
if err := rootCfg.Save(); err != nil {
|
if err != nil {
|
||||||
return fmt.Errorf("Error saving JWT Secret for custom config: %v", err)
|
return fmt.Errorf("error saving JWT Secret for custom config: %v", err)
|
||||||
|
}
|
||||||
|
rootCfg.Section("server").Key("LFS_JWT_SECRET").SetValue(LFS.JWTSecretBase64)
|
||||||
|
saveCfg.Section("server").Key("LFS_JWT_SECRET").SetValue(LFS.JWTSecretBase64)
|
||||||
|
if err := saveCfg.Save(); err != nil {
|
||||||
|
return fmt.Errorf("error saving JWT Secret for custom config: %v", err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -130,8 +130,13 @@ func loadOAuth2From(rootCfg ConfigProvider) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
secretBase64 := base64.RawURLEncoding.EncodeToString(key)
|
secretBase64 := base64.RawURLEncoding.EncodeToString(key)
|
||||||
|
saveCfg, err := rootCfg.PrepareSaving()
|
||||||
|
if err != nil {
|
||||||
|
log.Fatal("save oauth2.JWT_SECRET failed: %v", err)
|
||||||
|
}
|
||||||
rootCfg.Section("oauth2").Key("JWT_SECRET").SetValue(secretBase64)
|
rootCfg.Section("oauth2").Key("JWT_SECRET").SetValue(secretBase64)
|
||||||
if err := rootCfg.Save(); err != nil {
|
saveCfg.Section("oauth2").Key("JWT_SECRET").SetValue(secretBase64)
|
||||||
|
if err := saveCfg.Save(); err != nil {
|
||||||
log.Fatal("save oauth2.JWT_SECRET failed: %v", err)
|
log.Fatal("save oauth2.JWT_SECRET failed: %v", err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -89,8 +89,13 @@ func generateSaveInternalToken(rootCfg ConfigProvider) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
InternalToken = token
|
InternalToken = token
|
||||||
|
saveCfg, err := rootCfg.PrepareSaving()
|
||||||
|
if err != nil {
|
||||||
|
log.Fatal("Error saving internal token: %v", err)
|
||||||
|
}
|
||||||
rootCfg.Section("security").Key("INTERNAL_TOKEN").SetValue(token)
|
rootCfg.Section("security").Key("INTERNAL_TOKEN").SetValue(token)
|
||||||
if err := rootCfg.Save(); err != nil {
|
saveCfg.Section("security").Key("INTERNAL_TOKEN").SetValue(token)
|
||||||
|
if err = saveCfg.Save(); err != nil {
|
||||||
log.Fatal("Error saving internal token: %v", err)
|
log.Fatal("Error saving internal token: %v", err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -202,6 +202,7 @@ func Init(opts *Options) {
|
|||||||
}
|
}
|
||||||
var err error
|
var err error
|
||||||
CfgProvider, err = NewConfigProviderFromFile(opts)
|
CfgProvider, err = NewConfigProviderFromFile(opts)
|
||||||
|
CfgProvider.DisableSaving() // do not allow saving the CfgProvider into file, it will be polluted by the "MustXxx" calls
|
||||||
if err != nil {
|
if err != nil {
|
||||||
log.Fatal("newConfigProviderFromFile[%v]: %v", opts, err)
|
log.Fatal("newConfigProviderFromFile[%v]: %v", opts, err)
|
||||||
}
|
}
|
||||||
@ -214,7 +215,7 @@ func Init(opts *Options) {
|
|||||||
|
|
||||||
// loadCommonSettingsFrom loads common configurations from a configuration provider.
|
// loadCommonSettingsFrom loads common configurations from a configuration provider.
|
||||||
func loadCommonSettingsFrom(cfg ConfigProvider) error {
|
func loadCommonSettingsFrom(cfg ConfigProvider) error {
|
||||||
// WARNNING: don't change the sequence except you know what you are doing.
|
// WARNING: don't change the sequence except you know what you are doing.
|
||||||
loadRunModeFrom(cfg)
|
loadRunModeFrom(cfg)
|
||||||
loadLogGlobalFrom(cfg)
|
loadLogGlobalFrom(cfg)
|
||||||
loadServerFrom(cfg)
|
loadServerFrom(cfg)
|
||||||
|
Loading…
Reference in New Issue
Block a user