Skip to content

Commit 445bb92

Browse files
author
James Hamlin
committed
Support colon in final path segment, last match wins behavior (behind flags)
Signed-off-by: James Hamlin <[email protected]>
1 parent cddead4 commit 445bb92

File tree

7 files changed

+152
-21
lines changed

7 files changed

+152
-21
lines changed

‎protoc-gen-grpc-gateway/descriptor/registry.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,10 @@ type Registry struct {
6767
// If false, the default behavior is to concat the last 2 elements of the FQN if they are unique, otherwise concat
6868
// all the elements of the FQN without any separator
6969
useFQNForSwaggerName bool
70+
71+
// allowColonFinalSegments determines whether colons are permitted
72+
// in the final segment of a path.
73+
allowColonFinalSegments bool
7074
}
7175

7276
type repeatedFieldSeparator struct {
@@ -422,6 +426,16 @@ func (r *Registry) SetUseFQNForSwaggerName(use bool) {
422426
r.useFQNForSwaggerName = use
423427
}
424428

429+
// GetAllowColonFinalSegments returns allowColonFinalSegments
430+
func (r *Registry) GetAllowColonFinalSegments() bool {
431+
return r.allowColonFinalSegments
432+
}
433+
434+
// SetAllowColonFinalSegments sets allowColonFinalSegments
435+
func (r *Registry) SetAllowColonFinalSegments(use bool) {
436+
r.allowColonFinalSegments = use
437+
}
438+
425439
// GetUseFQNForSwaggerName returns useFQNForSwaggerName
426440
func (r *Registry) GetUseFQNForSwaggerName() bool {
427441
return r.useFQNForSwaggerName

‎protoc-gen-grpc-gateway/gengateway/template.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ type trailerParams struct {
136136
Services []*descriptor.Service
137137
UseRequestContext bool
138138
RegisterFuncSuffix string
139+
AssumeColonVerb bool
139140
}
140141

141142
func applyTemplate(p param, reg *descriptor.Registry) (string, error) {
@@ -176,10 +177,15 @@ func applyTemplate(p param, reg *descriptor.Registry) (string, error) {
176177
return "", errNoTargetService
177178
}
178179

180+
assumeColonVerb := true
181+
if reg != nil {
182+
assumeColonVerb = !reg.GetAllowColonFinalSegments()
183+
}
179184
tp := trailerParams{
180185
Services: targetServices,
181186
UseRequestContext: p.UseRequestContext,
182187
RegisterFuncSuffix: p.RegisterFuncSuffix,
188+
AssumeColonVerb: assumeColonVerb,
183189
}
184190
if err := trailerTemplate.Execute(w, tp); err != nil {
185191
return "", err
@@ -517,7 +523,7 @@ func (m response_{{$svc.GetName}}_{{$m.GetName}}_{{$b.Index}}) XXX_ResponseBody(
517523
var (
518524
{{range $m := $svc.Methods}}
519525
{{range $b := $m.Bindings}}
520-
pattern_{{$svc.GetName}}_{{$m.GetName}}_{{$b.Index}} = runtime.MustPattern(runtime.NewPattern({{$b.PathTmpl.Version}}, {{$b.PathTmpl.OpCodes | printf "%#v"}}, {{$b.PathTmpl.Pool | printf "%#v"}}, {{$b.PathTmpl.Verb | printf "%q"}}))
526+
pattern_{{$svc.GetName}}_{{$m.GetName}}_{{$b.Index}} = runtime.MustPattern(runtime.NewPattern({{$b.PathTmpl.Version}}, {{$b.PathTmpl.OpCodes | printf "%#v"}}, {{$b.PathTmpl.Pool | printf "%#v"}}, {{$b.PathTmpl.Verb | printf "%q"}}, runtime.AssumeColonVerbOpt({{$.AssumeColonVerb}})))
521527
{{end}}
522528
{{end}}
523529
)

‎protoc-gen-grpc-gateway/gengateway/template_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ func TestApplyTemplateRequestWithoutClientStreaming(t *testing.T) {
242242
if want := `func RegisterExampleServiceHandler(ctx context.Context, mux *runtime.ServeMux, conn *grpc.ClientConn) error {`; !strings.Contains(got, want) {
243243
t.Errorf("applyTemplate(%#v) = %s; want to contain %s", file, got, want)
244244
}
245-
if want := `pattern_ExampleService_Echo_0 = runtime.MustPattern(runtime.NewPattern(1, []int{0, 0}, []string(nil), ""))`; !strings.Contains(got, want) {
245+
if want := `pattern_ExampleService_Echo_0 = runtime.MustPattern(runtime.NewPattern(1, []int{0, 0}, []string(nil), "", runtime.AssumeColonVerbOpt(true)))`; !strings.Contains(got, want) {
246246
t.Errorf("applyTemplate(%#v) = %s; want to contain %s", file, got, want)
247247
}
248248
}
@@ -394,7 +394,7 @@ func TestApplyTemplateRequestWithClientStreaming(t *testing.T) {
394394
if want := `func RegisterExampleServiceHandler(ctx context.Context, mux *runtime.ServeMux, conn *grpc.ClientConn) error {`; !strings.Contains(got, want) {
395395
t.Errorf("applyTemplate(%#v) = %s; want to contain %s", file, got, want)
396396
}
397-
if want := `pattern_ExampleService_Echo_0 = runtime.MustPattern(runtime.NewPattern(1, []int{0, 0}, []string(nil), ""))`; !strings.Contains(got, want) {
397+
if want := `pattern_ExampleService_Echo_0 = runtime.MustPattern(runtime.NewPattern(1, []int{0, 0}, []string(nil), "", runtime.AssumeColonVerbOpt(true)))`; !strings.Contains(got, want) {
398398
t.Errorf("applyTemplate(%#v) = %s; want to contain %s", file, got, want)
399399
}
400400
}

‎protoc-gen-grpc-gateway/main.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ var (
3333
allowRepeatedFieldsInBody = flag.Bool("allow_repeated_fields_in_body", false, "allows to use repeated field in `body` and `response_body` field of `google.api.http` annotation option")
3434
repeatedPathParamSeparator = flag.String("repeated_path_param_separator", "csv", "configures how repeated fields should be split. Allowed values are `csv`, `pipes`, `ssv` and `tsv`.")
3535
allowPatchFeature = flag.Bool("allow_patch_feature", true, "determines whether to use PATCH feature involving update masks (using google.protobuf.FieldMask).")
36+
allowColonFinalSegments = flag.Bool("allow_colon_final_segments", false, "determines whether colons are permitted in the final segment of a path")
3637
versionFlag = flag.Bool("version", false, "print the current verison")
3738
)
3839

@@ -93,6 +94,7 @@ func main() {
9394
reg.SetImportPath(*importPath)
9495
reg.SetAllowDeleteBody(*allowDeleteBody)
9596
reg.SetAllowRepeatedFieldsInBody(*allowRepeatedFieldsInBody)
97+
reg.SetAllowColonFinalSegments(*allowColonFinalSegments)
9698
if err := reg.SetRepeatedPathParamSeparator(*repeatedPathParamSeparator); err != nil {
9799
emitError(err)
98100
return

‎runtime/mux.go

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ type ServeMux struct {
3737
streamErrorHandler StreamErrorHandlerFunc
3838
protoErrorHandler ProtoErrorHandlerFunc
3939
disablePathLengthFallback bool
40+
lastMatchWins bool
4041
}
4142

4243
// ServeMuxOption is an option that can be given to a ServeMux on construction.
@@ -133,6 +134,15 @@ func WithStreamErrorHandler(fn StreamErrorHandlerFunc) ServeMuxOption {
133134
}
134135
}
135136

137+
// WithLastMatchWins returns a ServeMuxOption that will enable "last
138+
// match wins" behavior, where if multiple path patterns match a
139+
// request path, the last one defined in the .proto file will be used.
140+
func WithLastMatchWins() ServeMuxOption {
141+
return func(serveMux *ServeMux) {
142+
serveMux.lastMatchWins = true
143+
}
144+
}
145+
136146
// NewServeMux returns a new ServeMux whose internal mapping is empty.
137147
func NewServeMux(opts ...ServeMuxOption) *ServeMux {
138148
serveMux := &ServeMux{
@@ -173,7 +183,11 @@ func NewServeMux(opts ...ServeMuxOption) *ServeMux {
173183

174184
// Handle associates "h" to the pair of HTTP method and path pattern.
175185
func (s *ServeMux) Handle(meth string, pat Pattern, h HandlerFunc) {
176-
s.handlers[meth] = append(s.handlers[meth], handler{pat: pat, h: h})
186+
if s.lastMatchWins {
187+
s.handlers[meth] = append([]handler{handler{pat: pat, h: h}}, s.handlers[meth]...)
188+
} else {
189+
s.handlers[meth] = append(s.handlers[meth], handler{pat: pat, h: h})
190+
}
177191
}
178192

179193
// ServeHTTP dispatches the request to the first handler whose pattern matches to r.Method and r.Path.

‎runtime/mux_test.go

Lines changed: 69 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ func TestMuxServeHTTP(t *testing.T) {
2222
verb string
2323
}
2424
for _, spec := range []struct {
25-
patterns []stubPattern
25+
patterns []stubPattern
26+
patternOpts []runtime.PatternOpt
2627

2728
reqMethod string
2829
reqPath string
@@ -33,6 +34,7 @@ func TestMuxServeHTTP(t *testing.T) {
3334

3435
disablePathLengthFallback bool
3536
errHandler runtime.ProtoErrorHandlerFunc
37+
muxOpts []runtime.ServeMuxOption
3638
}{
3739
{
3840
patterns: nil,
@@ -253,11 +255,11 @@ func TestMuxServeHTTP(t *testing.T) {
253255
pool: []string{"unimplemented"},
254256
},
255257
},
256-
reqMethod: "GET",
257-
reqPath: "/foobar",
258+
reqMethod: "GET",
259+
reqPath: "/foobar",
258260
respStatus: http.StatusNotFound,
259261
respContent: "GET /foobar",
260-
errHandler: unknownPathIs404,
262+
errHandler: unknownPathIs404,
261263
},
262264
{
263265
// server returning unimplemented results in 'Not Implemented' code
@@ -269,14 +271,72 @@ func TestMuxServeHTTP(t *testing.T) {
269271
pool: []string{"unimplemented"},
270272
},
271273
},
272-
reqMethod: "GET",
273-
reqPath: "/unimplemented",
274+
reqMethod: "GET",
275+
reqPath: "/unimplemented",
274276
respStatus: http.StatusNotImplemented,
275277
respContent: `GET /unimplemented`,
276-
errHandler: unknownPathIs404,
278+
errHandler: unknownPathIs404,
279+
},
280+
{
281+
patterns: []stubPattern{
282+
{
283+
method: "GET",
284+
ops: []int{int(utilities.OpLitPush), 0, int(utilities.OpPush), 0, int(utilities.OpConcatN), 1, int(utilities.OpCapture), 1},
285+
pool: []string{"foo", "id"},
286+
},
287+
},
288+
patternOpts: []runtime.PatternOpt{runtime.AssumeColonVerbOpt(false)},
289+
reqMethod: "GET",
290+
reqPath: "/foo/bar",
291+
headers: map[string]string{
292+
"Content-Type": "application/json",
293+
},
294+
respStatus: http.StatusOK,
295+
respContent: "GET /foo/{id=*}",
296+
},
297+
{
298+
patterns: []stubPattern{
299+
{
300+
method: "GET",
301+
ops: []int{int(utilities.OpLitPush), 0, int(utilities.OpPush), 0, int(utilities.OpConcatN), 1, int(utilities.OpCapture), 1},
302+
pool: []string{"foo", "id"},
303+
},
304+
},
305+
patternOpts: []runtime.PatternOpt{runtime.AssumeColonVerbOpt(false)},
306+
reqMethod: "GET",
307+
reqPath: "/foo/bar:123",
308+
headers: map[string]string{
309+
"Content-Type": "application/json",
310+
},
311+
respStatus: http.StatusOK,
312+
respContent: "GET /foo/{id=*}",
313+
},
314+
{
315+
patterns: []stubPattern{
316+
{
317+
method: "POST",
318+
ops: []int{int(utilities.OpLitPush), 0, int(utilities.OpPush), 0, int(utilities.OpConcatN), 1, int(utilities.OpCapture), 1},
319+
pool: []string{"foo", "id"},
320+
},
321+
{
322+
method: "POST",
323+
ops: []int{int(utilities.OpLitPush), 0, int(utilities.OpPush), 0, int(utilities.OpConcatN), 1, int(utilities.OpCapture), 1},
324+
pool: []string{"foo", "id"},
325+
verb: "verb",
326+
},
327+
},
328+
patternOpts: []runtime.PatternOpt{runtime.AssumeColonVerbOpt(false)},
329+
reqMethod: "POST",
330+
reqPath: "/foo/bar:verb",
331+
headers: map[string]string{
332+
"Content-Type": "application/json",
333+
},
334+
respStatus: http.StatusOK,
335+
respContent: "POST /foo/{id=*}:verb",
336+
muxOpts: []runtime.ServeMuxOption{runtime.WithLastMatchWins()},
277337
},
278338
} {
279-
var opts []runtime.ServeMuxOption
339+
opts := spec.muxOpts
280340
if spec.disablePathLengthFallback {
281341
opts = append(opts, runtime.WithDisablePathLengthFallback())
282342
}
@@ -286,7 +346,7 @@ func TestMuxServeHTTP(t *testing.T) {
286346
mux := runtime.NewServeMux(opts...)
287347
for _, p := range spec.patterns {
288348
func(p stubPattern) {
289-
pat, err := runtime.NewPattern(1, p.ops, p.pool, p.verb)
349+
pat, err := runtime.NewPattern(1, p.ops, p.pool, p.verb, spec.patternOpts...)
290350
if err != nil {
291351
t.Fatalf("runtime.NewPattern(1, %#v, %#v, %q) failed with %v; want success", p.ops, p.pool, p.verb, err)
292352
}

‎runtime/pattern.go

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,31 @@ type Pattern struct {
3535
tailLen int
3636
// verb is the VERB part of the path pattern. It is empty if the pattern does not have VERB part.
3737
verb string
38+
// assumeColonVerb indicates whether a path suffix after a final
39+
// colon may only be interpreted as a verb.
40+
assumeColonVerb bool
3841
}
3942

43+
type patternOptions struct {
44+
assumeColonVerb bool
45+
}
46+
47+
// PatternOpt is an option for creating Patterns.
48+
type PatternOpt func(*patternOptions)
49+
4050
// NewPattern returns a new Pattern from the given definition values.
4151
// "ops" is a sequence of op codes. "pool" is a constant pool.
4252
// "verb" is the verb part of the pattern. It is empty if the pattern does not have the part.
4353
// "version" must be 1 for now.
4454
// It returns an error if the given definition is invalid.
45-
func NewPattern(version int, ops []int, pool []string, verb string) (Pattern, error) {
55+
func NewPattern(version int, ops []int, pool []string, verb string, opts ...PatternOpt) (Pattern, error) {
56+
options := patternOptions{
57+
assumeColonVerb: true,
58+
}
59+
for _, o := range opts {
60+
o(&options)
61+
}
62+
4663
if version != 1 {
4764
grpclog.Infof("unsupported version: %d", version)
4865
return Pattern{}, ErrInvalidPattern
@@ -122,12 +139,13 @@ func NewPattern(version int, ops []int, pool []string, verb string) (Pattern, er
122139
typedOps = append(typedOps, op)
123140
}
124141
return Pattern{
125-
ops: typedOps,
126-
pool: pool,
127-
vars: vars,
128-
stacksize: maxstack,
129-
tailLen: tailLen,
130-
verb: verb,
142+
ops: typedOps,
143+
pool: pool,
144+
vars: vars,
145+
stacksize: maxstack,
146+
tailLen: tailLen,
147+
verb: verb,
148+
assumeColonVerb: options.assumeColonVerb,
131149
}, nil
132150
}
133151

@@ -144,7 +162,16 @@ func MustPattern(p Pattern, err error) Pattern {
144162
// If otherwise, the function returns an error.
145163
func (p Pattern) Match(components []string, verb string) (map[string]string, error) {
146164
if p.verb != verb {
147-
return nil, ErrNotMatch
165+
if p.assumeColonVerb || p.verb != "" {
166+
return nil, ErrNotMatch
167+
}
168+
if len(components) == 0 {
169+
components = []string{":" + verb}
170+
} else {
171+
components = append([]string{}, components...)
172+
components[len(components)-1] += ":" + verb
173+
}
174+
verb = ""
148175
}
149176

150177
var pos int
@@ -225,3 +252,11 @@ func (p Pattern) String() string {
225252
}
226253
return "/" + segs
227254
}
255+
256+
// AssumeColonVerbOpt indicates whether a path suffix after a final
257+
// colon may only be interpreted as a verb.
258+
func AssumeColonVerbOpt(val bool) PatternOpt {
259+
return PatternOpt(func(o *patternOptions) {
260+
o.assumeColonVerb = val
261+
})
262+
}

0 commit comments

Comments
 (0)