From 40315fe0922ad0d6cf4dfc47d4ac59686fc32614 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Mon, 22 Aug 2022 13:41:57 +0300 Subject: [PATCH] cli: check manifest for validness if it's got from user input And adjust the test case along the way, unnamed arguments are not allowed for valid manifest. --- cli/smartcontract/generate.go | 12 ++++----- cli/smartcontract/generate_test.go | 40 +++++++++++++++-------------- cli/smartcontract/manifest.go | 11 ++++++-- cli/smartcontract/smart_contract.go | 2 +- 4 files changed, 36 insertions(+), 29 deletions(-) diff --git a/cli/smartcontract/generate.go b/cli/smartcontract/generate.go index ba33dfffb..44d6f6f02 100644 --- a/cli/smartcontract/generate.go +++ b/cli/smartcontract/generate.go @@ -43,7 +43,11 @@ func contractGenerateWrapper(ctx *cli.Context) error { if err := cmdargs.EnsureNone(ctx); err != nil { return err } - m, _, err := readManifest(ctx.String("manifest")) + h, err := util.Uint160DecodeStringLE(strings.TrimPrefix(ctx.String("hash"), "0x")) + if err != nil { + return cli.NewExitError(fmt.Errorf("invalid contract hash: %w", err), 1) + } + m, _, err := readManifest(ctx.String("manifest"), h) if err != nil { return cli.NewExitError(fmt.Errorf("can't read contract manifest: %w", err), 1) } @@ -59,13 +63,7 @@ func contractGenerateWrapper(ctx *cli.Context) error { return cli.NewExitError(fmt.Errorf("can't parse config file: %w", err), 1) } } - cfg.Manifest = m - - h, err := util.Uint160DecodeStringLE(strings.TrimPrefix(ctx.String("hash"), "0x")) - if err != nil { - return cli.NewExitError(fmt.Errorf("invalid contract hash: %w", err), 1) - } cfg.Hash = h f, err := os.Create(ctx.String("out")) diff --git a/cli/smartcontract/generate_test.go b/cli/smartcontract/generate_test.go index 64629c695..e7f57371e 100644 --- a/cli/smartcontract/generate_test.go +++ b/cli/smartcontract/generate_test.go @@ -66,13 +66,6 @@ func TestGenerate(t *testing.T) { }, ReturnType: smartcontract.BoolType, }, - manifest.Method{ - Name: "emptyName", - Parameters: []manifest.Parameter{ - manifest.NewParameter("", smartcontract.MapType), - }, - ReturnType: smartcontract.AnyType, - }, manifest.Method{ Name: "searchStorage", Parameters: []manifest.Parameter{ @@ -193,11 +186,6 @@ func OtherTypes(ctr interop.Hash160, tx interop.Hash256, sig interop.Signature, return neogointernal.CallWithToken(Hash, "otherTypes", int(contract.All), ctr, tx, sig, data).(bool) } -// EmptyName invokes ` + "`emptyName`" + ` method of contract. -func EmptyName(arg0 map[string]interface{}) interface{} { - return neogointernal.CallWithToken(Hash, "emptyName", int(contract.All), arg0).(interface{}) -} - // SearchStorage invokes ` + "`searchStorage`" + ` method of contract. func SearchStorage(ctx storage.Context) iterator.Iterator { return neogointernal.CallWithToken(Hash, "searchStorage", int(contract.All), ctx).(iterator.Iterator) @@ -288,27 +276,41 @@ func TestGenerate_Errors(t *testing.T) { err := app.Run(append([]string{"", "generate-wrapper"}, args...)) require.True(t, strings.Contains(err.Error(), msg), "got: %v", err) } + t.Run("invalid hash", func(t *testing.T) { + checkError(t, "invalid contract hash", "--hash", "xxx") + }) t.Run("missing manifest argument", func(t *testing.T) { - checkError(t, errNoManifestFile.Error()) + checkError(t, errNoManifestFile.Error(), "--hash", util.Uint160{}.StringLE()) }) t.Run("missing manifest file", func(t *testing.T) { - checkError(t, "can't read contract manifest", "--manifest", "notexists") + checkError(t, "can't read contract manifest", "--manifest", "notexists", "--hash", util.Uint160{}.StringLE()) + }) + t.Run("empty manifest", func(t *testing.T) { + manifestFile := filepath.Join(t.TempDir(), "invalid.json") + require.NoError(t, os.WriteFile(manifestFile, []byte("[]"), os.ModePerm)) + checkError(t, "json: cannot unmarshal array into Go value of type manifest.Manifest", "--manifest", manifestFile, "--hash", util.Uint160{}.StringLE()) }) t.Run("invalid manifest", func(t *testing.T) { manifestFile := filepath.Join(t.TempDir(), "invalid.json") - require.NoError(t, os.WriteFile(manifestFile, []byte("[]"), os.ModePerm)) - checkError(t, "", "--manifest", manifestFile) + m := manifest.NewManifest("MyContract") // no methods + rawManifest, err := json.Marshal(m) + require.NoError(t, err) + require.NoError(t, os.WriteFile(manifestFile, rawManifest, os.ModePerm)) + checkError(t, "ABI: no methods", "--manifest", manifestFile, "--hash", util.Uint160{}.StringLE()) }) manifestFile := filepath.Join(t.TempDir(), "manifest.json") m := manifest.NewManifest("MyContract") + m.ABI.Methods = append(m.ABI.Methods, manifest.Method{ + Name: "method0", + Offset: 0, + ReturnType: smartcontract.AnyType, + Safe: true, + }) rawManifest, err := json.Marshal(m) require.NoError(t, err) require.NoError(t, os.WriteFile(manifestFile, rawManifest, os.ModePerm)) - t.Run("invalid hash", func(t *testing.T) { - checkError(t, "invalid contract hash", "--manifest", manifestFile, "--hash", "xxx") - }) t.Run("missing config", func(t *testing.T) { checkError(t, "can't read config file", "--manifest", manifestFile, "--hash", util.Uint160{}.StringLE(), diff --git a/cli/smartcontract/manifest.go b/cli/smartcontract/manifest.go index 1f4eca2a2..498e66f77 100644 --- a/cli/smartcontract/manifest.go +++ b/cli/smartcontract/manifest.go @@ -11,6 +11,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/core/state" "github.com/nspcc-dev/neo-go/pkg/smartcontract/manifest" "github.com/nspcc-dev/neo-go/pkg/smartcontract/nef" + "github.com/nspcc-dev/neo-go/pkg/util" "github.com/urfave/cli" ) @@ -29,7 +30,7 @@ func manifestAddGroup(ctx *cli.Context) error { } mPath := ctx.String("manifest") - m, _, err := readManifest(mPath) + m, _, err := readManifest(mPath, util.Uint160{}) if err != nil { return cli.NewExitError(fmt.Errorf("can't read contract manifest: %w", err), 1) } @@ -89,7 +90,10 @@ func readNEFFile(filename string) (*nef.File, []byte, error) { return &nefFile, f, nil } -func readManifest(filename string) (*manifest.Manifest, []byte, error) { +// readManifest unmarshalls manifest got from the provided filename and checks +// it for validness against the provided contract hash. If empty hash is specified +// then no hash-related manifest groups check is performed. +func readManifest(filename string, hash util.Uint160) (*manifest.Manifest, []byte, error) { if len(filename) == 0 { return nil, nil, errNoManifestFile } @@ -104,5 +108,8 @@ func readManifest(filename string) (*manifest.Manifest, []byte, error) { if err != nil { return nil, nil, err } + if err := m.IsValid(hash); err != nil { + return nil, nil, fmt.Errorf("manifest is invalid: %w", err) + } return m, manifestBytes, nil } diff --git a/cli/smartcontract/smart_contract.go b/cli/smartcontract/smart_contract.go index e260306aa..a61222ecb 100644 --- a/cli/smartcontract/smart_contract.go +++ b/cli/smartcontract/smart_contract.go @@ -922,7 +922,7 @@ func contractDeploy(ctx *cli.Context) error { return cli.NewExitError(err, 1) } - m, manifestBytes, err := readManifest(ctx.String("manifest")) + m, manifestBytes, err := readManifest(ctx.String("manifest"), util.Uint160{}) if err != nil { return cli.NewExitError(fmt.Errorf("failed to read manifest file: %w", err), 1) }