wee bug in http/server.go?

[Dave Miller]

the following looks suspicious:

func (mux *ServeMux) Handle(pattern string, handler Handler) {

// because here we ensure that len(pattern) is not zero:

if pattern == "" || pattern[0] != '/' {
panicln("http: invalid pattern", pattern)
}

mux.m[pattern] = handler

// Helpful behavior:
// If pattern is /tree/, insert permanent redirect for /tree.
n := len(pattern)

// and then here we test len(pattern) > 0

if n > 0 /*** test will always pass ***/ && pattern[n-1] == '/' {
mux.m[pattern[0:n-1]] = RedirectHandler(pattern,
StatusMovedPermanently)
}
}

It seems that this would register either "/" or "//" as a redirect for
"".

I think the test should panic on "//" and redirect on /x/ for x !=
"" (n > 2)

Dave

[Russ Cox]

> It seems that this would register either "/" or "//" as a redirect for
> "".

it redirects "" to "/", not "/" to "".

as for "//", the caller is trusted not to
pass in non-canonical paths.

russ

[Dave Miller]

The test package pasted below reveals the behavior
of http.ServerMux.Handle(pattern, handler):

this maps these redirects these
pattern to handler to serve MovedPermanently
/ / ""
// // "", /
/x //, /x "", /
/x/ //, /x/ "", /, /x

Is that exactly what you'd expect?

I think the redirection of "/" is bogus.
If so, the behavior is probably harmless, but the wee bug is real.

I also think panicing on "", "x" and "x/" is inappropriate, since it
requires the implementer to test on a condition that is already
being performed by ServeMux.Handler.

The ", ok" pattern might be more appropriate here.

Regards,
-dave

// test of http.ServeMux.Handle(pattern string, handler)
package main

import "http"
import "fmt"

func f(*http.Conn, *http.Request) {}

var sp = http.NewServeMux()

func try(s string) {
h := http.HandlerFunc(f)
fmt.Printf("h: %v\n", h)
sp.Handle(s, h)
fmt.Printf("%s yields %v\n", s, sp)
}

func main () {
// these panic
// try("")
// try("x")
// try("x/")
try("/")
try("//")
try("/x")
try("/x/")
}

[Russ Cox]

/x/ causes /x to redirect to /x/ which is correct behavior.
/ causes "" to redirect to / which is harmless behavior.

Russ