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.

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

Subrouters implement http.Handler but don't do anything useful. #41

Open
hydrogen18 opened this issue Nov 6, 2014 · 3 comments
Open

Comments

@hydrogen18
Copy link

Today I accidentally passed a gocraft/web subrouter to the net/http package. This was an accident on my part, but resulted in some strange behavior. It appears that for each request, it was performed twice. I was not even aware a subrouter could be pased to net/http.

This example demonstrates my mistake, but does not replicate the behavior.

// serve_subrouter.go
package main

import "net/http"
import "github.com/gocraft/web"
import "sync/atomic"
import "fmt"

var numcall uint32

type Context struct{}

func printNumCalls(rw web.ResponseWriter, req *web.Request, next web.NextMiddlewareFunc) {
    count := atomic.AddUint32(&numcall, 1)
    fmt.Printf("Call #%d\n", count)
    next(rw, req)
}

func helloHttp(rw web.ResponseWriter, req *web.Request) {
    rw.WriteHeader(http.StatusOK)
    fmt.Fprint(rw, "Hello HTTP\n")
}
func main() {
    router := web.New(Context{})
    subrouter := router.Subrouter(Context{}, "/foo")
    subrouter.Middleware(printNumCalls)
    subrouter.Get("/bar", helloHttp)

    http.ListenAndServe(":8080", subrouter) //use subrouter by accident
}

As you can see, subrouter can be used as it implements http.Handler. However, all I got was a panic when I used curl to get "http://localhost:8080/foo/bar".

ericu@ericu-desktop:~$ go run ./serve_subrouter.go 
2014/11/05 22:18:28 http: panic serving [::1]:51597: runtime error: makeslice: cap out of range
goroutine 20 [running]:
net/http.func·011()
    /home/ericu/builds/go/src/pkg/net/http/server.go:1100 +0xb7
runtime.panic(0x6cb260, 0x8bc0fc)
    /home/ericu/builds/go/src/pkg/runtime/panic.c:248 +0x18d
github.com/gocraft/web.(*Router).ServeHTTP(0xc208058180, 0x7fe673d358c8, 0xc20803cbe0, 0xc208029040)
    /home/ericu/gotmp/src/github.com/gocraft/web/router_serve.go:32 +0x82
net/http.serverHandler.ServeHTTP(0xc208004180, 0x7fe673d358c8, 0xc20803cbe0, 0xc208029040)
    /home/ericu/builds/go/src/pkg/net/http/server.go:1673 +0x19f
net/http.(*conn).serve(0xc20803e200)
    /home/ericu/builds/go/src/pkg/net/http/server.go:1174 +0xa7e
created by net/http.(*Server).Serve
    /home/ericu/builds/go/src/pkg/net/http/server.go:1721 +0x313
^Cexit status 2
ericu@ericu-desktop:~$ go run ./serve_subrouter.go 
2014/11/05 22:18:42 http: panic serving [::1]:51598: runtime error: makeslice: cap out of range
goroutine 20 [running]:
net/http.func·011()
    /home/ericu/builds/go/src/pkg/net/http/server.go:1100 +0xb7
runtime.panic(0x6cb260, 0x8bc0fc)
    /home/ericu/builds/go/src/pkg/runtime/panic.c:248 +0x18d
github.com/gocraft/web.(*Router).ServeHTTP(0xc208056180, 0x7f74ab7559c8, 0xc20803cbe0, 0xc208029450)
    /home/ericu/gotmp/src/github.com/gocraft/web/router_serve.go:32 +0x82
net/http.serverHandler.ServeHTTP(0xc208004180, 0x7f74ab7559c8, 0xc20803cbe0, 0xc208029450)
    /home/ericu/builds/go/src/pkg/net/http/server.go:1673 +0x19f
net/http.(*conn).serve(0xc20803e200)
    /home/ericu/builds/go/src/pkg/net/http/server.go:1174 +0xa7e
created by net/http.(*Server).Serve
    /home/ericu/builds/go/src/pkg/net/http/server.go:1721 +0x313

The obvious solution is not to pass subrouter to net/http. But it seems this mistake could be avoided by making the subrouter not implement the http.Handler type. This would generate a very obvious compile time error. If I come up with some patches to do this, would you be willing to accept them?

@hydrogen18 hydrogen18 changed the title Subrouters implement http.Handler but don't do anything useful. Subrouters implement http.Handler but doesn't do anything useful. Nov 6, 2014
@hydrogen18 hydrogen18 changed the title Subrouters implement http.Handler but doesn't do anything useful. Subrouters implement http.Handler but don't do anything useful. Nov 14, 2014
@cypriss
Copy link
Member

cypriss commented May 18, 2015

@hydrogen18 Sorry for the delay in responding. I might be willing to accept them, but I worry that fixing it will introduce more complexity than it solves.

@elimisteve
Copy link

I've run into this a couple times. Thank you @hydrogen18 for spotting the issue -- I created a subrouter with the name of the original router, renamed the original router to something else, in which case I ended up passing the subrouter to http.ListenAndServe. Oops!

@hydrogen18
Copy link
Author

I keep meaning to propose patches to work around this, but I've been too busy. Glad to know I'm not the only one @elimisteve

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants