Skip to content

Commit

Permalink
interp: fix type assertion for wrapped empty interface
Browse files Browse the repository at this point in the history
Although empty interfaces are usually not wrapped, for compatibility with the runtime, we may have to wrap them sometime into `valueInterface` type.

It allows to preserve interpreter type metadata for interface values exchanged with the runtime. It is necessary to resolve methods and receivers in the absence of reflect support.

During type assertions on empty interfaces, we now handle a possible valueInterface and dereference the original value to pursue the type assertion.

In the same change, we have improved the format of some panic messages at runtime to give location of offending source at interpreter level.

This change will allow to fix traefik/traefik#9362.
  • Loading branch information
mvertes committed Oct 3, 2022
1 parent dfeddbe commit 143e4a4
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 14 deletions.
56 changes: 56 additions & 0 deletions _test/cli7.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package main

import (
"fmt"
"io"
"log"
"net/http"
"net/http/httptest"
)

type T struct {
http.ResponseWriter
}

type mw1 struct {
}

var obj = map[string]interface{}{}

func (m *mw1) ServeHTTP(rw http.ResponseWriter, rq *http.Request) {
t := &T{
ResponseWriter: rw,
}
x := t.Header()
i := obj["m1"].(*mw1)
fmt.Fprint(rw, "Welcome to my website!", x, i)
}

func main() {
m1 := &mw1{}

obj["m1"] = m1

mux := http.NewServeMux()
mux.HandleFunc("/", m1.ServeHTTP)

server := httptest.NewServer(mux)
defer server.Close()

client(server.URL)
}

func client(uri string) {
resp, err := http.Get(uri)
if err != nil {
log.Fatal(err)
}
body, err := io.ReadAll(resp.Body)
if err != nil {
log.Fatal(err)
}
fmt.Println(string(body))
}

// Output:
// Welcome to my website!map[] &{}
6 changes: 3 additions & 3 deletions _test/type24.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,6 @@ func assertValue() {
}

