net/http: panic on duplicate registrations

Otherwise, the registration semantics are
init-order-dependent, which I was trying very hard
to avoid in the API.  This may break broken programs.

Fixes #2900.

R=golang-dev, r, bradfitz, dsymonds, balasanjay, kevlar
CC=golang-dev
https://golang.org/cl/5644051

diff --git a/doc/go1.html b/doc/go1.html
index 4191c4ba25d4927cf18a569962fbbf2027bade10..e3d2354e64e782b1b3e837753f5ce6498e896633 100644 (file)
--- a/doc/go1.html
+++ b/doc/go1.html
@@ -1146,10 +1146,16 @@ The affected items are:
 </ul>
 
 <p>
-Also, the <code>Request.RawURL</code> field has been removed; it was a
+The <code>Request.RawURL</code> field has been removed; it was a
 historical artifact.
 </p>
 
+<p>
+The <code>Handle</code> and <code>HandleFunc</code>
+functions, and the similarly-named methods of <code>ServeMux</code>,
+now panic if an attempt is made to register the same pattern twice.
+</p>
+
 <p>
 <em>Updating</em>:
 Running <code>go fix</code> will update the few programs that are affected except for

diff --git a/doc/go1.tmpl b/doc/go1.tmpl
index 819c71ed38850f082e42513365f9ab094e08ef20..8f276827807595fc936989599000d406f4143990 100644 (file)
--- a/doc/go1.tmpl
+++ b/doc/go1.tmpl
@@ -1049,10 +1049,16 @@ The affected items are:
 </ul>
 
 <p>
-Also, the <code>Request.RawURL</code> field has been removed; it was a
+The <code>Request.RawURL</code> field has been removed; it was a
 historical artifact.
 </p>
 
+<p>
+The <code>Handle</code> and <code>HandleFunc</code>
+functions, and the similarly-named methods of <code>ServeMux</code>,
+now panic if an attempt is made to register the same pattern twice.
+</p>
+
 <p>
 <em>Updating</em>:
 Running <code>go fix</code> will update the few programs that are affected except for

diff --git a/src/pkg/net/http/server.go b/src/pkg/net/http/server.go
index 288539ba5760745a5f0b50dc45905a57675acb2b..8c4822ec748f4b906b3dda3bcbd6df14c9a64188 100644 (file)
--- a/src/pkg/net/http/server.go
+++ b/src/pkg/net/http/server.go
@@ -833,11 +833,17 @@ func RedirectHandler(url string, code int) Handler {
 // redirecting any request containing . or .. elements to an
 // equivalent .- and ..-free URL.
 type ServeMux struct {
-       m map[string]Handler
+       mu sync.RWMutex
+       m  map[string]muxEntry
+}
+
+type muxEntry struct {
+       explicit bool
+       h        Handler
 }
 
 // NewServeMux allocates and returns a new ServeMux.
-func NewServeMux() *ServeMux { return &ServeMux{make(map[string]Handler)} }
+func NewServeMux() *ServeMux { return &ServeMux{m: make(map[string]muxEntry)} }
 
 // DefaultServeMux is the default ServeMux used by Serve.
 var DefaultServeMux = NewServeMux()
@@ -883,12 +889,28 @@ func (mux *ServeMux) match(path string) Handler {
                }
                if h == nil || len(k) > n {
                        n = len(k)
-                       h = v
+                       h = v.h
                }
        }
        return h
 }
 
+// handler returns the handler to use for the request r.
+func (mux *ServeMux) handler(r *Request) Handler {
+       mux.mu.RLock()
+       defer mux.mu.RUnlock()
+
+       // Host-specific pattern takes precedence over generic ones
+       h := mux.match(r.Host + r.URL.Path)
+       if h == nil {
+               h = mux.match(r.URL.Path)
+       }
+       if h == nil {
+               h = NotFoundHandler()
+       }
+       return h
+}
+
 // ServeHTTP dispatches the request to the handler whose
 // pattern most closely matches the request URL.
 func (mux *ServeMux) ServeHTTP(w ResponseWriter, r *Request) {
@@ -898,30 +920,33 @@ func (mux *ServeMux) ServeHTTP(w ResponseWriter, r *Request) {
                w.WriteHeader(StatusMovedPermanently)
                return
        }
-       // Host-specific pattern takes precedence over generic ones
-       h := mux.match(r.Host + r.URL.Path)
-       if h == nil {
-               h = mux.match(r.URL.Path)
-       }
-       if h == nil {
-               h = NotFoundHandler()
-       }
-       h.ServeHTTP(w, r)
+       mux.handler(r).ServeHTTP(w, r)
 }
 
 // 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) {
+       mux.mu.Lock()
+       defer mux.mu.Unlock()
+
        if pattern == "" {
                panic("http: invalid pattern " + pattern)
        }
+       if handler == nil {
+               panic("http: nil handler")
+       }
+       if mux.m[pattern].explicit {
+               panic("http: multiple registrations for " + pattern)
+       }
 
-       mux.m[pattern] = handler
+       mux.m[pattern] = muxEntry{explicit: true, h: handler}
 
        // Helpful behavior:
-       // If pattern is /tree/, insert permanent redirect for /tree.
+       // If pattern is /tree/, insert an implicit permanent redirect for /tree.
+       // It can be overridden by an explicit registration.
        n := len(pattern)
-       if n > 0 && pattern[n-1] == '/' {
-               mux.m[pattern[0:n-1]] = RedirectHandler(pattern, StatusMovedPermanently)
+       if n > 0 && pattern[n-1] == '/' && !mux.m[pattern[0:n-1]].explicit {
+               mux.m[pattern[0:n-1]] = muxEntry{h: RedirectHandler(pattern, StatusMovedPermanently)}
        }
 }