Set the pattern to the http.Request #5697
Conversation
| @@ -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() | |||
There was a problem hiding this comment.
Isn't this going to overwrite the Pattern set by net/http? What about users who use r.Pattern today?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
| @@ -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() | |||
There was a problem hiding this comment.
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?
runtime/pattern.go
Outdated
| "google.golang.org/grpc/grpclog" | ||
|
|
||
| "github.com/grpc-ecosystem/grpc-gateway/v2/utilities" |
There was a problem hiding this comment.
Revert this change please, we use two import groups
References to other Issues or PRs
The PR is related to Issue #3700
Have you read the Contributing Guidelines?
Yes
Brief description of what is fixed or changed
Now
http.Requesthas thePatternfield set.httpServeMuxdoes this and other libraries make use of this value. Nowruntime.ServeMuxalso populates this field.Other comments
A bit more context in the comment