Skip to content

Commit

Permalink
Disallow dollar sign in opaque value, allow recursive expansion to be…
Browse files Browse the repository at this point in the history
… implemented in the future (#6268)

Signed-off-by: Bogdan <bogdandrutu@gmail.com>

Signed-off-by: Bogdan <bogdandrutu@gmail.com>
  • Loading branch information
bogdandrutu committed Oct 11, 2022
1 parent 3875a92 commit ff73e49
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 1 deletion.
5 changes: 4 additions & 1 deletion confmap/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ has a `<scheme>` associated with it, and will provide configs for `configURI` th
This format is compatible with the URI definition (see [RFC 3986](https://datatracker.ietf.org/doc/html/rfc3986)).
The `<scheme>` MUST be always included in the `configURI`. The scheme for any `Provider` MUST be at least 2
characters long to avoid conflicting with a driver-letter identifier as specified in
[file URI syntax](https://tools.ietf.org/id/draft-kerwin-file-scheme-07.html#syntax).
[file URI syntax](https://datatracker.ietf.org/doc/html/rfc8089#section-2).

## Converter

Expand All @@ -37,6 +37,9 @@ that can be used by code that is oblivious to the usage of `Providers` and `Conv
or an individual value (partial configuration) when the `configURI` is embedded into the `Conf` as a values using
the syntax `${configURI}`.

**Limitation:** when embed a `${configURI}` the uri cannot contain dollar sign ("$") character. This is to allow the
current implementation to evolve in the future to support embedded uri within uri, e.g. `${http://my.domain.com?os=${OS}}`.

```terminal
Resolver Provider
Resolve │ │
Expand Down
3 changes: 3 additions & 0 deletions confmap/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,9 @@ func (mr *Resolver) expandValue(ctx context.Context, value interface{}) (interfa
if err != nil {
return nil, false, err
}
if strings.Contains(lURI.opaqueValue, "$") {
return nil, false, fmt.Errorf("the uri %q contains unsupported characters ('$')", lURI.asString())
}
ret, err := mr.retrieveValue(ctx, lURI)
if err != nil {
return nil, false, err
Expand Down
17 changes: 17 additions & 0 deletions confmap/resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,23 @@ func TestResolverExpandInvalidScheme(t *testing.T) {
assert.EqualError(t, err, `invalid uri: "g_c_s:VALUE"`)
}

func TestResolverExpandInvalidOpaqueValue(t *testing.T) {
provider := newFakeProvider("input", func(context.Context, string, WatcherFunc) (*Retrieved, error) {
return NewRetrieved(map[string]interface{}{"test": []interface{}{map[string]interface{}{"test": "${test:$VALUE}"}}})
})

testProvider := newFakeProvider("test", func(context.Context, string, WatcherFunc) (*Retrieved, error) {
return NewRetrieved(errors.New("invalid value"))
})

resolver, err := NewResolver(ResolverSettings{URIs: []string{"input:"}, Providers: makeMapProvidersMap(provider, testProvider), Converters: nil})
require.NoError(t, err)
resolver.enableExpand = true

_, err = resolver.Resolve(context.Background())
assert.EqualError(t, err, `the uri "test:$VALUE" contains unsupported characters ('$')`)
}

func makeMapProvidersMap(providers ...Provider) map[string]Provider {
ret := make(map[string]Provider, len(providers))
for _, provider := range providers {
Expand Down

0 comments on commit ff73e49

Please sign in to comment.