diff --git a/golang/cosmos/x/vstorage/keeper/querier.go b/golang/cosmos/x/vstorage/keeper/querier.go index 44a8a8d40b4..8e2eb91db47 100644 --- a/golang/cosmos/x/vstorage/keeper/querier.go +++ b/golang/cosmos/x/vstorage/keeper/querier.go @@ -1,8 +1,6 @@ package keeper import ( - "strings" - abci "github.com/tendermint/tendermint/abci/types" "github.com/cosmos/cosmos-sdk/codec" @@ -18,14 +16,36 @@ const ( QueryChildren = "children" ) -// NewQuerier is the module level router for state queries +// getVstorageEntryPath validates that a request URL path represents a valid +// entry path with no extra data, and returns the path of that vstorage entry. +func getVstorageEntryPath(urlPathSegments []string) (string, error) { + if len(urlPathSegments) != 1 || types.ValidatePath(urlPathSegments[0]) != nil { + return "", sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "invalid vstorage entry path") + } + return urlPathSegments[0], nil +} + +// NewQuerier returns the function for handling queries routed to this module. +// It performs its own routing based on the first slash-separated URL path +// segment (e.g., URL path `/data/foo.bar` is a request for the value associated +// with vstorage path "foo.bar", and `/children/foo.bar` is a request for the +// child path segments immediately underneath vstorage path "foo.bar" which may +// be used to extend it to a vstorage path such as "foo.bar.baz"). func NewQuerier(keeper Keeper, legacyQuerierCdc *codec.LegacyAmino) sdk.Querier { - return func(ctx sdk.Context, path []string, req abci.RequestQuery) (res []byte, err error) { - switch path[0] { + return func(ctx sdk.Context, urlPathSegments []string, req abci.RequestQuery) (res []byte, err error) { + switch urlPathSegments[0] { case QueryData: - return queryData(ctx, strings.Join(path[1:], "/"), req, keeper, legacyQuerierCdc) + entryPath, entryPathErr := getVstorageEntryPath(urlPathSegments[1:]) + if entryPathErr != nil { + return nil, entryPathErr + } + return queryData(ctx, entryPath, req, keeper, legacyQuerierCdc) case QueryChildren: - return queryChildren(ctx, strings.Join(path[1:], "/"), req, keeper, legacyQuerierCdc) + entryPath, entryPathErr := getVstorageEntryPath(urlPathSegments[1:]) + if entryPathErr != nil { + return nil, entryPathErr + } + return queryChildren(ctx, entryPath, req, keeper, legacyQuerierCdc) default: return nil, sdkerrors.Wrap(sdkerrors.ErrUnknownRequest, "unknown vstorage query endpoint") } @@ -36,12 +56,12 @@ func NewQuerier(keeper Keeper, legacyQuerierCdc *codec.LegacyAmino) sdk.Querier func queryData(ctx sdk.Context, path string, req abci.RequestQuery, keeper Keeper, legacyQuerierCdc *codec.LegacyAmino) (res []byte, err error) { entry := keeper.GetEntry(ctx, path) if !entry.HasValue() { - return nil, sdkerrors.Wrap(sdkerrors.ErrUnknownRequest, "could not get vstorage path") + return nil, sdkerrors.Wrap(sdkerrors.ErrNotFound, "no data for vstorage path") } - bz, err2 := codec.MarshalJSONIndent(legacyQuerierCdc, types.Data{Value: entry.StringValue()}) - if err2 != nil { - return nil, sdkerrors.Wrap(sdkerrors.ErrJSONMarshal, err2.Error()) + bz, marshalErr := codec.MarshalJSONIndent(legacyQuerierCdc, types.Data{Value: entry.StringValue()}) + if marshalErr != nil { + return nil, sdkerrors.Wrap(sdkerrors.ErrJSONMarshal, marshalErr.Error()) } return bz, nil diff --git a/golang/cosmos/x/vstorage/types/path_keys.go b/golang/cosmos/x/vstorage/types/path_keys.go index 297022a903e..60703798c5d 100644 --- a/golang/cosmos/x/vstorage/types/path_keys.go +++ b/golang/cosmos/x/vstorage/types/path_keys.go @@ -7,9 +7,17 @@ import ( "strings" ) -// - A "path" is a sequence of zero or more dot-separated nonempty strings of -// 7-bit non-nul, non-dot ASCII characters. So `""`, `"foo"`, and -// `"foo.bar.baz"` are paths but `"."`, "foo.", and "fo\0o" are not. +// - A "path" is a sequence of zero or more dot-separated nonempty segments +// using a restricted alphabet of ASCII alphanumerics plus underscore and dash, +// consistent with packages/internal/src/lib-chainStorage.js but not currently +// enforcing a length restriction on path segments. +// So `""`, `"foo"`, and `"foo.bar__baz.qux--quux"` are paths but `"."`, +// `"foo/bar"`, `"fo\to"`, and `"foö"` are not. +// This alphabet might be expanded in the future, but such expansion SHOULD NOT +// include control characters (including those that are not ASCII, such as +// U+202E RIGHT-TO-LEFT OVERRIDE), slash `/` (which separates ABCI request path +// segments in e.g. `custom/vstorage/data/foo`), or backslash `\` (which should +// be reserved for adding escape sequences). // // - An encoded key for a path is the path prefixed with its length (in ASCII // digits), separated by nul, followed by the path with dots replaced with nul. @@ -42,22 +50,26 @@ func EncodedKeyToPath(key []byte) string { return string(pathBytes) } -var pathPattern = regexp.MustCompile(`[-a-zA-Z0-9_` + PathSeparator + `]*`) +var pathSegmentPattern = `[a-zA-Z0-9_-]+` +var pathSeparatorPattern = `\` + PathSeparator +var pathPattern = fmt.Sprintf(`^$|^%[1]s(%[2]s%[1]s)*$`, pathSegmentPattern, pathSeparatorPattern) +var pathMatcher = regexp.MustCompile(pathPattern) func ValidatePath(path string) error { - if !pathPattern.MatchString(path) { - return fmt.Errorf("path %q contains invalid characters", path) - } - if strings.Contains(path, PathSeparator+PathSeparator) { - return fmt.Errorf("path %q contains doubled separators", path) + if pathMatcher.MatchString(path) { + return nil } + // Rescan the string to give a useful error message. if strings.HasPrefix(path, PathSeparator) { return fmt.Errorf("path %q starts with separator", path) } if strings.HasSuffix(path, PathSeparator) { return fmt.Errorf("path %q ends with separator", path) } - return nil + if strings.Contains(path, PathSeparator+PathSeparator) { + return fmt.Errorf("path %q contains doubled separators", path) + } + return fmt.Errorf("path %q contains invalid characters", path) } // PathToEncodedKey converts a path to a byte slice key diff --git a/golang/cosmos/x/vstorage/types/path_keys_test.go b/golang/cosmos/x/vstorage/types/path_keys_test.go index e56eb5dfb23..0d9972e0907 100644 --- a/golang/cosmos/x/vstorage/types/path_keys_test.go +++ b/golang/cosmos/x/vstorage/types/path_keys_test.go @@ -2,40 +2,106 @@ package types import ( "bytes" + "fmt" + "strings" "testing" ) func Test_Key_Encoding(t *testing.T) { tests := []struct { - name string - childStr string - key []byte + name string + path string + key []byte + errContains string }{ { - name: "empty key is prefixed", - childStr: "", - key: []byte("0\x00"), + name: "empty path", + path: "", + key: []byte("0\x00"), }, { - name: "some key string", - childStr: "some", - key: []byte("1\x00some"), + name: "single-segment path", + path: "some", + key: []byte("1\x00some"), }, { - name: "dot-separated", - childStr: "some.child.grandchild", - key: []byte("3\x00some\x00child\x00grandchild"), + name: "multi-segment path", + path: "some.child.grandchild", + key: []byte("3\x00some\x00child\x00grandchild"), + }, + { + name: "non-letters", + path: "-_0_-", + key: []byte("1\x00-_0_-"), + }, + { + name: "lone dot", + path: ".", + errContains: "starts with separator", + }, + { + name: "starts with dot", + path: ".foo", + errContains: "starts with separator", + }, + { + name: "ends with dot", + path: "foo.", + errContains: "ends with separator", + }, + { + name: "empty path segment", + path: "foo..bar", + errContains: "doubled separators", + }, + { + name: "invalid path character U+0000 NUL", + path: "foo\x00bar", + errContains: "invalid character", + }, + { + name: "invalid path character U+002F SOLIDUS", + path: "foo/bar", + errContains: "invalid character", + }, + { + name: "invalid path character U+005C REVERSE SOLIDUS", + path: "foo\\bar", + errContains: "invalid character", + }, + { + name: "invalid path character U+007C VERTICAL LINE", + path: "foo|bar", + errContains: "invalid character", }, } for _, tt := range tests { + if tt.key != nil { + t.Run(tt.name, func(t *testing.T) { + if key := PathToEncodedKey(tt.path); !bytes.Equal(key, tt.key) { + t.Errorf("pathToKey(%q) = []byte(%q), want []byte(%q)", tt.path, key, tt.key) + } + if path := EncodedKeyToPath(tt.key); path != tt.path { + t.Errorf("keyToPath([]byte(%q)) = %q, want %q", tt.key, path, tt.path) + } + }) + continue + } + expect := tt.errContains t.Run(tt.name, func(t *testing.T) { - if key := PathToEncodedKey(tt.childStr); !bytes.Equal(key, tt.key) { - t.Errorf("pathToKey(%q) = %v, want %v", tt.childStr, key, tt.key) - } - if childStr := EncodedKeyToPath(tt.key); childStr != tt.childStr { - t.Errorf("keyToString(%v) = %q, want %q", tt.key, childStr, tt.childStr) - } + var key []byte + defer func() { + if err := recover(); err != nil { + errStr := fmt.Sprintf("%v", err) + if !strings.Contains(errStr, expect) { + t.Errorf("pathToKey(%q) = error %q, want error %v", tt.path, errStr, expect) + } + } else { + t.Errorf("pathToKey(%q) = []byte(%q), want error %v", tt.path, key, expect) + } + }() + key = PathToEncodedKey(tt.path) }) } } diff --git a/packages/internal/src/lib-chainStorage.js b/packages/internal/src/lib-chainStorage.js index b61f841971d..385fc886abe 100644 --- a/packages/internal/src/lib-chainStorage.js +++ b/packages/internal/src/lib-chainStorage.js @@ -95,6 +95,7 @@ harden(assertCapData); // Must be nonempty and disallow (unescaped) `.`, and for simplicity // (and future possibility of e.g. escaping) we currently limit to // ASCII alphanumeric plus underscore and dash. +// Should remain consistent with golang/cosmos/x/vstorage/types/path_keys.go const pathSegmentPattern = /^[a-zA-Z0-9_-]{1,100}$/; /** @type {(name: string) => void} */