// Output:
// interface conversion: interface {} is int, not string
// interface conversion: interface {} is nil, not string
// interface conversion: *httptest.ResponseRecorder is not http.Pusher: missing method Push
// 22:10: interface conversion: interface {} is int, not string
// 32:10: interface conversion: interface {} is nil, not string
// 42:10: interface conversion: *httptest.ResponseRecorder is not http.Pusher: missing method Push
1 change: 1 addition & 0 deletions interp/interp_consistent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ func TestInterpConsistencyBuild(t *testing.T) {
file.Name() == "range9.go" || // expect error
file.Name() == "unsafe6.go" || // needs go.mod to be 1.17
file.Name() == "unsafe7.go" || // needs go.mod to be 1.17
file.Name() == "type24.go" || // expect error
file.Name() == "type27.go" || // expect error
file.Name() == "type28.go" || // expect error
file.Name() == "type29.go" || // expect error
Expand Down
5 changes: 4 additions & 1 deletion interp/interp_file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"go/token"
"os"
"path/filepath"
"regexp"
"strings"
"testing"

Expand Down Expand Up @@ -76,7 +77,9 @@ func runCheck(t *testing.T, p string) {
t.Fatal(err)
}

if res := strings.TrimSpace(stdout.String()); res != wanted {
// Remove path in output, to have results independent of location.
re := regexp.MustCompile(p + ":")
if res := re.ReplaceAllString(strings.TrimSpace(stdout.String()), ""); res != wanted {
t.Errorf("\ngot: %q,\nwant: %q", res, wanted)
}
}
Expand Down
24 changes: 14 additions & 10 deletions interp/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -401,15 +401,15 @@ func typeAssert(n *node, withResult, withOk bool) {
ok = v.IsValid()
if !ok {
if !withOk {
panic(fmt.Sprintf("interface conversion: interface {} is nil, not %s", rtype.String()))
panic(n.cfgErrorf("interface conversion: interface {} is nil, not %s", rtype.String()))
}
return next
}
ok = canAssertTypes(leftType, rtype)
if !ok {
if !withOk {
method := firstMissingMethod(leftType, rtype)
panic(fmt.Sprintf("interface conversion: %s is not %s: missing method %s", leftType.String(), rtype.String(), method))
panic(n.cfgErrorf("interface conversion: %s is not %s: missing method %s", leftType.String(), rtype.String(), method))
}
return next
}
Expand All @@ -430,14 +430,18 @@ func typeAssert(n *node, withResult, withOk bool) {
concrete := val.Interface()
ctyp := reflect.TypeOf(concrete)

if vv, ok := concrete.(valueInterface); ok {
ctyp = vv.value.Type()
concrete = vv.value.Interface()
}
ok = canAssertTypes(ctyp, rtype)
if !ok {
if !withOk {
// TODO(mpl): think about whether this should ever happen.
if ctyp == nil {
panic(fmt.Sprintf("interface conversion: interface {} is nil, not %s", rtype.String()))
panic(n.cfgErrorf("interface conversion: interface {} is nil, not %s", rtype.String()))
}
panic(fmt.Sprintf("interface conversion: interface {} is %s, not %s", ctyp.String(), rtype.String()))
panic(n.cfgErrorf("interface conversion: interface {} is %s, not %s", ctyp.String(), rtype.String()))
}
return next
}
Expand All @@ -462,7 +466,7 @@ func typeAssert(n *node, withResult, withOk bool) {
}
if !ok {
if !withOk {
panic(fmt.Sprintf("interface conversion: interface {} is nil, not %s", rtype.String()))
panic(n.cfgErrorf("interface conversion: interface {} is nil, not %s", rtype.String()))
}
return next
}
Expand All @@ -475,7 +479,7 @@ func typeAssert(n *node, withResult, withOk bool) {
if !ok {
if !withOk {
method := firstMissingMethod(v.Type(), rtype)
panic(fmt.Sprintf("interface conversion: %s is not %s: missing method %s", v.Type().String(), rtype.String(), method))
panic(n.cfgErrorf("interface conversion: %s is not %s: missing method %s", v.Type().String(), rtype.String(), method))
}
return next
}
Expand All @@ -495,15 +499,15 @@ func typeAssert(n *node, withResult, withOk bool) {
if !ok || !v.value.IsValid() {
ok = false
if !withOk {
panic(fmt.Sprintf("interface conversion: interface {} is nil, not %s", rtype.String()))
panic(n.cfgErrorf("interface conversion: interface {} is nil, not %s", rtype.String()))
}
return next
}

ok = canAssertTypes(v.value.Type(), rtype)
if !ok {
if !withOk {
panic(fmt.Sprintf("interface conversion: interface {} is %s, not %s", v.value.Type().String(), rtype.String()))
panic(n.cfgErrorf("interface conversion: interface {} is %s, not %s", v.value.Type().String(), rtype.String()))
}
return next
}
Expand Down Expand Up @@ -567,7 +571,7 @@ func convert(n *node) {
n.exec = func(f *frame) bltn {
n, ok := value(f).Interface().(*node)
if !ok || !n.typ.convertibleTo(c.typ) {
panic("cannot convert")
panic(n.cfgErrorf("cannot convert to %s", c.typ.id()))
}
n1 := *n
n1.typ = c.typ
Expand Down Expand Up @@ -3611,7 +3615,7 @@ func convertConstantValue(n *node) {
case constant.Int:
i, x := constant.Int64Val(c)
if !x {
panic(fmt.Sprintf("constant %s overflows int64", c.ExactString()))
panic(n.cfgErrorf("constant %s overflows int64", c.ExactString()))
}
v = reflect.ValueOf(int(i))
case constant.Float:
Expand Down

0 comments on commit 143e4a4

Please sign in to comment.