Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions runtime/mux.go
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,7 @@ type handler struct {
}

func (s *ServeMux) handleHandler(h handler, w http.ResponseWriter, r *http.Request, pathParams map[string]string) {
r.Pattern = h.pat.GetPathPattern()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this going to overwrite the Pattern set by net/http? What about users who use r.Pattern today?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This field is only set once by the ServeMux at this line. If grpc-gateway’s ServeMux doesn’t set the r.Pattern field, it will remain unset — nothing else will populate it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic runs after the net/http ServeMux logic that sets the field, so this will overwrite the field, correct? We may thus change what's written to this field for existing users who uses the existing field value for their own logic. That would constitute a breaking change in behavior, no?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently this field is not set, that's why I've even created this PR. Maybe I've missed the thing: could you explain when net/http.ServeMux is called?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are not calling it explicitly, but since our ServeMux type is a http.Handler, we can expect users to use a http.ServeMux with the gateway as a parameter:

mux := http.NewServeMux()
mux.Handle("/api", gwMux)
// Now each API call goes through the http ServeMux and will have the `r.Pattern` populated.

In this case, this change will break any users who is using the pattern today, I think? Could you test this?

h.h(w, r.WithContext(withHTTPPattern(r.Context(), h.pat)), pathParams)
}

Expand Down
32 changes: 32 additions & 0 deletions runtime/pattern.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,38 @@ func (p Pattern) String() string {
return "/" + segs
}

// GetPathPattern generates path pattern from the Pattern.
// It does mostly the same as Pattern.String except it excludes
// utilities.OpPush and utilities.OpPushM operands from the result.
func (p Pattern) GetPathPattern() string {
var stack []string
for _, op := range p.ops {
switch op.code {
case utilities.OpNop:
continue
case utilities.OpPush:
stack = append(stack, "*")
case utilities.OpLitPush:
stack = append(stack, p.pool[op.operand])
case utilities.OpPushM:
stack = append(stack, "**")
case utilities.OpConcatN:
n := op.operand
l := len(stack) - n
stack = append(stack[:l], strings.Join(stack[l:], "/"))
case utilities.OpCapture:
n := len(stack) - 1
// just leave the operand and skip everything else
stack[n] = fmt.Sprintf("{%s}", p.vars[op.operand])
}
}
segs := strings.Join(stack, "/")
if p.verb != "" {
return fmt.Sprintf("/%s:%s", segs, p.verb)
}
return "/" + segs
}

/*
* The following code is adopted and modified from Go's standard library
* and carries the attached license.
Expand Down
85 changes: 85 additions & 0 deletions runtime/pattern_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -634,3 +634,88 @@ func TestPatternString(t *testing.T) {
}
}
}

func TestGetPathPattern(t *testing.T) {
for _, spec := range []struct {
ops []int
pool []string
verb string

want string
}{
{
want: "/",
},
{
ops: []int{int(utilities.OpNop), anything},
want: "/",
},
{
ops: []int{int(utilities.OpPush), anything},
want: "/*",
},
{
ops: []int{int(utilities.OpLitPush), 0},
pool: []string{"endpoint"},
want: "/endpoint",
},
{
ops: []int{int(utilities.OpPushM), anything},
want: "/**",
},
{
ops: []int{
int(utilities.OpPush), anything,
int(utilities.OpConcatN), 1,
},
want: "/*",
},
{
ops: []int{
int(utilities.OpPush), anything,
int(utilities.OpConcatN), 1,
int(utilities.OpCapture), 0,
},
pool: []string{"name"},
want: "/{name}",
},
{
ops: []int{
int(utilities.OpLitPush), 0,
int(utilities.OpLitPush), 1,
int(utilities.OpPush), anything,
int(utilities.OpConcatN), 2,
int(utilities.OpCapture), 2,
int(utilities.OpLitPush), 3,
int(utilities.OpPushM), anything,
int(utilities.OpLitPush), 4,
int(utilities.OpConcatN), 3,
int(utilities.OpCapture), 6,
int(utilities.OpLitPush), 5,
},
pool: []string{"v1", "buckets", "bucket_name", "objects", ".ext", "tail", "name"},
want: "/v1/{bucket_name}/{name}/tail",
},
{
ops: []int{
int(utilities.OpLitPush), 0,
int(utilities.OpLitPush), 1,
int(utilities.OpPush), anything,
int(utilities.OpConcatN), 2,
int(utilities.OpCapture), 2,
},
pool: []string{"v1", "bucket", "name"},
verb: "LOCK",
want: "/v1/{name}:LOCK",
},
} {
p, err := NewPattern(validVersion, spec.ops, spec.pool, spec.verb)
if err != nil {
t.Errorf("NewPattern(%d, %v, %q, %q) failed with %v; want success", validVersion, spec.ops, spec.pool, spec.verb, err)
continue
}
if got, want := p.GetPathPattern(), spec.want; got != want {
t.Errorf("%#v.GetPathPattern() = %q; want %q", p, got, want)
}
}
}