Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 3d0aec6

Browse files
diegommmantonmedv
andauthored
Fix #844 alternative 2: do not allow access to unexported struct fields (#846)
* add regression test * fix #844: disallow direct access to unexported struct fields --------- Co-authored-by: Anton Medvedev <anton@medv.io>
1 parent 7dbd409 commit 3d0aec6

File tree

3 files changed

+199
-4
lines changed

3 files changed

+199
-4
lines changed

‎checker/checker_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -760,8 +760,8 @@ func TestCheck_TaggedFieldName(t *testing.T) {
760760

761761
config := conf.CreateNew()
762762
expr.Env(struct {
763-
x struct {
764-
y bool `expr:"bar"`
763+
X struct {
764+
Y bool `expr:"bar"`
765765
} `expr:"foo"`
766766
}{})(config)
767767
expr.AsBool()(config)

‎checker/nature/utils.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,13 @@ func (s *structData) structField(c *Cache, parentEmbed *structData, name string)
5252
// Lookup own fields first.
5353
for ; s.ownIdx < s.numField; s.ownIdx++ {
5454
field := s.rType.Field(s.ownIdx)
55-
// BUG: we should skip if !field.IsExported() here
56-
5755
if field.Anonymous && s.anonIdx < 0 {
5856
// start iterating anon fields on the first instead of zero
5957
s.anonIdx = s.ownIdx
6058
}
59+
if !field.IsExported() {
60+
continue
61+
}
6162
fName, ok := fieldName(field.Name, field.Tag)
6263
if !ok || fName == "" {
6364
// name can still be empty for a type created at runtime with

‎test/issues/844/issue_test.go‎

Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
package main
2+
3+
import (
4+
"strings"
5+
"testing"
6+
7+
"github.com/expr-lang/expr"
8+
"github.com/expr-lang/expr/checker"
9+
"github.com/expr-lang/expr/conf"
10+
"github.com/expr-lang/expr/internal/testify/require"
11+
"github.com/expr-lang/expr/parser"
12+
)
13+
14+
func TestIssue844(t *testing.T) {
15+
testCases := []struct {
16+
name string
17+
env any
18+
expression string
19+
shouldFail bool
20+
}{
21+
{
22+
name: "exported env, exported field",
23+
env: ExportedEnv{},
24+
expression: `ExportedEmbedded`,
25+
shouldFail: false,
26+
},
27+
{
28+
name: "exported env, unexported field",
29+
env: ExportedEnv{},
30+
expression: `unexportedEmbedded`,
31+
shouldFail: true,
32+
},
33+
{
34+
name: "exported env, exported field inherited from exported field",
35+
env: ExportedEnv{},
36+
expression: `Str`,
37+
shouldFail: false,
38+
},
39+
{
40+
name: "exported env, unexported field inherited from exported field",
41+
env: ExportedEnv{},
42+
expression: `str`,
43+
shouldFail: true,
44+
},
45+
{
46+
name: "exported env, exported field inherited from exported field",
47+
env: ExportedEnv{},
48+
expression: `Integer`,
49+
shouldFail: false,
50+
},
51+
{
52+
name: "exported env, unexported field inherited from exported field",
53+
env: ExportedEnv{},
54+
expression: `integer`,
55+
shouldFail: true,
56+
},
57+
{
58+
name: "exported env, exported field directly accessed from exported field",
59+
env: ExportedEnv{},
60+
expression: `ExportedEmbedded.Str`,
61+
shouldFail: false,
62+
},
63+
{
64+
name: "exported env, unexported field directly accessed from exported field",
65+
env: ExportedEnv{},
66+
expression: `ExportedEmbedded.str`,
67+
shouldFail: true,
68+
},
69+
{
70+
name: "exported env, exported field directly accessed from exported field",
71+
env: ExportedEnv{},
72+
expression: `unexportedEmbedded.Integer`,
73+
shouldFail: true,
74+
},
75+
{
76+
name: "exported env, unexported field directly accessed from exported field",
77+
env: ExportedEnv{},
78+
expression: `unexportedEmbedded.integer`,
79+
shouldFail: true,
80+
},
81+
{
82+
name: "unexported env, exported field",
83+
env: unexportedEnv{},
84+
expression: `ExportedEmbedded`,
85+
shouldFail: false,
86+
},
87+
{
88+
name: "unexported env, unexported field",
89+
env: unexportedEnv{},
90+
expression: `unexportedEmbedded`,
91+
shouldFail: true,
92+
},
93+
{
94+
name: "unexported env, exported field inherited from exported field",
95+
env: unexportedEnv{},
96+
expression: `Str`,
97+
shouldFail: false,
98+
},
99+
{
100+
name: "unexported env, unexported field inherited from exported field",
101+
env: unexportedEnv{},
102+
expression: `str`,
103+
shouldFail: true,
104+
},
105+
{
106+
name: "unexported env, exported field inherited from exported field",
107+
env: unexportedEnv{},
108+
expression: `Integer`,
109+
shouldFail: false,
110+
},
111+
{
112+
name: "unexported env, unexported field inherited from exported field",
113+
env: unexportedEnv{},
114+
expression: `integer`,
115+
shouldFail: true,
116+
},
117+
{
118+
name: "unexported env, exported field directly accessed from exported field",
119+
env: unexportedEnv{},
120+
expression: `ExportedEmbedded.Str`,
121+
shouldFail: false,
122+
},
123+
{
124+
name: "unexported env, unexported field directly accessed from exported field",
125+
env: unexportedEnv{},
126+
expression: `ExportedEmbedded.str`,
127+
shouldFail: true,
128+
},
129+
{
130+
name: "unexported env, exported field directly accessed from exported field",
131+
env: unexportedEnv{},
132+
expression: `unexportedEmbedded.Integer`,
133+
shouldFail: true,
134+
},
135+
{
136+
name: "unexported env, unexported field directly accessed from exported field",
137+
env: unexportedEnv{},
138+
expression: `unexportedEmbedded.integer`,
139+
shouldFail: true,
140+
},
141+
}
142+
143+
for _, tc := range testCases {
144+
t.Run(tc.name, func(t *testing.T) {
145+
config := conf.New(tc.env)
146+
tree, err := parser.ParseWithConfig(tc.expression, config)
147+
require.NoError(t, err)
148+
149+
_, err = new(checker.Checker).PatchAndCheck(tree, config)
150+
if tc.shouldFail {
151+
require.Error(t, err)
152+
errStr := err.Error()
153+
if !strings.Contains(errStr, "unknown name") &&
154+
!strings.Contains(errStr, " has no field ") {
155+
t.Fatalf("expected a different error, got: %v", err)
156+
}
157+
} else {
158+
require.NoError(t, err)
159+
}
160+
161+
// We add this because the issue was actually not catching something
162+
// that sometimes failed with the error:
163+
// reflect.Value.Interface: cannot return value obtained from unexported field or method
164+
// This way, we test that everything we allow passing is also
165+
// allowed later
166+
_, err = expr.Eval(tc.expression, tc.env)
167+
if tc.shouldFail {
168+
require.Error(t, err)
169+
} else {
170+
require.NoError(t, err)
171+
}
172+
})
173+
}
174+
}
175+
176+
type ExportedEnv struct {
177+
ExportedEmbedded
178+
unexportedEmbedded
179+
}
180+
181+
type unexportedEnv struct {
182+
ExportedEmbedded
183+
unexportedEmbedded
184+
}
185+
186+
type ExportedEmbedded struct {
187+
Str string
188+
str string
189+
}
190+
191+
type unexportedEmbedded struct {
192+
Integer int
193+
integer int
194+
}

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /