From b06a0b0e3244b624631ae090b9a5b59c2bd6520c Mon Sep 17 00:00:00 2001 From: rob-deutsch Date: Fri, 14 Sep 2018 12:24:10 +1000 Subject: [PATCH] fix behaviour of key rename to same name Renaming a key to the same name always succeeds regardless of force flag. Fixed bug whereby renaming to same name (with --force) would delete the key by accident fixed #5450 License: MIT Signed-off-by: Rob Deutsch --- core/coreapi/key.go | 8 +++++- core/coreapi/key_test.go | 56 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/core/coreapi/key.go b/core/coreapi/key.go index 036c4f2f9..46b7523c8 100644 --- a/core/coreapi/key.go +++ b/core/coreapi/key.go @@ -8,10 +8,10 @@ import ( coreiface "github.com/ipfs/go-ipfs/core/coreapi/interface" caopts "github.com/ipfs/go-ipfs/core/coreapi/interface/options" - ipfspath "gx/ipfs/QmX7uSbkNz76yNwBhuwYwRbhihLnJqM73VTCjS3UMJud9A/go-path" crypto "gx/ipfs/QmPvyPwuCgJ7pDmrKDxRtsScJgBaM5h4EpRL2qQJsmXf4n/go-libp2p-crypto" peer "gx/ipfs/QmQsErDt8Qgw1XrsXf2BpEzDgGWtB1YLsTAARBup5b6B9W/go-libp2p-peer" + ipfspath "gx/ipfs/QmX7uSbkNz76yNwBhuwYwRbhihLnJqM73VTCjS3UMJud9A/go-path" ) type KeyAPI CoreAPI @@ -159,6 +159,12 @@ func (api *KeyAPI) Rename(ctx context.Context, oldName string, newName string, o return nil, false, err } + // This is important, because future code will delete key `oldName` + // even if it is the same as newName. + if newName == oldName { + return &key{oldName, pid}, false, nil + } + overwrite := false if options.Force { exist, err := ks.Has(newName) diff --git a/core/coreapi/key_test.go b/core/coreapi/key_test.go index 55bc3b6b9..6d0796402 100644 --- a/core/coreapi/key_test.go +++ b/core/coreapi/key_test.go @@ -366,6 +366,62 @@ func TestRenameOverwrite(t *testing.T) { } } +func TestRenameSameNameNoForce(t *testing.T) { + ctx := context.Background() + _, api, err := makeAPI(ctx) + if err != nil { + t.Error(err) + } + + _, err = api.Key().Generate(ctx, "foo") + if err != nil { + t.Fatal(err) + return + } + + k, overwrote, err := api.Key().Rename(ctx, "foo", "foo") + if err != nil { + t.Fatal(err) + return + } + + if overwrote { + t.Error("overwrote should be false") + } + + if k.Name() != "foo" { + t.Errorf("returned key should be called 'foo', got '%s'", k.Name()) + } +} + +func TestRenameSameName(t *testing.T) { + ctx := context.Background() + _, api, err := makeAPI(ctx) + if err != nil { + t.Error(err) + } + + _, err = api.Key().Generate(ctx, "foo") + if err != nil { + t.Fatal(err) + return + } + + k, overwrote, err := api.Key().Rename(ctx, "foo", "foo", opt.Key.Force(true)) + if err != nil { + t.Fatal(err) + return + } + + if overwrote { + t.Error("overwrote should be false") + } + + if k.Name() != "foo" { + t.Errorf("returned key should be called 'foo', got '%s'", k.Name()) + } +} + func TestRemove(t *testing.T) { ctx := context.Background() _, api, err := makeAPI(ctx)