Skip to content

Commit

Permalink
fix: prevent panic on depinject when input or output struct has an un… (
Browse files Browse the repository at this point in the history
#12786)

…exported fild



## Description

Closes: #1943



---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/main/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
  • Loading branch information
JeancarloBarrios authored Aug 3, 2022
1 parent 07f7035 commit 7103587
Show file tree
Hide file tree
Showing 3 changed files with 171 additions and 3 deletions.
5 changes: 5 additions & 0 deletions depinject/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,11 @@ func (c *container) build(loc Location, outputs ...interface{}) error {

for i, output := range outputs {
val := reflect.ValueOf(output)

if !values[i].CanInterface() {
return []reflect.Value{}, fmt.Errorf("depinject.Out struct %s on package can't have unexported field", values[i].String())

}
val.Elem().Set(values[i])
}

Expand Down
152 changes: 152 additions & 0 deletions depinject/container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,15 @@ type KeeperB struct {
msgClientA MsgClientA
}

type KeeperC struct {
key KVStoreKey
msgClientA MsgClientA
}

type KeeperD struct {
key KVStoreKey
}

type Handler struct {
Handle func()
}
Expand Down Expand Up @@ -82,6 +91,149 @@ func (ModuleB) Provide(dependencies BDependencies) (BProvides, Handler, error) {
}, Handler{}, nil
}

type ModuleUnexportedDependency struct{}

func (ModuleUnexportedDependency) Provide(dependencies UnexportedFieldCDependencies) (CProvides, Handler, error) {
return CProvides{
KeeperC: KeeperC{
key: dependencies.key,
msgClientA: dependencies.A,
},
Commands: []Command{{}, {}},
}, Handler{}, nil
}

type UnexportedFieldCDependencies struct {
depinject.In

key KVStoreKey
A MsgClientA
}

type CProvides struct {
depinject.Out

KeeperC KeeperC
Commands []Command
}

type ModuleUnexportedProvides struct{}

type CDependencies struct {
depinject.In

Key KVStoreKey
A MsgClientA
}

type UnexportedFieldCProvides struct {
depinject.Out

keeperC KeeperC
Commands []Command
}

func (ModuleUnexportedProvides) Provide(dependencies CDependencies) (UnexportedFieldCProvides, Handler, error) {
return UnexportedFieldCProvides{
keeperC: KeeperC{
key: dependencies.Key,
msgClientA: dependencies.A,
},
Commands: []Command{{}, {}},
}, Handler{}, nil
}

type ModuleD struct{}

type DDependencies struct {
depinject.In

Key KVStoreKey
KeeperC KeeperC
}

type DProvides struct {
depinject.Out

KeeperD KeeperD
Commands []Command
}

func (ModuleD) Provide(dependencies DDependencies) (DProvides, Handler, error) {
return DProvides{
KeeperD: KeeperD{
key: dependencies.Key,
},
Commands: []Command{{}, {}},
}, Handler{}, nil
}

func TestUnexportedField(t *testing.T) {
var (
handlers map[string]Handler
commands []Command
a KeeperA
c KeeperC
d KeeperD

scenarioConfigProvides = depinject.Configs(
depinject.Provide(ProvideMsgClientA),
depinject.ProvideInModule("runtime", ProvideKVStoreKey),
depinject.ProvideInModule("a", wrapMethod0(ModuleA{})),
depinject.ProvideInModule("c", wrapMethod0(ModuleUnexportedProvides{})),
)

scenarioConfigDependency = depinject.Configs(
depinject.Provide(ProvideMsgClientA),
depinject.ProvideInModule("runtime", ProvideKVStoreKey),
depinject.ProvideInModule("a", wrapMethod0(ModuleA{})),
depinject.ProvideInModule("c", wrapMethod0(ModuleUnexportedDependency{})),
)

scenarioConfigProvidesDependency = depinject.Configs(
depinject.Provide(ProvideMsgClientA),
depinject.ProvideInModule("runtime", ProvideKVStoreKey),
depinject.ProvideInModule("a", wrapMethod0(ModuleA{})),
depinject.ProvideInModule("c", wrapMethod0(ModuleUnexportedProvides{})),
depinject.ProvideInModule("d", wrapMethod0(ModuleD{})),
)
)

require.ErrorContains(t,
depinject.Inject(
scenarioConfigProvides,
&handlers,
&commands,
&a,
&c,
),
"depinject.Out struct",
)

require.ErrorContains(t,
depinject.Inject(
scenarioConfigDependency,
&handlers,
&commands,
&a,
&c,
),
"depinject.In struct",
)

require.ErrorContains(t,
depinject.Inject(
scenarioConfigProvidesDependency,
&handlers,
&commands,
&a,
&c,
&d,
),
"depinject.Out struct",
)
}

var scenarioConfig = depinject.Configs(
depinject.Provide(ProvideMsgClientA),
depinject.ProvideInModule("runtime", ProvideKVStoreKey),
Expand Down
17 changes: 14 additions & 3 deletions depinject/struct_args.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package depinject

import (
"fmt"
"reflect"

"github.com/pkg/errors"
Expand Down Expand Up @@ -74,7 +75,10 @@ func expandStructArgsFn(provider ProviderDescriptor) func(inputs []reflect.Value
inputs1 := make([]reflect.Value, len(inParams))
for i, in := range inParams {
if in.Type.AssignableTo(isInType) {
v, n := buildIn(in.Type, inputs[j:])
v, n, err := buildIn(in.Type, inputs[j:])
if err != nil {
return []reflect.Value{}, err
}
inputs1[i] = v
j += n
} else {
Expand Down Expand Up @@ -158,7 +162,7 @@ func structArgsOutTypes(typ reflect.Type) []ProviderOutput {
return res
}

func buildIn(typ reflect.Type, values []reflect.Value) (reflect.Value, int) {
func buildIn(typ reflect.Type, values []reflect.Value) (reflect.Value, int, error) {
numFields := typ.NumField()
j := 0
res := reflect.New(typ)
Expand All @@ -167,11 +171,18 @@ func buildIn(typ reflect.Type, values []reflect.Value) (reflect.Value, int) {
if f.Type.AssignableTo(isInType) {
continue
}
if !res.Elem().Field(i).CanSet() {
return reflect.Value{}, 0, fmt.Errorf("depinject.In struct %s on package %s can't have unexported field", res.Elem().String(), f.PkgPath)
}
if !values[j].CanInterface() {
return reflect.Value{}, 0, fmt.Errorf("depinject.Out struct %s on package %s can't have unexported field", res.Elem().String(), f.PkgPath)

}

res.Elem().Field(i).Set(values[j])
j++
}
return res.Elem(), j
return res.Elem(), j, nil
}

func extractFromOut(typ reflect.Type, value reflect.Value) []reflect.Value {
Expand Down

0 comments on commit 7103587

Please sign in to comment.