From 9fcaa9130b4132bebb31d2ad0efcf24e9ec9554b Mon Sep 17 00:00:00 2001 From: Gareth Kirwan Date: Thu, 6 Mar 2025 01:12:57 +0000 Subject: [PATCH] Config: Tighten config version handling as uint16 (#1825) * Config: Tighten config version handling as uint16 This constrains the versions to uint16 and improves error handling. LatestVersion becomes literally that. Fixes handling for negative or overflowing versions in config * Config: Rename LatestVersion to UseLatestVersion --- cmd/config/config.go | 13 ++++---- config/config.go | 2 +- config/versions/versions.go | 51 +++++++++++++++++++------------- config/versions/versions_test.go | 45 +++++++++++++++------------- 4 files changed, 62 insertions(+), 49 deletions(-) diff --git a/cmd/config/config.go b/cmd/config/config.go index 0952955e..dc22ca71 100644 --- a/cmd/config/config.go +++ b/cmd/config/config.go @@ -4,6 +4,7 @@ import ( "context" "flag" "fmt" + "math" "os" "slices" "strings" @@ -23,7 +24,7 @@ func main() { var in, out, keyStr string var inplace bool - var version int + var version uint fs := flag.NewFlagSet("config", flag.ExitOnError) fs.Usage = func() { usage(fs) } @@ -31,7 +32,7 @@ func main() { fs.StringVar(&out, "out", "[in].out", "The config output file") fs.BoolVar(&inplace, "edit", false, "Edit; Save result to the original file") fs.StringVar(&keyStr, "key", "", "The key to use for AES encryption") - fs.IntVar(&version, "version", 0, "The version to downgrade to") + fs.UintVar(&version, "version", 0, "The version to downgrade to") cmd, args := parseCommand(os.Args[1:]) if cmd == "" { @@ -85,13 +86,13 @@ func main() { usage(fs) os.Exit(3) } - version = versions.LatestVersion - } else if version < 0 { - fmt.Fprintln(os.Stderr, "Error: version must be positive") + version = versions.UseLatestVersion + } else if version >= math.MaxUint16 { + fmt.Fprintln(os.Stderr, "Error: version must be less than 65535") usage(fs) os.Exit(3) } - if data, err = versions.Manager.Deploy(context.Background(), data, version); err != nil { + if data, err = versions.Manager.Deploy(context.Background(), data, uint16(version)); err != nil { fatal("Unable to " + cmd + " config; Error: " + err.Error()) } if !isEncrypted { diff --git a/config/config.go b/config/config.go index 89c3a1f9..615da017 100644 --- a/config/config.go +++ b/config/config.go @@ -1504,7 +1504,7 @@ func (c *Config) readConfig(d io.Reader) error { } } - if j, err = versions.Manager.Deploy(context.Background(), j, versions.LatestVersion); err != nil { + if j, err = versions.Manager.Deploy(context.Background(), j, versions.UseLatestVersion); err != nil { return err } diff --git a/config/versions/versions.go b/config/versions/versions.go index cdaa6600..f669de05 100644 --- a/config/versions/versions.go +++ b/config/versions/versions.go @@ -17,6 +17,7 @@ import ( "errors" "fmt" "log" + "math" "os" "slices" "strconv" @@ -26,17 +27,20 @@ import ( "github.com/thrasher-corp/gocryptotrader/common" ) -// LatestVersion used as version param to Deploy to automatically use the latest version -const LatestVersion = -1 +// UseLatestVersion used as version param to Deploy to automatically use the latest version +const UseLatestVersion = math.MaxUint16 var ( - errMissingVersion = errors.New("missing version") - errVersionIncompatible = errors.New("version does not implement ConfigVersion or ExchangeVersion") - errModifyingExchange = errors.New("error modifying exchange config") - errNoVersions = errors.New("error retrieving latest config version: No config versions are registered") - errApplyingVersion = errors.New("error applying version") - errConfigVersion = errors.New("version in config file is higher than the latest available version") - errTargetVersion = errors.New("target downgrade version is higher than the latest available version") + errMissingVersion = errors.New("missing version") + errVersionIncompatible = errors.New("version does not implement ConfigVersion or ExchangeVersion") + errModifyingExchange = errors.New("error modifying exchange config") + errNoVersions = errors.New("error retrieving latest config version: No config versions are registered") + errApplyingVersion = errors.New("error applying version") + errTargetVersion = errors.New("target downgrade version is higher than the latest available version") + errConfigVersion = errors.New("invalid version in config") + errConfigVersionUnavail = errors.New("version is higher than the latest available version") + errConfigVersionNegative = errors.New("version is negative") + errConfigVersionMax = errors.New("version is above max versions") ) // ConfigVersion is a version that affects the general configuration @@ -62,9 +66,9 @@ type manager struct { var Manager = &manager{} // Deploy upgrades or downgrades the config between versions -// Pass LatestVersion for version to use the latest version automatically +// Pass UseLatestVersion for version to use the latest version automatically // Prints an error an exits if the config file version or version param is not registered -func (m *manager) Deploy(ctx context.Context, j []byte, version int) ([]byte, error) { +func (m *manager) Deploy(ctx context.Context, j []byte, version uint16) ([]byte, error) { if err := m.checkVersions(); err != nil { return j, err } @@ -75,7 +79,7 @@ func (m *manager) Deploy(ctx context.Context, j []byte, version int) ([]byte, er } target := latest - if version != LatestVersion { + if version != UseLatestVersion { target = version } @@ -83,20 +87,25 @@ func (m *manager) Deploy(ctx context.Context, j []byte, version int) ([]byte, er defer m.m.RUnlock() current64, err := jsonparser.GetInt(j, "version") - current := int(current64) switch { case errors.Is(err, jsonparser.KeyPathNotFoundError): - current = -1 + // With no version first upgrade is to Version1; current64 is already 0 case err != nil: - return j, fmt.Errorf("%w `version`: %w", common.ErrGettingField, err) + return j, fmt.Errorf("%w: %w `version`: %w", errConfigVersion, common.ErrGettingField, err) + case current64 < 0: + return j, fmt.Errorf("%w: %w `version`: `%d`", errConfigVersion, errConfigVersionNegative, current64) + case current64 >= UseLatestVersion: + return j, fmt.Errorf("%w: %w `version`: `%d`", errConfigVersion, errConfigVersionMax, current64) } + current := uint16(current64) switch { case target == current: return j, nil case latest < current: - warnVersionNotRegistered(current, latest, errConfigVersion) - return j, errConfigVersion + err := fmt.Errorf("%w: %w", errConfigVersion, errConfigVersionUnavail) + warnVersionNotRegistered(current, latest, err) + return j, err case target > latest: warnVersionNotRegistered(target, latest, errTargetVersion) return j, errTargetVersion @@ -136,7 +145,7 @@ func (m *manager) Deploy(ctx context.Context, j []byte, version int) ([]byte, er current = patchVersion - 1 } - if j, err = jsonparser.Set(j, []byte(strconv.Itoa(current)), "version"); err != nil { + if j, err = jsonparser.Set(j, []byte(strconv.FormatUint(uint64(current), 10)), "version"); err != nil { return j, fmt.Errorf("%w `version` during %s %v: %w", common.ErrSettingField, action, patchVersion, err) } } @@ -202,13 +211,13 @@ func (m *manager) registerVersion(ver int, v any) { } // latest returns the highest version number -func (m *manager) latest() (int, error) { +func (m *manager) latest() (uint16, error) { m.m.RLock() defer m.m.RUnlock() if len(m.versions) == 0 { return 0, errNoVersions } - return len(m.versions) - 1, nil + return uint16(len(m.versions)) - 1, nil } // checkVersions ensures that registered versions are consistent @@ -228,7 +237,7 @@ func (m *manager) checkVersions() error { return nil } -func warnVersionNotRegistered(current, latest int, msg error) { +func warnVersionNotRegistered(current, latest uint16, msg error) { fmt.Fprintf(os.Stderr, ` %s ('%d' > '%d') Switch back to the version of GoCryptoTrader containing config version '%d' and run: diff --git a/config/versions/versions_test.go b/config/versions/versions_test.go index a98eae09..320fc755 100644 --- a/config/versions/versions_test.go +++ b/config/versions/versions_test.go @@ -13,56 +13,59 @@ import ( func TestDeploy(t *testing.T) { t.Parallel() m := manager{} - _, err := m.Deploy(context.Background(), []byte(``), LatestVersion) + _, err := m.Deploy(context.Background(), []byte(``), UseLatestVersion) assert.ErrorIs(t, err, errNoVersions) m.registerVersion(1, &TestVersion1{}) - _, err = m.Deploy(context.Background(), []byte(``), LatestVersion) + _, err = m.Deploy(context.Background(), []byte(``), UseLatestVersion) require.ErrorIs(t, err, errVersionIncompatible) m = manager{} m.registerVersion(0, &Version0{}) - _, err = m.Deploy(context.Background(), []byte(`not an object`), LatestVersion) + m.registerVersion(1, &Version1{}) + _, err = m.Deploy(context.Background(), []byte(`not an object`), UseLatestVersion) require.ErrorIs(t, err, jsonparser.KeyPathNotFoundError, "Must throw the correct error trying to add version to bad json") require.ErrorIs(t, err, common.ErrSettingField, "Must throw the correct error trying to add version to bad json") require.ErrorContains(t, err, "version", "Must throw the correct error trying to add version to bad json") - _, err = m.Deploy(context.Background(), []byte(`{"version":"not an int"}`), LatestVersion) + _, err = m.Deploy(context.Background(), []byte(`{"version":"not an int"}`), UseLatestVersion) require.ErrorIs(t, err, common.ErrGettingField, "Must throw the correct error trying to get version from bad json") - in := []byte(`{"version":0,"exchanges":[{"name":"Juan"}]}`) - j, err := m.Deploy(context.Background(), in, LatestVersion) - require.NoError(t, err) - assert.Equal(t, string(in), string(j)) + _, err = m.Deploy(context.Background(), []byte(`{"version":65535}`), UseLatestVersion) + require.ErrorIs(t, err, errConfigVersionMax, "Must throw the correct error when version is too high") - m.registerVersion(1, &Version1{}) - j, err = m.Deploy(context.Background(), in, LatestVersion) + _, err = m.Deploy(context.Background(), []byte(`{"version":-1}`), UseLatestVersion) + require.ErrorIs(t, err, errConfigVersionNegative, "Must throw the correct error when version is negative") + + in := []byte(`{"version":0,"exchanges":[{"name":"Juan"}]}`) + j, err := m.Deploy(context.Background(), in, UseLatestVersion) require.NoError(t, err) assert.Contains(t, string(j), `"version": 1`) + j2, err := m.Deploy(context.Background(), j, UseLatestVersion) + require.NoError(t, err, "Deploy the same version again must not error") + require.Equal(t, string(j2), string(j), "Deploy the same version again must not change config") + _, err = m.Deploy(context.Background(), j, 2) assert.ErrorIs(t, err, errTargetVersion, "Downgrade to a unregistered version should not be allowed") m.versions = append(m.versions, &TestVersion2{ConfigErr: true, ExchErr: false}) - _, err = m.Deploy(context.Background(), j, LatestVersion) + _, err = m.Deploy(context.Background(), j, UseLatestVersion) require.ErrorIs(t, err, errUpgrade) m.versions[len(m.versions)-1] = &TestVersion2{ConfigErr: false, ExchErr: true} - _, err = m.Deploy(context.Background(), in, LatestVersion) + _, err = m.Deploy(context.Background(), in, UseLatestVersion) require.Implements(t, (*ExchangeVersion)(nil), m.versions[1]) require.ErrorIs(t, err, errUpgrade) - j2, err := m.Deploy(context.Background(), j, 0) + j2, err = m.Deploy(context.Background(), j, 0) require.NoError(t, err) assert.Contains(t, string(j2), `"version": 0`, "Explicit downgrade should work correctly") m.versions = m.versions[:1] - _, err = m.Deploy(context.Background(), j, LatestVersion) - assert.ErrorIs(t, err, errConfigVersion, "Config version ahead of latest version should error") - - _, err = m.Deploy(context.Background(), j, 0) - assert.ErrorIs(t, err, errConfigVersion, "Config version ahead of latest version should error") + _, err = m.Deploy(context.Background(), j, UseLatestVersion) + assert.ErrorIs(t, err, errConfigVersionUnavail, "Config version ahead of latest version should error") } // TestExchangeDeploy exercises exchangeDeploy @@ -70,7 +73,7 @@ func TestDeploy(t *testing.T) { func TestExchangeDeploy(t *testing.T) { t.Parallel() m := manager{} - _, err := m.Deploy(context.Background(), []byte(``), LatestVersion) + _, err := m.Deploy(context.Background(), []byte(``), UseLatestVersion) assert.ErrorIs(t, err, errNoVersions) v := &TestVersion2{} @@ -113,10 +116,10 @@ func TestLatest(t *testing.T) { m.registerVersion(1, &Version1{}) v, err := m.latest() require.NoError(t, err) - assert.Equal(t, 1, v) + assert.Equal(t, uint16(1), v) m.registerVersion(2, &Version2{}) v, err = m.latest() require.NoError(t, err) - assert.Equal(t, 2, v) + assert.Equal(t, uint16(2), v) }