Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

mux: fix path components mutation #3001

Merged
merged 2 commits into from Nov 8, 2022
Merged

mux: fix path components mutation #3001

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bfc7e0c
Add verb test case for mux
cclien0725 Nov 7, 2022
c0c13cd
avoid mutating path components inside for loop
jonathaningram Nov 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
48 changes: 28 additions & 20 deletions runtime/mux.go
View file
Open in desktop
Expand Up @@ -320,17 +320,6 @@ func (s *ServeMux) ServeHTTP(w http.ResponseWriter, r *http.Request) {
path = r.URL.RawPath
}

var components []string
// since in UnescapeModeLegacy, the URL will already have been fully unescaped, if we also split on "%2F"
// in this escaping mode we would be double unescaping but in UnescapingModeAllCharacters, we still do as the
// path is the RawPath (i.e. unescaped). That does mean that the behavior of this function will change its default
// behavior when the UnescapingModeDefault gets changed from UnescapingModeLegacy to UnescapingModeAllExceptReserved
if s.unescapingMode == UnescapingModeAllCharacters {
components = encodedPathSplitter.Split(path[1:], -1)
} else {
components = strings.Split(path[1:], "/")
}

if override := r.Header.Get("X-HTTP-Method-Override"); override != "" && s.isPathLengthFallback(r) {
r.Method = strings.ToUpper(override)
if err := r.ParseForm(); err != nil {
Expand All @@ -341,7 +330,18 @@ func (s *ServeMux) ServeHTTP(w http.ResponseWriter, r *http.Request) {
}
}

lastComponent := components[len(components)-1]
var pathComponents []string
// since in UnescapeModeLegacy, the URL will already have been fully unescaped, if we also split on "%2F"
// in this escaping mode we would be double unescaping but in UnescapingModeAllCharacters, we still do as the
// path is the RawPath (i.e. unescaped). That does mean that the behavior of this function will change its default
// behavior when the UnescapingModeDefault gets changed from UnescapingModeLegacy to UnescapingModeAllExceptReserved
if s.unescapingMode == UnescapingModeAllCharacters {
pathComponents = encodedPathSplitter.Split(path[1:], -1)
} else {
pathComponents = strings.Split(path[1:], "/")
}

lastPathComponent := pathComponents[len(pathComponents)-1]

for _, h := range s.handlers[r.Method] {
// If the pattern has a verb, explicitly look for a suffix in the last
Expand All @@ -357,19 +357,23 @@ func (s *ServeMux) ServeHTTP(w http.ResponseWriter, r *http.Request) {
patVerb := h.pat.Verb()

idx := -1
if patVerb != "" && strings.HasSuffix(lastComponent, ":"+patVerb) {
idx = len(lastComponent) - len(patVerb) - 1
if patVerb != "" && strings.HasSuffix(lastPathComponent, ":"+patVerb) {
idx = len(lastPathComponent) - len(patVerb) - 1
}
if idx == 0 {
_, outboundMarshaler := MarshalerForRequest(s, r)
s.routingErrorHandler(ctx, s, outboundMarshaler, w, r, http.StatusNotFound)
return
}

comps := make([]string, len(pathComponents))
copy(comps, pathComponents)

if idx > 0 {
components[len(components)-1], verb = lastComponent[:idx], lastComponent[idx+1:]
comps[len(comps)-1], verb = lastPathComponent[:idx], lastPathComponent[idx+1:]
}

pathParams, err := h.pat.MatchAndEscape(components, verb, s.unescapingMode)
pathParams, err := h.pat.MatchAndEscape(comps, verb, s.unescapingMode)
if err != nil {
var mse MalformedSequenceError
if ok := errors.As(err, &mse); ok {
Expand All @@ -396,14 +400,18 @@ func (s *ServeMux) ServeHTTP(w http.ResponseWriter, r *http.Request) {
patVerb := h.pat.Verb()

idx := -1
if patVerb != "" && strings.HasSuffix(lastComponent, ":"+patVerb) {
idx = len(lastComponent) - len(patVerb) - 1
if patVerb != "" && strings.HasSuffix(lastPathComponent, ":"+patVerb) {
idx = len(lastPathComponent) - len(patVerb) - 1
}

comps := make([]string, len(pathComponents))
copy(comps, pathComponents)

if idx > 0 {
components[len(components)-1], verb = lastComponent[:idx], lastComponent[idx+1:]
comps[len(comps)-1], verb = lastPathComponent[:idx], lastPathComponent[idx+1:]
}

pathParams, err := h.pat.MatchAndEscape(components, verb, s.unescapingMode)
pathParams, err := h.pat.MatchAndEscape(comps, verb, s.unescapingMode)
if err != nil {
var mse MalformedSequenceError
if ok := errors.As(err, &mse); ok {
Expand Down
42 changes: 42 additions & 0 deletions runtime/mux_test.go
View file
Open in desktop
Expand Up @@ -478,6 +478,48 @@ func TestMuxServeHTTP(t *testing.T) {
unescapingMode: runtime.UnescapingModeAllCharacters,
respContent: "POST /api/v1/{name=organizations/*}:action",
},
{
patterns: []stubPattern{
{
method: "POST",
ops: []int{
int(utilities.OpLitPush), 0,
int(utilities.OpLitPush), 1,
int(utilities.OpLitPush), 2,
},
pool: []string{"api", "v1", "organizations"},
verb: "verb",
},
{
method: "POST",
ops: []int{
int(utilities.OpLitPush), 0,
int(utilities.OpLitPush), 1,
int(utilities.OpLitPush), 2,
},
pool: []string{"api", "v1", "organizations"},
verb: "",
},
{
method: "POST",
ops: []int{
int(utilities.OpLitPush), 0,
int(utilities.OpLitPush), 1,
int(utilities.OpLitPush), 2,
},
pool: []string{"api", "v1", "dummies"},
verb: "verb",
},
},
reqMethod: "POST",
reqPath: "/api/v1/organizations:verb",
headers: map[string]string{
"Content-Type": "application/json",
},
respStatus: http.StatusOK,
unescapingMode: runtime.UnescapingModeAllCharacters,
respContent: "POST /api/v1/organizations:verb",
},
} {
t.Run(strconv.Itoa(i), func(t *testing.T) {
var opts []runtime.ServeMuxOption
Expand Down