proposal: net/http/v2: thoughts for Go 2.0

[bradfitz]

Wishlist bug for what net/http would look like if we could make backwards-incompatible
changes.

Anything goes.

I'll start:

* Handler (http://golang.org/pkg/net/http/#Handler) currently takes a pointer to a 208
byte Request struct.  

* The Request struct (http://golang.org/pkg/net/http/#Request) has all its fields
publicly exposed, most of which require memory allocation:

  -- Method
  -- URL (itself 104 bytes, with a bunch of strings requiring allocation)
  -- Header map + slices + strings (even if never accessed)
  -- TransferEncoding slice (even if not accessed)
  -- Host string (even if not accessed)
  -- RemoteAddr string (even if not accessed)
  -- RequestURI (even if not accessed)
  -- TLS state struct (even if not accessed)

For a lightweight handler that doesn't touch anything (say, serves some static content
from memory), this means we can't do any better than generating ~1KB of garbage per
request.

I'd prefer to make a ServerRequest interface with accessor methods which can generate
needed data on demand.

This would also simplify our docs on our doubly-abused-in-different-ways *Request, which
contains documentation gems like:

    // PostForm contains the parsed form data from POST or PUT
    // body parameters.
    // This field is only available after ParseForm is called.
    // The HTTP client ignores PostForm and uses Body instead.
    PostForm url.Values

If we had byte views (issue #5376), I would also say that most fields in the
ServerRequest, URL, Header, and FormValues are all byte views with validity scoped to
the duration of the http.Handler call, instead of strings.

[nigeltao]

Comment 1:

A CookieJar's methods should return error, and maybe be renamed from http.CookieJar to
cookiejar.Interface.

[gopherbot]

Comment 2 by robryk:

In http.Client, there should be a uniform way of specifying request headers on the
initial request and subsequent requests when following a redirect. Current the
Request.Header field is useless if the request might get redirected. See issue #5782.
Maybe something like a func(*Request) field in the request, that gets called on this
request and each redirected one before it gets sent?

[gopherbot]

Comment 3 by jeff.allen:

The fact that headers are parsed into a map is a mega bummer for garbage reduction. If
the main way headers were exposed was more opaque, the headers map could be made up of
offsets into the buffer that came straight from Read.

[rsc]

Comment 4:

Labels changed: added repo-main.

[nigeltao]

Comment 5:

Related to comment #1, net/http/cookiejar.PublicSuffixList's PublicSuffix method should
perhaps return (string, error) instead of (string), in case the PSL implementation is
passed non-canonicalized input such as upper-cased "EXAMPLE.COM.AU". Returning an
explicit error is probably safer than silently returning a "" that would mean a liberal
cookie policy.

[alberts]

Comment 6:

It would be nice to have a handle on when all the goroutines that handle requests have
terminated after closing the listener (thereby breaking out of the accept loop) for
cases where they are using a shared resource that needs to be cleaned up (e.g. an
in-process database that needs to be closed). And maybe a way to force-close all their
connections. Especially useful for tests and SIGTERM situations.

[gopherbot]

Comment 7 by robryk:

You can do that by counting open connections returned by Listener's
Accept: http://play.golang.org/p/sPmvXaR-2M
(Disclaimer: I didn't test that code.)
Robert

[alberts]

Comment 8:

@Robert thanks, I did something similar in the end.

[alberts]

Comment 9:

@Robert For what it's worth, I think your listener breaks in the more general case where
you hand off the socket to both a reader and a writer goroutine. There a "done" channel
might be better.

[gopherbot]

Comment 10 by robryk:

Summarizing a discussion with fullung@: That version didn't handle multiple calls to
Close and had a (likely irrelevant) ReadWrite/Close race condition. The corrected
version is here: http://play.golang.org/p/WTKa021ipJ

[gopherbot]

Comment 11 by robryk:

Summarizing a discussion with fullung@: That version didn't handle multiple calls to
Close and had a (likely irrelevant) ReadWrite/Close race condition. The corrected
version is here: http://play.golang.org/p/WTKa021ipJ

[rsc]

Comment 12:

Labels changed: added release-none.

[gopherbot]

Comment 13 by robryk:

We have http.StripPrefix which modifies Request.URL.Path and passes the modified request
to a handler. We also have http.Redirect, which assumes that Request.URL.Path is
absolute.
Thus, if one tries to use http.Redirect within a handler that is wrapped in
http.StripPrefix the redirect will point to a wrong URL. In fact, we don't use
http.Redirect in http.FileServer for this very reason:
https://code.google.com/p/go/source/browse/src/pkg/net/http/fs.go#339
We could abandon StripPrefix and have another way of passing the prefix to handlers. We
could also keep the original path and the possibly-shortened path in request, so that
redirect could use the original path.

[gopherbot]

Comment 14 by nigel.tao.gnome:

Make http.Server recovering from panics opt-in instead of opt-out.
https://groups.google.com/forum/#!msg/golang-dev/rXs4TG1gdXw/7BQ29S4NPrgJ