From d476c5d5b0992dc210cbbc35b371689ca09b7826 Mon Sep 17 00:00:00 2001 From: Antonio Navarro Perez Date: Wed, 30 Nov 2022 15:43:54 +0100 Subject: [PATCH] Avoid unknown fields on config This will reduce headaches when adding the wrong fields to the config. Signed-off-by: Antonio Navarro Perez --- cmd/ipfs/daemon.go | 3 +- config/config.go | 103 ++++++++++++++++++++++++----- config/config_test.go | 63 ++++++++++++++++++ config/serialize/serialize.go | 72 -------------------- config/serialize/serialize_test.go | 37 ----------- docs/environment-variables.md | 7 ++ plugin/loader/loader.go | 5 +- repo/fsrepo/fsrepo.go | 15 ++--- test/sharness/t0021-config.sh | 7 +- 9 files changed, 172 insertions(+), 140 deletions(-) delete mode 100644 config/serialize/serialize.go delete mode 100644 config/serialize/serialize_test.go diff --git a/cmd/ipfs/daemon.go b/cmd/ipfs/daemon.go index 1bfb9d6f1137..5f27f2953678 100644 --- a/cmd/ipfs/daemon.go +++ b/cmd/ipfs/daemon.go @@ -19,7 +19,6 @@ import ( utilmain "github.com/ipfs/kubo/cmd/ipfs/util" oldcmds "github.com/ipfs/kubo/commands" config "github.com/ipfs/kubo/config" - cserial "github.com/ipfs/kubo/config/serialize" "github.com/ipfs/kubo/core" commands "github.com/ipfs/kubo/core/commands" "github.com/ipfs/kubo/core/coreapi" @@ -252,7 +251,7 @@ func daemonFunc(req *cmds.Request, re cmds.ResponseEmitter, env cmds.Environment var conf *config.Config if cfgLocation != "" { - if conf, err = cserial.Load(cfgLocation); err != nil { + if conf, err = config.Load(cfgLocation); err != nil { return err } } diff --git a/config/config.go b/config/config.go index 93494265d052..869d812c297c 100644 --- a/config/config.go +++ b/config/config.go @@ -4,14 +4,21 @@ package config import ( "bytes" "encoding/json" + "errors" "fmt" + "io" "os" "path/filepath" "strings" + "github.com/facebookgo/atomicfile" "github.com/mitchellh/go-homedir" ) +// ErrNotInitialized is returned when we fail to read the config because the +// repo doesn't exist. +var ErrNotInitialized = errors.New("ipfs not initialized, please run 'ipfs init'") + // Config is used to load ipfs config files. type Config struct { Identity Identity // local node's peer identity @@ -101,35 +108,36 @@ func HumanOutput(value interface{}) ([]byte, error) { if ok { return []byte(strings.Trim(s, "\n")), nil } - return Marshal(value) -} -// Marshal configuration with JSON -func Marshal(value interface{}) ([]byte, error) { - // need to prettyprint, hence MarshalIndent, instead of Encoder - return json.MarshalIndent(value, "", " ") + buf := new(bytes.Buffer) + if err := encodeConfigFile(buf, value); err != nil { + return nil, err + } + + return buf.Bytes(), nil } func FromMap(v map[string]interface{}) (*Config, error) { buf := new(bytes.Buffer) - if err := json.NewEncoder(buf).Encode(v); err != nil { + + if err := encodeConfigFile(buf, v); err != nil { return nil, err } var conf Config - if err := json.NewDecoder(buf).Decode(&conf); err != nil { - return nil, fmt.Errorf("failure to decode config: %s", err) + if err := decodeConfigFile(buf, &conf); err != nil { + return nil, err } return &conf, nil } func ToMap(conf *Config) (map[string]interface{}, error) { buf := new(bytes.Buffer) - if err := json.NewEncoder(buf).Encode(conf); err != nil { + if err := encodeConfigFile(buf, conf); err != nil { return nil, err } var m map[string]interface{} - if err := json.NewDecoder(buf).Decode(&m); err != nil { - return nil, fmt.Errorf("failure to decode config: %s", err) + if err := decodeConfigFile(buf, &m); err != nil { + return nil, err } return m, nil } @@ -139,13 +147,74 @@ func (c *Config) Clone() (*Config, error) { var newConfig Config var buf bytes.Buffer - if err := json.NewEncoder(&buf).Encode(c); err != nil { - return nil, fmt.Errorf("failure to encode config: %s", err) + if err := encodeConfigFile(&buf, c); err != nil { + return nil, err } - - if err := json.NewDecoder(&buf).Decode(&newConfig); err != nil { - return nil, fmt.Errorf("failure to decode config: %s", err) + if err := decodeConfigFile(&buf, &newConfig); err != nil { + return nil, err } return &newConfig, nil } + +// ReadConfigFile reads the config from `filename` into `cfg`. +func ReadConfigFile(filename string, cfg interface{}) error { + f, err := os.Open(filename) + if err != nil { + if os.IsNotExist(err) { + err = ErrNotInitialized + } + return err + } + defer f.Close() + + return decodeConfigFile(f, cfg) +} + +func decodeConfigFile(r io.Reader, cfg interface{}) error { + dec := json.NewDecoder(r) + if os.Getenv("IPFS_CONFIG_TOLERANT_MODE") == "" { + dec.DisallowUnknownFields() + } + + if err := dec.Decode(cfg); err != nil { + return fmt.Errorf("failure to decode config: %s", err) + } + + return nil +} + +// WriteConfigFile writes the config from `cfg` into `filename`. +func WriteConfigFile(filename string, cfg interface{}) error { + err := os.MkdirAll(filepath.Dir(filename), 0755) + if err != nil { + return err + } + + f, err := atomicfile.New(filename, 0600) + if err != nil { + return err + } + defer f.Close() + + return encodeConfigFile(f, cfg) +} + +// encodeConfigFile encodes configuration with JSON +func encodeConfigFile(w io.Writer, value interface{}) error { + enc := json.NewEncoder(w) + enc.SetIndent("", " ") + + return enc.Encode(value) +} + +// Load reads given file and returns the read config, or error. +func Load(filename string) (*Config, error) { + var cfg Config + err := ReadConfigFile(filename, &cfg) + if err != nil { + return nil, err + } + + return &cfg, err +} diff --git a/config/config_test.go b/config/config_test.go index dead06f8a236..167978246292 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -1,6 +1,8 @@ package config import ( + "os" + "runtime" "testing" ) @@ -27,3 +29,64 @@ func TestClone(t *testing.T) { t.Fatal("HTTP headers not preserved") } } + +func TestConfig(t *testing.T) { + const filename = ".ipfsconfig" + cfgWritten := new(Config) + cfgWritten.Identity.PeerID = "faketest" + + err := WriteConfigFile(filename, cfgWritten) + if err != nil { + t.Fatal(err) + } + cfgRead, err := Load(filename) + if err != nil { + t.Fatal(err) + } + if cfgWritten.Identity.PeerID != cfgRead.Identity.PeerID { + t.Fatal() + } + st, err := os.Stat(filename) + if err != nil { + t.Fatalf("cannot stat config file: %v", err) + } + + if runtime.GOOS != "windows" { // see https://golang.org/src/os/types_windows.go + if g := st.Mode().Perm(); g&0117 != 0 { + t.Fatalf("config file should not be executable or accessible to world: %v", g) + } + } +} + +func TestConfigUnknownField(t *testing.T) { + const filename = ".ipfsconfig" + + badConfig := map[string]string{ + "BadField": "Value", + } + + err := WriteConfigFile(filename, badConfig) + if err != nil { + t.Fatal(err) + } + + _, err = Load(filename) + if err == nil { + t.Fatal("load must fail") + } + + if err.Error() != "failure to decode config: json: unknown field \"BadField\"" { + t.Fatal("unexpected error:", err) + } + + mapOut := make(map[string]string) + + err = ReadConfigFile(filename, &mapOut) + if err != nil { + t.Fatal(err) + } + + if mapOut["BadField"] != "Value" { + t.Fatal(err) + } +} diff --git a/config/serialize/serialize.go b/config/serialize/serialize.go deleted file mode 100644 index d20e48118f5f..000000000000 --- a/config/serialize/serialize.go +++ /dev/null @@ -1,72 +0,0 @@ -package fsrepo - -import ( - "encoding/json" - "errors" - "fmt" - "io" - "os" - "path/filepath" - - "github.com/ipfs/kubo/config" - - "github.com/facebookgo/atomicfile" -) - -// ErrNotInitialized is returned when we fail to read the config because the -// repo doesn't exist. -var ErrNotInitialized = errors.New("ipfs not initialized, please run 'ipfs init'") - -// ReadConfigFile reads the config from `filename` into `cfg`. -func ReadConfigFile(filename string, cfg interface{}) error { - f, err := os.Open(filename) - if err != nil { - if os.IsNotExist(err) { - err = ErrNotInitialized - } - return err - } - defer f.Close() - if err := json.NewDecoder(f).Decode(cfg); err != nil { - return fmt.Errorf("failure to decode config: %s", err) - } - return nil -} - -// WriteConfigFile writes the config from `cfg` into `filename`. -func WriteConfigFile(filename string, cfg interface{}) error { - err := os.MkdirAll(filepath.Dir(filename), 0755) - if err != nil { - return err - } - - f, err := atomicfile.New(filename, 0600) - if err != nil { - return err - } - defer f.Close() - - return encode(f, cfg) -} - -// encode configuration with JSON -func encode(w io.Writer, value interface{}) error { - // need to prettyprint, hence MarshalIndent, instead of Encoder - buf, err := config.Marshal(value) - if err != nil { - return err - } - _, err = w.Write(buf) - return err -} - -// Load reads given file and returns the read config, or error. -func Load(filename string) (*config.Config, error) { - var cfg config.Config - err := ReadConfigFile(filename, &cfg) - if err != nil { - return nil, err - } - - return &cfg, err -} diff --git a/config/serialize/serialize_test.go b/config/serialize/serialize_test.go deleted file mode 100644 index cc161c80dbd3..000000000000 --- a/config/serialize/serialize_test.go +++ /dev/null @@ -1,37 +0,0 @@ -package fsrepo - -import ( - "os" - "runtime" - "testing" - - config "github.com/ipfs/kubo/config" -) - -func TestConfig(t *testing.T) { - const filename = ".ipfsconfig" - cfgWritten := new(config.Config) - cfgWritten.Identity.PeerID = "faketest" - - err := WriteConfigFile(filename, cfgWritten) - if err != nil { - t.Fatal(err) - } - cfgRead, err := Load(filename) - if err != nil { - t.Fatal(err) - } - if cfgWritten.Identity.PeerID != cfgRead.Identity.PeerID { - t.Fatal() - } - st, err := os.Stat(filename) - if err != nil { - t.Fatalf("cannot stat config file: %v", err) - } - - if runtime.GOOS != "windows" { // see https://golang.org/src/os/types_windows.go - if g := st.Mode().Perm(); g&0117 != 0 { - t.Fatalf("config file should not be executable or accessible to world: %v", g) - } - } -} diff --git a/docs/environment-variables.md b/docs/environment-variables.md index 98b449054493..38ce217e79f3 100644 --- a/docs/environment-variables.md +++ b/docs/environment-variables.md @@ -114,6 +114,13 @@ $ ipfs resolve -r /ipns/dnslink-test2.example.com /ipfs/bafkreicysg23kiwv34eg2d7qweipxwosdo2py4ldv42nbauguluen5v6am ``` +## `IPFS_CONFIG_TOLERANT_MODE` + +Disables strict config valiadtion to allow unsupported fields on JSON config. + +Default: false + + ## `LIBP2P_TCP_REUSEPORT` Kubo tries to reuse the same source port for all connections to improve NAT diff --git a/plugin/loader/loader.go b/plugin/loader/loader.go index 0742e6a491ce..2411a2d214dd 100644 --- a/plugin/loader/loader.go +++ b/plugin/loader/loader.go @@ -9,7 +9,6 @@ import ( "strings" config "github.com/ipfs/kubo/config" - cserialize "github.com/ipfs/kubo/config/serialize" "github.com/ipld/go-ipld-prime/multicodec" "github.com/ipfs/kubo/core" @@ -97,9 +96,9 @@ type PluginLoader struct { func NewPluginLoader(repo string) (*PluginLoader, error) { loader := &PluginLoader{plugins: make(map[string]plugin.Plugin, len(preloadPlugins)), repo: repo} if repo != "" { - cfg, err := cserialize.Load(filepath.Join(repo, config.DefaultConfigFile)) + cfg, err := config.Load(filepath.Join(repo, config.DefaultConfigFile)) switch err { - case cserialize.ErrNotInitialized: + case config.ErrNotInitialized: case nil: loader.config = cfg.Plugins default: diff --git a/repo/fsrepo/fsrepo.go b/repo/fsrepo/fsrepo.go index 219f136f71e6..eb419c172ad9 100644 --- a/repo/fsrepo/fsrepo.go +++ b/repo/fsrepo/fsrepo.go @@ -23,7 +23,6 @@ import ( util "github.com/ipfs/go-ipfs-util" logging "github.com/ipfs/go-log" config "github.com/ipfs/kubo/config" - serialize "github.com/ipfs/kubo/config/serialize" "github.com/ipfs/kubo/repo/fsrepo/migrations" homedir "github.com/mitchellh/go-homedir" ma "github.com/multiformats/go-multiaddr" @@ -248,7 +247,7 @@ func initConfig(path string, conf *config.Config) error { // initialization is the one time when it's okay to write to the config // without reading the config from disk and merging any user-provided keys // that may exist. - if err := serialize.WriteConfigFile(configFilename, conf); err != nil { + if err := config.WriteConfigFile(configFilename, conf); err != nil { return err } @@ -429,7 +428,7 @@ func (r *FSRepo) SetGatewayAddr(addr net.Addr) error { // openConfig returns an error if the config file is not present. func (r *FSRepo) openConfig() error { - conf, err := serialize.Load(r.configFilePath) + conf, err := config.Load(r.configFilePath) if err != nil { return err } @@ -603,7 +602,7 @@ func (r *FSRepo) SetConfig(updated *config.Config) error { // as a map, write the updated struct values to the map and write the map // to disk. var mapconf map[string]interface{} - if err := serialize.ReadConfigFile(r.configFilePath, &mapconf); err != nil { + if err := config.ReadConfigFile(r.configFilePath, &mapconf); err != nil { return err } m, err := config.ToMap(updated) @@ -611,7 +610,7 @@ func (r *FSRepo) SetConfig(updated *config.Config) error { return err } mergedMap := common.MapMergeDeep(mapconf, m) - if err := serialize.WriteConfigFile(r.configFilePath, mergedMap); err != nil { + if err := config.WriteConfigFile(r.configFilePath, mergedMap); err != nil { return err } // Do not use `*r.config = ...`. This will modify the *shared* config @@ -630,7 +629,7 @@ func (r *FSRepo) GetConfigKey(key string) (interface{}, error) { } var cfg map[string]interface{} - if err := serialize.ReadConfigFile(r.configFilePath, &cfg); err != nil { + if err := config.ReadConfigFile(r.configFilePath, &cfg); err != nil { return nil, err } return common.MapGetKV(cfg, key) @@ -647,7 +646,7 @@ func (r *FSRepo) SetConfigKey(key string, value interface{}) error { // Load into a map so we don't end up writing any additional defaults to the config file. var mapconf map[string]interface{} - if err := serialize.ReadConfigFile(r.configFilePath, &mapconf); err != nil { + if err := config.ReadConfigFile(r.configFilePath, &mapconf); err != nil { return err } @@ -677,7 +676,7 @@ func (r *FSRepo) SetConfigKey(key string, value interface{}) error { } r.config = conf - if err := serialize.WriteConfigFile(r.configFilePath, mapconf); err != nil { + if err := config.WriteConfigFile(r.configFilePath, mapconf); err != nil { return err } diff --git a/test/sharness/t0021-config.sh b/test/sharness/t0021-config.sh index 5264908c73f8..cbfb8a878bb1 100755 --- a/test/sharness/t0021-config.sh +++ b/test/sharness/t0021-config.sh @@ -4,6 +4,8 @@ test_description="Test config command" . lib/test-lib.sh +export IPFS_CONFIG_TOLERANT_MODE=true + # we use a function so that we can run it both offline + online test_config_cmd_set() { @@ -87,7 +89,7 @@ test_profile_apply_dry_run_not_alter() { } test_config_cmd() { - test_config_cmd_set "beep" "boop" + test_config_cmd_set "beep" "boop" test_config_cmd_set "beep1" "boop2" test_config_cmd_set "beep1" "boop2" test_config_cmd_set "--bool" "beep2" "true" @@ -298,5 +300,8 @@ test_launch_ipfs_daemon test_config_cmd test_kill_ipfs_daemon +# Be sure that on strict mode we can set config values that exists on Config struct +export IPFS_CONFIG_TOLERANT_MODE= +test_config_cmd_set "--json" "Discovery.MDNS.Enabled" "false" test_done