Skip to content

Commit 94d50bf

Browse files
valyria257valyria257
authored
Merge pull request #30 from nginxinc/analyze-map
Fix analyze map body validation
2 parents e515f5f + 154eff2 commit 94d50bf

File tree

4 files changed

+263
-33
lines changed

4 files changed

+263
-33
lines changed

‎analyze_map.go

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
/**
2+
* Copyright (c) F5, Inc.
3+
*
4+
* This source code is licensed under the Apache License, Version 2.0 license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
package crossplane
9+
10+
import "fmt"
11+
12+
// mapParameterMasks holds bit masks that define the behavior of the body of map-like directives.
13+
// Some map directives have "special parameter" with different behaviors than the default.
14+
type mapParameterMasks struct {
15+
specialParameterMasks map[string]uint
16+
defaultMasks uint
17+
}
18+
19+
//nolint:gochecknoglobals
20+
var mapBodies = map[string]mapParameterMasks{
21+
"charset_map": {
22+
defaultMasks: ngxConfTake1,
23+
},
24+
"geo": {
25+
specialParameterMasks: map[string]uint{"ranges": ngxConfNoArgs, "proxy_recursive": ngxConfNoArgs},
26+
defaultMasks: ngxConfTake1,
27+
},
28+
"map": {
29+
specialParameterMasks: map[string]uint{"volatile": ngxConfNoArgs, "hostnames": ngxConfNoArgs},
30+
defaultMasks: ngxConfTake1,
31+
},
32+
"match": {
33+
defaultMasks: ngxConf1More,
34+
},
35+
}
36+
37+
// analyzeMapBody validates the body of a map-like directive. Map-like directives are block directives
38+
// that don't contain nginx directives, and therefore cannot be analyzed in the same way as other blocks.
39+
func analyzeMapBody(fname string, parameter *Directive, term string, mapCtx string) error {
40+
masks, known := mapBodies[mapCtx]
41+
42+
// if we're not inside a known map-like directive, don't bother analyzing
43+
if !known {
44+
return nil
45+
}
46+
47+
if term != ";" {
48+
return &ParseError{
49+
What: fmt.Sprintf(`unexpected "%s"`, term),
50+
File: &fname,
51+
Line: &parameter.Line,
52+
}
53+
}
54+
55+
if mask, ok := masks.specialParameterMasks[parameter.Directive]; ok {
56+
// use mask to check the parameter's arguments
57+
if hasValidArguments(mask, parameter.Args) {
58+
return nil
59+
}
60+
61+
return &ParseError{
62+
What: "invalid number of parameters",
63+
File: &fname,
64+
Line: &parameter.Line,
65+
}
66+
}
67+
68+
mask := masks.defaultMasks
69+
70+
// use mask to check the parameter's arguments
71+
if hasValidArguments(mask, parameter.Args) {
72+
return nil
73+
}
74+
75+
return &ParseError{
76+
What: "invalid number of parameters",
77+
File: &fname,
78+
Line: &parameter.Line,
79+
}
80+
}
81+
82+
func hasValidArguments(mask uint, args []string) bool {
83+
return ((mask>>len(args)&1) != 0 && len(args) <= 7) || // NOARGS to TAKE7
84+
((mask&ngxConfFlag) != 0 && len(args) == 1 && validFlag(args[0])) ||
85+
((mask & ngxConfAny) != 0) ||
86+
((mask&ngxConf1More) != 0 && len(args) >= 1) ||
87+
((mask&ngxConf2More) != 0 && len(args) >= 2)
88+
}

‎analyze_map_test.go

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
/**
2+
* Copyright (c) F5, Inc.
3+
*
4+
* This source code is licensed under the Apache License, Version 2.0 license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
package crossplane
9+
10+
import (
11+
"testing"
12+
13+
"github.com/stretchr/testify/require"
14+
)
15+
16+
// nolint:funlen
17+
func TestAnalyzeMapBody(t *testing.T) {
18+
t.Parallel()
19+
20+
testcases := map[string]struct {
21+
mapDirective string
22+
parameter *Directive
23+
term string
24+
wantErr *ParseError
25+
}{
26+
"valid map": {
27+
mapDirective: "map",
28+
parameter: &Directive{
29+
Directive: "default",
30+
Args: []string{"0"},
31+
Line: 5,
32+
Block: Directives{},
33+
},
34+
term: ";",
35+
},
36+
"valid map volatile parameter": {
37+
mapDirective: "map",
38+
parameter: &Directive{
39+
Directive: "volatile",
40+
Line: 5,
41+
Block: Directives{},
42+
},
43+
term: ";",
44+
},
45+
"invalid map volatile parameter": {
46+
mapDirective: "map",
47+
parameter: &Directive{
48+
Directive: "volatile",
49+
Args: []string{"1"},
50+
Line: 5,
51+
Block: Directives{},
52+
},
53+
term: ";",
54+
wantErr: &ParseError{What: "invalid number of parameters"},
55+
},
56+
"valid map hostnames parameter": {
57+
mapDirective: "map",
58+
parameter: &Directive{
59+
Directive: "hostnames",
60+
Line: 5,
61+
Block: Directives{},
62+
},
63+
term: ";",
64+
},
65+
"invalid map hostnames parameter": {
66+
mapDirective: "map",
67+
parameter: &Directive{
68+
Directive: "hostnames",
69+
Args: []string{"foo"},
70+
Line: 5,
71+
Block: Directives{},
72+
},
73+
term: ";",
74+
wantErr: &ParseError{What: "invalid number of parameters"},
75+
},
76+
"valid geo proxy_recursive parameter": {
77+
mapDirective: "geo",
78+
parameter: &Directive{
79+
Directive: "proxy_recursive",
80+
Line: 5,
81+
Block: Directives{},
82+
},
83+
term: ";",
84+
},
85+
"invalid geo proxy_recursive parameter": {
86+
mapDirective: "geo",
87+
parameter: &Directive{
88+
Directive: "proxy_recursive",
89+
Args: []string{"1"},
90+
Line: 5,
91+
Block: Directives{},
92+
},
93+
term: ";",
94+
wantErr: &ParseError{What: "invalid number of parameters"},
95+
},
96+
"valid geo ranges parameter": {
97+
mapDirective: "geo",
98+
parameter: &Directive{
99+
Directive: "ranges",
100+
Line: 5,
101+
Block: Directives{},
102+
},
103+
term: ";",
104+
},
105+
"invalid geo ranges parameter": {
106+
mapDirective: "geo",
107+
parameter: &Directive{
108+
Directive: "ranges",
109+
Args: []string{"0", "0", "0"},
110+
Line: 5,
111+
Block: Directives{},
112+
},
113+
term: ";",
114+
wantErr: &ParseError{What: "invalid number of parameters"},
115+
},
116+
"invalid number of parameters in map": {
117+
mapDirective: "map",
118+
parameter: &Directive{
119+
Directive: "default",
120+
Args: []string{"0", "0"},
121+
Line: 5,
122+
Block: Directives{},
123+
},
124+
term: ";",
125+
wantErr: &ParseError{What: "invalid number of parameters"},
126+
},
127+
"missing semicolon": {
128+
mapDirective: "map",
129+
parameter: &Directive{
130+
Directive: "default",
131+
Args: []string{"0", "0"},
132+
Line: 5,
133+
Block: Directives{},
134+
},
135+
term: "}",
136+
wantErr: &ParseError{What: `unexpected "}"`},
137+
},
138+
}
139+
140+
for name, tc := range testcases {
141+
tc := tc
142+
t.Run(name, func(t *testing.T) {
143+
t.Parallel()
144+
145+
err := analyzeMapBody("nginx.conf", tc.parameter, tc.term, tc.mapDirective)
146+
if tc.wantErr == nil {
147+
require.NoError(t, err)
148+
return
149+
}
150+
151+
require.Error(t, err)
152+
153+
var perr *ParseError
154+
require.ErrorAs(t, err, &perr)
155+
require.Equal(t, tc.wantErr.What, perr.What)
156+
})
157+
}
158+
}

‎analyze_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ func TestAnalyze_auth_jwt(t *testing.T) {
7171
Line: 5,
7272
},
7373
blockCtx{"http", "location", "limit_except"},
74-
true,
74+
false,
7575
},
7676
"auth_jwt not ok": {
7777
&Directive{
@@ -108,11 +108,11 @@ func TestAnalyze_auth_jwt(t *testing.T) {
108108
t.Parallel()
109109
err := analyze("nginx.conf", tc.stmt, ";", tc.ctx, &ParseOptions{})
110110

111-
if tc.wantErr && err != nil {
111+
if !tc.wantErr && err != nil {
112112
t.Fatal(err)
113113
}
114114

115-
if !tc.wantErr && err == nil {
115+
if tc.wantErr && err == nil {
116116
t.Fatal("expected error, got nil")
117117
}
118118
})

‎parse.go

Lines changed: 14 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -268,21 +268,23 @@ func (p *parser) parse(parsing *Config, tokens <-chan NgxToken, ctx blockCtx, co
268268
}
269269
}
270270

271-
// if inside map or geo block - add contents to payload, but do not parse further
272-
if len(ctx) > 0 && (ctx[len(ctx)-1] == "map" || ctx[len(ctx)-1] == "charset_map" || ctx[len(ctx)-1] == "geo") {
273-
mapErr := analyzeMapContents(parsing.File, stmt, t.Value)
274-
if mapErr != nil && p.options.StopParsingOnError {
275-
return nil, mapErr
276-
} else if mapErr != nil {
277-
p.handleError(parsing, mapErr)
278-
// consume invalid block
279-
if t.Value == "{" && !t.IsQuoted {
280-
_, _ = p.parse(parsing, tokens, nil, true)
271+
// if inside "map-like" block - add contents to payload, but do not parse further
272+
if len(ctx) > 0 {
273+
if _, ok := mapBodies[ctx[len(ctx)-1]]; ok {
274+
mapErr := analyzeMapBody(parsing.File, stmt, t.Value, ctx[len(ctx)-1])
275+
if mapErr != nil && p.options.StopParsingOnError {
276+
return nil, mapErr
277+
} else if mapErr != nil {
278+
p.handleError(parsing, mapErr)
279+
// consume invalid block
280+
if t.Value == "{" && !t.IsQuoted {
281+
_, _ = p.parse(parsing, tokens, nil, true)
282+
}
283+
continue
281284
}
285+
parsed = append(parsed, stmt)
282286
continue
283287
}
284-
parsed = append(parsed, stmt)
285-
continue
286288
}
287289

288290
// consume the directive if it is ignored and move on
@@ -442,21 +444,3 @@ func (p *parser) isAcyclic() bool {
442444
}
443445
return fileCount != len(p.includeInDegree)
444446
}
445-
446-
func analyzeMapContents(fname string, stmt *Directive, term string) error {
447-
if term != ";" {
448-
return &ParseError{
449-
What: fmt.Sprintf(`unexpected "%s"`, term),
450-
File: &fname,
451-
Line: &stmt.Line,
452-
}
453-
}
454-
if len(stmt.Args) != 1 && stmt.Directive != "ranges" {
455-
return &ParseError{
456-
What: "invalid number of parameters",
457-
File: &fname,
458-
Line: &stmt.Line,
459-
}
460-
}
461-
return nil
462-
}

0 commit comments

Comments
 (0)