From: Jonathan Amsterdam Date: Sat, 23 Sep 2023 21:05:42 +0000 (-0400) Subject: net/http: add GODEBUG setting for old ServeMux behavior X-Git-Tag: go1.22rc1~712 X-Git-Url: http://www.git.cypherpunks.ru/?a=commitdiff_plain;h=eb070d7483f5d206008aa05921652e595b8425f2;p=gostls13.git net/http: add GODEBUG setting for old ServeMux behavior Add the GODEBUG setting httpmuxgo121. When set to "1", ServeMux behaves exactly like it did in Go 1.21. Implemented by defining a new, unexported type, serveMux121, that uses the original code. Updates #61410. Change-Id: I0a9d0fe2a2286e442d680393e62895ab50683cea Reviewed-on: https://go-review.googlesource.com/c/go/+/530461 Run-TryBot: Jonathan Amsterdam TryBot-Result: Gopher Robot Reviewed-by: Damien Neil --- diff --git a/doc/godebug.md b/doc/godebug.md index f35abe1104..d578e740be 100644 --- a/doc/godebug.md +++ b/doc/godebug.md @@ -138,6 +138,12 @@ Go 1.22 made it an error for a request or response read by a net/http client or server to have an empty Content-Length header. This behavior is controlled by the `httplaxcontentlength` setting. +Go 1.22 changed the behavior of ServeMux to accept extended +patterns and unescape both patterns and request paths by segment. +This behavior can be controlled by the +[`httpmuxgo121` setting](/pkg/net/http/#ServeMux). + + ### Go 1.21 Go 1.21 made it a run-time error to call `panic` with a nil interface value, diff --git a/src/internal/godebugs/table.go b/src/internal/godebugs/table.go index 09d5616c9d..2f6d713363 100644 --- a/src/internal/godebugs/table.go +++ b/src/internal/godebugs/table.go @@ -33,6 +33,7 @@ var All = []Info{ {Name: "http2debug", Package: "net/http", Opaque: true}, {Name: "http2server", Package: "net/http"}, {Name: "httplaxcontentlength", Package: "net/http", Changed: 22, Old: "1"}, + {Name: "httpmuxgo121", Package: "net/http", Changed: 22, Old: "1"}, {Name: "installgoroot", Package: "go/build"}, {Name: "jstmpllitinterp", Package: "html/template"}, //{Name: "multipartfiles", Package: "mime/multipart"}, diff --git a/src/net/http/request_test.go b/src/net/http/request_test.go index 835db91a1a..1531da3d8c 100644 --- a/src/net/http/request_test.go +++ b/src/net/http/request_test.go @@ -1554,54 +1554,3 @@ func TestStatus(t *testing.T) { } } } - -func TestEscapedPathsAndPatterns(t *testing.T) { - matches := []struct { - pattern string - paths []string - }{ - { - "/a", // this pattern matches a path that unescapes to "/a" - []string{"/a", "/%61"}, - }, - { - "/%62", // patterns are unescaped by segment; matches paths that unescape to "/b" - []string{"/b", "/%62"}, - }, - { - "/%7B/%7D", // the only way to write a pattern that matches '{' or '}' - []string{"/{/}", "/%7b/}", "/{/%7d", "/%7B/%7D"}, - }, - { - "/%x", // patterns that do not unescape are left unchanged - []string{"/%25x"}, - }, - } - - mux := NewServeMux() - var gotPattern string - for _, m := range matches { - mux.HandleFunc(m.pattern, func(w ResponseWriter, r *Request) { - gotPattern = m.pattern - }) - } - - server := httptest.NewServer(mux) - defer server.Close() - - for _, m := range matches { - for _, p := range m.paths { - res, err := Get(server.URL + p) - if err != nil { - t.Fatal(err) - } - if res.StatusCode != 200 { - t.Errorf("%s: got code %d, want 200", p, res.StatusCode) - continue - } - if g, w := gotPattern, m.pattern; g != w { - t.Errorf("%s: pattern: got %q, want %q", p, g, w) - } - } - } -} diff --git a/src/net/http/servemux121.go b/src/net/http/servemux121.go new file mode 100644 index 0000000000..c0a4b77010 --- /dev/null +++ b/src/net/http/servemux121.go @@ -0,0 +1,211 @@ +// Copyright 2009 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package http + +// This file implements ServeMux behavior as in Go 1.21. +// The behavior is controlled by a GODEBUG setting. +// Most of this code is derived from commit 08e35cc334. +// Changes are minimal: aside from the different receiver type, +// they mostly involve renaming functions, usually by unexporting them. + +import ( + "internal/godebug" + "net/url" + "sort" + "strings" + "sync" +) + +var httpmuxgo121 = godebug.New("httpmuxgo121") + +var use121 bool + +// Read httpmuxgo121 once at startup, since dealing with changes to it during +// program execution is too complex and error-prone. +func init() { + if httpmuxgo121.Value() == "1" { + use121 = true + httpmuxgo121.IncNonDefault() + } +} + +// serveMux121 holds the state of a ServeMux needed for Go 1.21 behavior. +type serveMux121 struct { + mu sync.RWMutex + m map[string]muxEntry + es []muxEntry // slice of entries sorted from longest to shortest. + hosts bool // whether any patterns contain hostnames +} + +type muxEntry struct { + h Handler + pattern string +} + +// Formerly ServeMux.Handle. +func (mux *serveMux121) handle(pattern string, handler Handler) { + mux.mu.Lock() + defer mux.mu.Unlock() + + if pattern == "" { + panic("http: invalid pattern") + } + if handler == nil { + panic("http: nil handler") + } + if _, exist := mux.m[pattern]; exist { + panic("http: multiple registrations for " + pattern) + } + + if mux.m == nil { + mux.m = make(map[string]muxEntry) + } + e := muxEntry{h: handler, pattern: pattern} + mux.m[pattern] = e + if pattern[len(pattern)-1] == '/' { + mux.es = appendSorted(mux.es, e) + } + + if pattern[0] != '/' { + mux.hosts = true + } +} + +func appendSorted(es []muxEntry, e muxEntry) []muxEntry { + n := len(es) + i := sort.Search(n, func(i int) bool { + return len(es[i].pattern) < len(e.pattern) + }) + if i == n { + return append(es, e) + } + // we now know that i points at where we want to insert + es = append(es, muxEntry{}) // try to grow the slice in place, any entry works. + copy(es[i+1:], es[i:]) // Move shorter entries down + es[i] = e + return es +} + +// Formerly ServeMux.HandleFunc. +func (mux *serveMux121) handleFunc(pattern string, handler func(ResponseWriter, *Request)) { + if handler == nil { + panic("http: nil handler") + } + mux.handle(pattern, HandlerFunc(handler)) +} + +// Formerly ServeMux.Handler. +func (mux *serveMux121) findHandler(r *Request) (h Handler, pattern string) { + + // CONNECT requests are not canonicalized. + if r.Method == "CONNECT" { + // If r.URL.Path is /tree and its handler is not registered, + // the /tree -> /tree/ redirect applies to CONNECT requests + // but the path canonicalization does not. + if u, ok := mux.redirectToPathSlash(r.URL.Host, r.URL.Path, r.URL); ok { + return RedirectHandler(u.String(), StatusMovedPermanently), u.Path + } + + return mux.handler(r.Host, r.URL.Path) + } + + // All other requests have any port stripped and path cleaned + // before passing to mux.handler. + host := stripHostPort(r.Host) + path := cleanPath(r.URL.Path) + + // If the given path is /tree and its handler is not registered, + // redirect for /tree/. + if u, ok := mux.redirectToPathSlash(host, path, r.URL); ok { + return RedirectHandler(u.String(), StatusMovedPermanently), u.Path + } + + if path != r.URL.Path { + _, pattern = mux.handler(host, path) + u := &url.URL{Path: path, RawQuery: r.URL.RawQuery} + return RedirectHandler(u.String(), StatusMovedPermanently), pattern + } + + return mux.handler(host, r.URL.Path) +} + +// handler is the main implementation of findHandler. +// The path is known to be in canonical form, except for CONNECT methods. +func (mux *serveMux121) handler(host, path string) (h Handler, pattern string) { + mux.mu.RLock() + defer mux.mu.RUnlock() + + // Host-specific pattern takes precedence over generic ones + if mux.hosts { + h, pattern = mux.match(host + path) + } + if h == nil { + h, pattern = mux.match(path) + } + if h == nil { + h, pattern = NotFoundHandler(), "" + } + return +} + +// Find a handler on a handler map given a path string. +// Most-specific (longest) pattern wins. +func (mux *serveMux121) match(path string) (h Handler, pattern string) { + // Check for exact match first. + v, ok := mux.m[path] + if ok { + return v.h, v.pattern + } + + // Check for longest valid match. mux.es contains all patterns + // that end in / sorted from longest to shortest. + for _, e := range mux.es { + if strings.HasPrefix(path, e.pattern) { + return e.h, e.pattern + } + } + return nil, "" +} + +// redirectToPathSlash determines if the given path needs appending "/" to it. +// This occurs when a handler for path + "/" was already registered, but +// not for path itself. If the path needs appending to, it creates a new +// URL, setting the path to u.Path + "/" and returning true to indicate so. +func (mux *serveMux121) redirectToPathSlash(host, path string, u *url.URL) (*url.URL, bool) { + mux.mu.RLock() + shouldRedirect := mux.shouldRedirectRLocked(host, path) + mux.mu.RUnlock() + if !shouldRedirect { + return u, false + } + path = path + "/" + u = &url.URL{Path: path, RawQuery: u.RawQuery} + return u, true +} + +// shouldRedirectRLocked reports whether the given path and host should be redirected to +// path+"/". This should happen if a handler is registered for path+"/" but +// not path -- see comments at ServeMux. +func (mux *serveMux121) shouldRedirectRLocked(host, path string) bool { + p := []string{path, host + path} + + for _, c := range p { + if _, exist := mux.m[c]; exist { + return false + } + } + + n := len(path) + if n == 0 { + return false + } + for _, c := range p { + if _, exist := mux.m[c+"/"]; exist { + return path[n-1] != '/' + } + } + + return false +} diff --git a/src/net/http/server.go b/src/net/http/server.go index ee02d776ac..f456e43cce 100644 --- a/src/net/http/server.go +++ b/src/net/http/server.go @@ -33,6 +33,8 @@ import ( "golang.org/x/net/http/httpguts" ) +// TODO(jba): test + // Errors used by the HTTP server. var ( // ErrBodyNotAllowed is returned by ResponseWriter.Write calls @@ -2348,7 +2350,8 @@ type ServeMux struct { mu sync.RWMutex tree routingNode index routingIndex - patterns []*pattern // TODO(jba): remove if possible + patterns []*pattern // TODO(jba): remove if possible + mux121 serveMux121 // used only when GODEBUG=httpmuxgo121=1 } // NewServeMux allocates and returns a new ServeMux. @@ -2412,6 +2415,9 @@ func stripHostPort(h string) string { // If there is no registered handler that applies to the request, // Handler returns a “page not found” handler and an empty pattern. func (mux *ServeMux) Handler(r *Request) (h Handler, pattern string) { + if use121 { + return mux.mux121.findHandler(r) + } h, p, _, _ := mux.findHandler(r) return h, p } @@ -2585,9 +2591,12 @@ func (mux *ServeMux) ServeHTTP(w ResponseWriter, r *Request) { w.WriteHeader(StatusBadRequest) return } - h, _, pat, matches := mux.findHandler(r) - r.pat = pat - r.matches = matches + var h Handler + if use121 { + h, _ = mux.mux121.findHandler(r) + } else { + h, _, r.pat, r.matches = mux.findHandler(r) + } h.ServeHTTP(w, r) } @@ -2597,23 +2606,35 @@ func (mux *ServeMux) ServeHTTP(w ResponseWriter, r *Request) { // Handle registers the handler for the given pattern. // If a handler already exists for pattern, Handle panics. func (mux *ServeMux) Handle(pattern string, handler Handler) { + if use121 { + mux.mux121.handle(pattern, handler) + } mux.register(pattern, handler) } // HandleFunc registers the handler function for the given pattern. func (mux *ServeMux) HandleFunc(pattern string, handler func(ResponseWriter, *Request)) { + if use121 { + mux.mux121.handleFunc(pattern, handler) + } mux.register(pattern, HandlerFunc(handler)) } // Handle registers the handler for the given pattern in [DefaultServeMux]. // The documentation for [ServeMux] explains how patterns are matched. func Handle(pattern string, handler Handler) { + if use121 { + DefaultServeMux.mux121.handle(pattern, handler) + } DefaultServeMux.register(pattern, handler) } // HandleFunc registers the handler function for the given pattern in [DefaultServeMux]. // The documentation for [ServeMux] explains how patterns are matched. func HandleFunc(pattern string, handler func(ResponseWriter, *Request)) { + if use121 { + DefaultServeMux.mux121.handleFunc(pattern, handler) + } DefaultServeMux.register(pattern, HandlerFunc(handler)) } diff --git a/src/net/http/server_test.go b/src/net/http/server_test.go index a96d87656e..d418573452 100644 --- a/src/net/http/server_test.go +++ b/src/net/http/server_test.go @@ -174,6 +174,68 @@ func TestExactMatch(t *testing.T) { } } +func TestEscapedPathsAndPatterns(t *testing.T) { + matches := []struct { + pattern string + paths []string // paths that match the pattern + paths121 []string // paths that matched the pattern in Go 1.21. + }{ + { + "/a", // this pattern matches a path that unescapes to "/a" + []string{"/a", "/%61"}, + []string{"/a", "/%61"}, + }, + { + "/%62", // patterns are unescaped by segment; matches paths that unescape to "/b" + []string{"/b", "/%62"}, + []string{"/%2562"}, // In 1.21, patterns were not unescaped but paths were. + }, + { + "/%7B/%7D", // the only way to write a pattern that matches '{' or '}' + []string{"/{/}", "/%7b/}", "/{/%7d", "/%7B/%7D"}, + []string{"/%257B/%257D"}, // In 1.21, patterns were not unescaped. + }, + { + "/%x", // patterns that do not unescape are left unchanged + []string{"/%25x"}, + []string{"/%25x"}, + }, + } + + run := func(t *testing.T, test121 bool) { + defer func(u bool) { use121 = u }(use121) + use121 = test121 + + mux := NewServeMux() + for _, m := range matches { + mux.HandleFunc(m.pattern, func(w ResponseWriter, r *Request) {}) + } + + for _, m := range matches { + paths := m.paths + if use121 { + paths = m.paths121 + } + for _, p := range paths { + u, err := url.ParseRequestURI(p) + if err != nil { + t.Fatal(err) + } + req := &Request{ + URL: u, + } + _, gotPattern := mux.Handler(req) + if g, w := gotPattern, m.pattern; g != w { + t.Errorf("%s: pattern: got %q, want %q", p, g, w) + } + } + } + } + + t.Run("latest", func(t *testing.T) { run(t, false) }) + t.Run("1.21", func(t *testing.T) { run(t, true) }) +} + func BenchmarkServerMatch(b *testing.B) { fn := func(w ResponseWriter, r *Request) { fmt.Fprintf(w, "OK") diff --git a/src/runtime/metrics/doc.go b/src/runtime/metrics/doc.go index bf7c96f8b5..8d79df622d 100644 --- a/src/runtime/metrics/doc.go +++ b/src/runtime/metrics/doc.go @@ -259,6 +259,10 @@ Below is the full list of supported metrics, ordered lexicographically. package due to a non-default GODEBUG=httplaxcontentlength=... setting. + /godebug/non-default-behavior/httpmuxgo121:events + The number of non-default behaviors executed by the net/http + package due to a non-default GODEBUG=httpmuxgo121=... setting. + /godebug/non-default-behavior/installgoroot:events The number of non-default behaviors executed by the go/build package due to a non-default GODEBUG=installgoroot=... setting.