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

net/http: add example for ServeMux to make "/.*" behavior clear #4799

Closed
shields opened this issue Feb 13, 2013 · 11 comments
Closed

net/http: add example for ServeMux to make "/.*" behavior clear #4799

shields opened this issue Feb 13, 2013 · 11 comments

Comments

@shields
Copy link
Contributor

shields commented Feb 13, 2013

As discussed on golang-nuts:

I expected this code to serve pages at http://localhost:8080/ and
http://localhost:8080/foo, with 404s for any other path.

        http.HandleFunc("/", indexHandler)
        http.HandleFunc("/foo", fooHandler)
        log.Fatal(http.ListenAndServe(":8080", nil))

Surprisingly, /bar is also valid.  It gets indexHandler, because as "/subdir/"
includes an entire subtree, "/" includes the entire tree. This means that
indexHandler must test req.URL.Path == "/" and explicitly 404 any other paths.

Being able to serve all endpoints easily is useful, but I think in many cases people are
doing it unintentionally, i.e., writing bugs.

This is only surprising for "/", since the documentation is clear about how
"/foo/" works.

Possible resolutions:


1. I suggested allowing "" to match the root path only.  "" is
currently an invalid pattern, so this is a backwards-compatible change.


2. adg suggested a wrapper:

        func ExactPath(path string, http.Handler) http.Handler

Using this involves some stutter:

        mux.Handle("/", http.ExactPath("/", h))

This seems more general in that you could also use it for non-index paths such as
"/a/b/", but there would still be a questionable redirect from
"/a/b".



3. Remove the stutter and generality:

        mux.Handle("/", http.IndexOnly(indexHandler))


4. Exact registration, which would work for non-index paths:

        mux.HandleExact("/", indexHandler)


5. We could just warn people to check req.URL.Path in their index handlers.
@bradfitz
Copy link
Contributor

Comment 1:

One answer which you might not like:
The ServeMux isn't fundamental to the net/http package.  If you don't like its behavior,
you can write your own mux.  Several people in the community have.  I often do my own
routing when a ServeMux is over- or under-kill.

@gopherbot
Copy link

Comment 2 by alec@swapoff.org:

What an odd coincidence... I also hit this today. It was quite confusing.
Writing your own mux (or using an existing one like gorilla) is a workaround, but given
the corresponding loss of integration with other stdlib packages like expvar and
net/http/pprof, not a great one.

@shields
Copy link
Contributor Author

shields commented Feb 13, 2013

Comment 3:

Sure, working around this is straightforward.  Even simpler is:
func indexHandler(w http.ResponseWriter, r *http.Request) {
    if r.URL.Path != "/" {
        http.NotFound(w, r)
        return
    }
        ....
}
What concerns me is that serving that page at all endpoints is a bug
that was latent in my program.  I'd like to help others not have that
bug.
I still like the idea of "/" including all paths and "" including the
root only, similar to the trailing slash in "/foo/" vs. "/foo".  The
other suggestions expand the API more than I think is justified.

@rsc
Copy link
Contributor

rsc commented Feb 13, 2013

Comment 4:

I am not sure this is worth fixing. If it is worth fixing, then "" is the
answer. http.ExactPath is overkill: the only thing it would be good for is
this one situation.

@rsc
Copy link
Contributor

rsc commented Mar 12, 2013

Comment 6:

[The time for maybe has passed.]

Labels changed: removed go1.1maybe.

@adg
Copy link
Contributor

adg commented Mar 22, 2013

Comment 7:

My opinion is that "" should be a bug.
"/" is a good pedagogical case. A lot of people don't know that "/foo/" matches
"/foo/.*".
Perhaps the answer is more examples?

Labels changed: added priority-later, removed priority-triage.

Status changed to Accepted.

@shields
Copy link
Contributor Author

shields commented Mar 22, 2013

Comment 8:

I don't think it is a good idea to make it easy to serve the same page at all paths,
even though it does seem clean when you understand the mechanism.  Let me give some
examples of how serving the same content for every page is harmful.
1. You have a load balancer that health-checks a number of backends and forwards queries
to the ones that are serving 200s.  By mistake, you include a machine in the pool that
is serving a different service.  Everything appears to work, but you are unknowingly
serving the wrong content to some fraction of users.  The fix is to health-check not /
but /healthy-for-foo; 404s from other backends will stop them from seeing user traffic,
and your monitoring will pick up the unhealthiness of the incorrectly configured
backends.
2.  becomes a valid link, not to http://www.foo.bar/path as
you intended but to http://www.your.site/www.foo.bar/path.  If you crawl your site
checking broken links, this won't be noticed.  No errors will appear in your logs.
3. Trolls can construct valid URLs to your site with "creative" paths.  They can send
links to these paths to other users, and they can get search engines to crawl and index
them.
Therefore I think it should be the best practice to serve 200 for / and 404 for
everything else.
I think "" is the easiest API to use, but if you prefer, we could also document that
everyone using ServeMux should include boilerplate on their / handler to serve only /.

@adg
Copy link
Contributor

adg commented Mar 23, 2013

Comment 9:

You make good points. Whether it is a good idea or not is irrelevant now. The behaviour
of Handle("/", too) is set in stone for Go 1.x.  
This issue is about the possibility of changing the meaning of the "" path, and any
documentation changes. I think a good example on Handle could clear up a lot of
confusion.

@rsc
Copy link
Contributor

rsc commented Jul 30, 2013

Comment 10:

Labels changed: added go1.2maybe.

@rsc
Copy link
Contributor

rsc commented Sep 10, 2013

Comment 11:

Labels changed: added documentation.

@bradfitz
Copy link
Contributor

Comment 12:

This issue was closed by revision 1a819be.

Status changed to Fixed.

@rsc rsc added this to the Go1.2 milestone Apr 14, 2015
@rsc rsc removed the go1.2maybe label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants