]> Cypherpunks.ru repositories - gostls13.git/commitdiff
net/http: add GODEBUG setting for old ServeMux behavior
authorJonathan Amsterdam <jba@google.com>
Sat, 23 Sep 2023 21:05:42 +0000 (17:05 -0400)
committerJonathan Amsterdam <jba@google.com>
Mon, 2 Oct 2023 20:28:30 +0000 (20:28 +0000)
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 <jba@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
doc/godebug.md
src/internal/godebugs/table.go
src/net/http/request_test.go
src/net/http/servemux121.go [new file with mode: 0644]
src/net/http/server.go
src/net/http/server_test.go
src/runtime/metrics/doc.go

index f35abe11042635fb73b3edc713e6ad442c5e3b12..d578e740be33c41d62bd708492c55da61d0f0e83 100644 (file)
@@ -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,
index 09d5616c9d660d7986f13b0f82083c9981e537e9..2f6d7133630f023910db8a97b478293623e8d201 100644 (file)
@@ -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"},
index 835db91a1a33cb08a0497e956da2861e0ddf97d1..1531da3d8ca60adc0c9fe469064106c83a83a101 100644 (file)
@@ -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 (file)
index 0000000..c0a4b77
--- /dev/null
@@ -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
+}
index ee02d776ac5fc5e5e011bfcd16cf46e052ee5c9d..f456e43cce5b61e9c7d4ddcc88eab38a391439a4 100644 (file)
@@ -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))
 }
 
index a96d87656e757b2c62a16c383b15ba4f2ac1fd35..d418573452ded85832be225079eea007ee4cd4d4 100644 (file)
@@ -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")
index bf7c96f8b5972cd9336e6edc0f0729078198446a..8d79df622d7ab39fb8c2a1018a74a1ff94d39546 100644 (file)
@@ -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.