Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exposing an interface will not prevent calling methods from the underlying type #744

Open
StarpTech opened this issue Jan 4, 2025 · 2 comments
Assignees
Labels

Comments

@StarpTech
Copy link

StarpTech commented Jan 4, 2025

Hello, as titled. We figured out that the library doesn't hide methods that aren't accessible by the interface definition. We would expect that interface boundaries are enforced.

Simple example:

type Context {
	Header RequestHeaders
}
type RequestHeaders interface {
	Get(key string) string
}

Pseudocode:

code := `header.Set('foo', 'bar')`

program, err := expr.Compile(code, expr.Env(Context{}))
if err != nil {
    panic(err)
}

output, err := expr.Run(program, Context{Header: req.header})
if err != nil {
    panic(err)
}

Workaround

It looks like expr:"-"can be used as a tag to prevent exposing fields. I haven't find it in the docs unfortunately.

@antonmedv
Copy link
Member

Hello! Yes, I think this is a problem. Expr uses reflect to check if method is available on provided instance (not interface). As during runtime real object is present.

We need to improve the type checker, and enforce strict validation during compile time.

Nice idea with expr:"-". Please open separate issue to track it.

@breml
Copy link

breml commented Jan 7, 2025

I hit this issue as well, but slightly different. In my case, I pass an interface to expr during compile time with expr.Env(...) and I observed, that compiling the expression fails, if I just pass a nil instance of the interface and the expression contains one of the methods defined in the interface. My use case for this is, that I want to validate an expression provided by the user, that later will be executed against a concrete instances implementing the interface. But these instances are not yet known when the user provides the expression.

Minimal reproducer:

package main

import "github.com/expr-lang/expr"

type greeter interface {
	Hi() string
}

type say struct {
	name string
}

func (s say) Hi() string {
	return "Hi " + s.name
}

var _ greeter = say{}

func main() {
	var err error
	s := say{name: "foo"}

	_, err = expr.Compile(`Hi() == "Hi foo"`, expr.Env(s))
	if err != nil {
		panic("struct: " + err.Error())
	}

	var g greeter = say{name: "foo"}

	_, err = expr.Compile(`Hi() == "Hi foo"`, expr.Env(g))
	if err != nil {
		panic("non nil interface: " + err.Error())
	}

	var nilGreeter greeter

	_, err = expr.Compile(`Hi() == "Hi foo"`, expr.Env(nilGreeter))
	if err != nil {
		panic("nil interface: " + err.Error())
	}
}

Expected output: no panic

Actual output:

panic: nil interface: unknown name Hi (1:1)
 | Hi() == "Hi foo"
 | ^

goroutine 1 [running]:
main.main()
        /workspaces/migration-manager/internal/batch/test/main.go:39 +0x1f4
exit status 2

Side note: when I created the above reproducer, I found that expr also is not able to handle types derived from base types, which provide methods. My initial definition for say was like this (which also implements the above mentioned interface):

type say string

func (s say) Hi() string {
	return "Hi " + string(s)
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants