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: enhanced ServeMux routing #61410

Closed
jba opened this issue Jul 18, 2023 · 159 comments
Closed

net/http: enhanced ServeMux routing #61410

jba opened this issue Jul 18, 2023 · 159 comments

Comments

@jba
Copy link
Contributor

jba commented Jul 18, 2023

10 October 2023: Updated to clarify escaping: both paths and patterns are unescaped segment by segment, not as a whole. We found during implementation that this gives the behavior we would expect.


7 August 2023: updated with two changes:

  • added Request.SetPathValue
  • GET also matches HEAD

We propose to expand the standard HTTP mux's capabilities by adding two features: distinguishing requests based on HTTP method (GET, POST, ...) and supporting wildcards in the matched paths.

See the top post of this discussion for background and motivation.

Proposed Changes

Methods

A pattern can start with an optional method followed by a space, as in GET /codesearch or GET codesearch.google.com/. A pattern with a method is used only to match requests with that method,
with one exception: the method GET also matches HEAD.
It is possible to have the same path pattern registered with different methods:

GET /foo
POST /foo

Wildcards

A pattern can include wildcard path elements of the form {name} or {name...}. For example, /b/{bucket}/o/{objectname...}. The name must be a valid Go identifier; that is, it must fully match the regular expression [_\pL][_\pL\p{Nd}]*.

These wildcards must be full path elements, meaning they must be preceded by a slash and followed by either a slash or the end of the string. For example, /b_{bucket} is not a valid pattern. Cases like these can be resolved by additional logic in the handler itself. Here, one can write /{bucketlink} and parse the actual bucket name from the value of bucketlink. Alternatively, using other routers will continue to be a good choice.

Normally a wildcard matches only a single path element, ending at the next literal slash (not %2F) in the request URL. If the ... is present, then the wildcard matches the remainder of the URL path, including slashes. (Therefore it is invalid for a ... wildcard to appear anywhere but at the end of a pattern.) Although wildcard matches occur against the escaped path, wildcard values are unescaped. For example, if a wildcard matches a%2Fb, its value is a/b.

There is one last, special wildcard: {$} matches only the end of the URL, allowing writing a pattern that ends in slash but does not match all extensions of that path. For example, the pattern /{$} matches the root page / but (unlike the pattern / today) does not match a request for /anythingelse.

Precedence

There is a single precedence rule: if two patterns overlap (have some requests in common), then the more specific pattern takes precedence. A pattern P1 is more specific than P2 if P1 matches a (strict) subset of P2’s requests; that is, if P2 matches all the requests of P1 and more. If neither is more specific, then the patterns conflict.

There is one exception to this rule, for backwards compatibility: if two patterns would otherwise conflict and one has a host while the other does not, then the pattern with the host takes precedence.

These Venn diagrams illustrate the relationships between two patterns P1 and P2 in terms of the requests they match:

relationships between two patterns

Here are some examples where one pattern is more specific than another:

  • example.com/ is more specific than / because the first matches only requests with host example.com, while the second matches any request.
  • GET / is more specific than / because the first matches only GET and HEAD requests while the second matches any request.
  • HEAD / is more specific than GET / because the first matches only HEAD requests while the second matches both GET and HEAD requests.
  • /b/{bucket}/o/default is more specific than /b/{bucket}/o/{noun} because the first matches only paths whose fourth element is the literal “default”, while in the second, the fourth element can be anything.

In contrast to the last example, the patterns /b/{bucket}/{verb}/default and /b/{bucket}/o/{noun} conflict with each other:

  • They overlap because both match the path /b/k/o/default.
  • The first is not more specific because it matches the path /b/k/a/default while the second doesn’t.
  • The second is not more specific because it matches the path /b/k/o/n while the first doesn’t.

Using specificity for matching is easy to describe and preserves the order-independence of the original ServeMux patterns. But it can be hard to see at a glance which of two patterns is the more specific, or why two patterns conflict. For that reason, the panic messages that are generated when conflicting patterns are registered will demonstrate the conflict by providing example paths, as in the previous paragraph.

The reference implementation for this proposal includes a DescribeRelationship method that explains how two patterns are related. That method is not a part of the proposal, but can help in understanding it. You can use it in the playground.

More Examples

This section illustrates the precedence rule for a complete set of routing patterns.

Say the following patterns are registered:

/item/
POST /item/{user}
/item/{user}
/item/{user}/{id}
/item/{$}
POST alt.com/item/{user}

In the examples that follow, the host in the request is example.com and the method is GET unless otherwise specified.

  1. “/item/jba” matches “/item/{user}”. The pattern "/item/" also matches, but "/item/{user}" is more specific.
  2. A POST to “/item/jba” matches “POST /item/{user}” because that pattern is more specific than "/item/{user}" due to its explicit method.
  3. A POST to “/item/jba/17” matches “/item/{user}/{id}”. As in the first case, the only other candidate is the less specific "/item/".
  4. “/item/” matches “/item/{$}” because it is more specific than "/item/".
  5. “/item/jba/17/line2” matches “/item/”. Patterns that end in a slash match entire subtrees, and no other more specific pattern matches.
  6. A POST request with host “alt.com” and path “/item/jba” matches “POST alt.com/item/{user}". That pattern is more specific than “POST /item/{user}” because it has a host.
  7. A GET request with host “alt.com” and path “/item/jba” matches “/item/{user}”. Although the pattern with a host is more specific, in this case it doesn’t match, because it specifies a different method.

API

To support this API, the net/http package adds two new methods to Request:

package http

func (*Request) PathValue(wildcardName string) string
func (*Request) SetPathValue(name, value string)

PathValue returns the part of the path associated with the wildcard in the matching pattern, or the empty string if there was no such wildcard in the matching pattern. (Note that a successful match can also be empty, for a "..." wildcard.)

SetPathValue sets the value of name to value, so that subsequent calls to PathValue(name) will return value.

Response Codes

If no pattern matches a request, ServeMux typically serves a 404 (Not Found). But if there is a pattern that matches with a different method, then it serves a 405 (Method Not Allowed) instead. This is not a breaking change, since patterns with methods did not previously exist.

Backwards Compatibility

As part of this proposal, we would change the way that ServeMux matches paths to use the escaped path (fixing #21955). That means that slashes and braces in an incoming URL would be escaped and so would not affect matching. We will provide the GODEBUG setting httpmuxgo121=1 to enable the old behavior.

More precisely: both patterns and paths are unescaped segment by segment. For example, "/%2F/%61", whether it is a pattern or an incoming path to be matched, is treated as having two segments containing "/" and "a". This is a breaking change for both patterns, which were not unescaped at all, and paths, which were unescaped in their entirety.

Performance

There are two situations where questions of performance arise: matching requests, and detecting conflicts during registration.

The reference implementation for this proposal matches requests about as fast as the current ServeMux on Julien Schmidt’s static benchmark. Faster routers exist, but there doesn't seem to be a good reason to try to match their speed. Evidence that routing time is not important comes from gorilla/mux, which is still quite popular despite being unmaintained until recently, and about 30 times slower than the standard ServeMux.

Using the specificity precedence rule, detecting conflicts when a pattern is registered seems to require checking all previously registered patterns in general. This makes registering a set of patterns quadratic in the worst case. Indexing the patterns as they are registered can significantly speed up the common case. See this comment for details. We would like to collect examples of large pattern sets (in the thousands of patterns) so we can make sure our indexing scheme works well on them.

@AndrewHarrisSPU
Copy link

Really enthusiastic about this, seems like an excellent improvement to me!

To support this API, the net/http package adds a new method to Request:

package http

func (*Request) PathValue(wildcardName string) string

Is the status of setting PathValues still as mentioned here: #60227 (comment) - omitted for minimal proposal?

(I convinced myself something like a one-shot Request.ParsePathValues(pattern string), slicing the clean URL path, was the least hazard-free thing. I guess I'd be surprised if Handlers start to depend on PathValue but there was no way to populate independently of ServeMux.)

@jba
Copy link
Contributor Author

jba commented Jul 18, 2023

@AndrewHarrisSPU, yes, I deliberately omitted a way to set path values to be minimal. But I do see what you mean about having no way to set it. At the very least, tests should be able to set path values on a request without going through the matching process.

I don't think that exposing the parsing logic should be done as a method on Request; I'm not sure it should be done at all.

I would go with something simpler. Perhaps:

// SetPathValue sets key to value, so that subsequent calls to r.PathValue(key) return value.
func (r *Request) SetPathValue(key, value string)

@flibustenet
Copy link

Could you expose the pro and con about adding SetPathValue in the current proposal ?

@jba
Copy link
Contributor Author

jba commented Jul 18, 2023

@flibustenet, the pros of being able to set values are that it would be easier to write tests, and it would make it possible for middleware to transmit information via the path values.

That second pro is also a con: once we allow setting, then we have introduced this bag of key-value pairs to http.Request, which anyone can use for whatever they want. Middleware could overwrite path wildcards that a handler depended on. Or the storage could be co-opted for something completely unrelated to the path, much as people abuse contexts (only here it's worse, since setting a path value is a destructive change, so it is visible to everyone who has a pointer to the same request).

@benhoyt
Copy link
Contributor

benhoyt commented Jul 18, 2023

As someone who's played around with various Go routing techniques, I really like this, and think it'll more or less eliminate the need for third-party routers, except for very specialized cases.

I've tested it using the reference implementation in my go-routing suite, and it was very easy to use. I just copied the chi version and tweaked it a bit. Lack of regex matching like [0-9]+ means you have to error-check any strconv.Atoi calls for integer path values, but that's not a big deal. I much prefer the simplicity of just handling strings rather than fancy regex and type features.

A few comments/questions:

  1. One thing I noticed is that old versions of ServeMux accept patterns like HandleFunc("GET /foo", ...). This would mean code written for the enhanced ServeMux would compile and appear to run fine on older versions of Go, but of course the routes wouldn't match anything. I think that's a bit error-prone, and wonder if there's a workaround. Possibilities are:
    • Instead of overloading the existing methods, use new methods like HandleMatch and HandleMatchFunc. Obviously a fairly big departure from the current proposal, but at least clear what's old vs new.
    • Update old versions of Go (maybe the last couple of versions) to panic on patterns that have a space in them, or perhaps patterns that use any of the enhanced features. This might be too much of a breaking change.
  2. How would you customise the 405 response? For example, for API servers that need to always return JSON responses. The 404 handler is sort of easy to override with a catch-all / pattern. But for 405 it's harder. You'd have to wrap the ResponseWriter in something that recorded the status code and add some middleware that checked that -- quite tricky to get right.
  3. {$} is a bit awkward. Is $ by itself ever used in URLs, and would it be backwards-incompatible just to use it directly?
  4. Regarding SetPathValue: I agree something like this would be good for testing. Perhaps instead of being a method on Request, it could use httptest.NewRequestWithPathValues or similar, so it's clearly a function marked for testing and not general use? I'm not sure how that would be implemented in terms of package-internals though.

@francislavoie
Copy link

francislavoie commented Jul 18, 2023

In Caddy, we have the concept of request matchers. I'm seeing isn't covered by this proposal is how to route requests based on headers. For example, something commonly done is multiplexing websockets with regular HTTP traffic based on the presence of Connection: Upgrade and Upgrade: websocket regardless of the path. What's the recommendation for routing by header?

Another learning from Caddy is we decided to go with "path matching is exact by default" and we use * as a fast suffix match, so /foo/* matches /foo/ and /foo/bar but not /foo, and /foo* matches /foo and /foo/bar and /foobar. Two paths can be defined for a route like path /foo /foo/* to disallow /foobar but still work without the trailing slash. We went with this approach because it gives ultimate flexibility by having routes that match exactly one URL nothing else.

Finally, there's an experimental https://developer.mozilla.org/en-US/docs/Web/API/URL_Pattern_API which has a lot of great ideas for matching URL patterns. I'd suggest considering some of the syntax from there, for example ? making the trailing slash optional.

@muirdm
Copy link

muirdm commented Jul 19, 2023

Although wildcard matches occur against the escaped path, wildcard values are unescaped. For example, if a wildcard matches a%2Fb, its value is a/b.

Does that apply for "..." wildcards as well? If so, would that mean the handler couldn't differentiate literal slashes from escaped slashes in "..." wildcard values?

@jba
Copy link
Contributor Author

jba commented Jul 19, 2023

@muirdm: Yes, in a "..." wildcard you couldn't tell the difference between "a%2Fb" and "a/b". If a handler cared it would have to look at Request.EscapedPath.

@jba
Copy link
Contributor Author

jba commented Jul 19, 2023

@francislavoie: Thanks for all that info.

The Caddy routing framework seems very powerful. But I think routing by headers is out of scope for this proposal. Same for many of the features Caddy routing and the URL pattern API you link to. I think we expect that people will just write code for all those cases. By the usual Go standards, this proposal is already a big change.

@jba
Copy link
Contributor Author

jba commented Jul 19, 2023

@benhoyt, thanks for playing around with this.

  1. I don't know what to do about older versions of ServeMux.Handle accepting "GET /". We can't change the behavior in older Go versions. If we added new methods we could make Handle panic in the new version, which I guess would solve the problem for code that was written in the new version but ended up being compiled with the old version. But that's probably rare. Maybe a vet check would make sense.

  2. Again, no good answer to how to customize the 405. You could write a method-less pattern for each pattern that had a method, but that's not very satisfactory. But really, this is a general problem with the Handler signature: you can't find the status code without writing a ResponseWriter wrapper. That's probably pretty common—I know I've written one or two—so maybe it's not a big deal to have to use it to customize the 405 response.

  3. $ is not escaped in URLs, so we can't use it as a metacharacter by itself.

  4. I like the idea that only httptest can set the path values. We can always get around the visibility issues with an internal package, so I'm not worried about that.

@rsc
Copy link
Contributor

rsc commented Jul 19, 2023

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@flibustenet
Copy link

I see two others usages of SetPathValue

To can start with standard router and switch to an improved one if needed without changing all the methods to retrieve the PathValue in the handlers. @jba you said that it's anyway a big work to switch the routers but I don't think so, I every time begin by standard router and only switch when needed without so much works, and we'll do even more with this proposal.

There is also the case where we want to route from a handler to an other handler or middleware and adjust the PathValues. For example with a complex parsing /api/{complex...} we'll want to expand complex in some PathValues and like to use them with the same method PathValue(). Or we'll need to use request.Context bag and it's worse.

It will prevent the temptation to make mux overcomplicated when we can improve it easily by adding/setting PathValues. We already do this with Form.

@treuherz
Copy link

One small addition to this I would really like to see is the option to read back from a request which pattern it was matched with. We find this functionality in other frameworks really helpful for logging and metrics, and for resolving ambiguity in how a request reached a handler. Where it doesn't exist, we add it by using a middleware to add a request tag string to the context, but this causes a stutter wherever it's declared and pollutes logical concerns (routing) with cross-cutting ones (observability) in a way that makes otherwise-simple routing code harder to read and write.

Currently this doesn't exist in net/http, but could sometimes be fudged if a handler separately knew the path prefix it was registered under. With this change the template could be even more different from the URL's Path, so being able to explicitly pull it out becomes more important.

My proposed API would be only adding a Pattern string field to http.Request, which would be set by the router. I realise this brings up similar potential concerns about providing more places for data to be smuggled into requests, but this placement would match most of the information already on a Request, and would be able to be set by other frameworks as well.

Similar functionality exists already in gorilla/mux and chi.

I realise this change is kind of orthogonal to the routing additions, although I think it is made more pressing by them. Happy to open this as a separate issue if that's preferred.

@jba
Copy link
Contributor Author

jba commented Jul 24, 2023

@flibustenet, good point about Request.Form already being a bag of values. In fact, there are several others as well: PostForm, MultipartForm, Header and Trailer. Given those, SetPathValue seems relatively benign.

@jba
Copy link
Contributor Author

jba commented Jul 24, 2023

@treuherz, I see the usefulness for retrieving the pattern, but it feels out of scope for this issue. I'd suggest a separate proposal.

@iamdlfl
Copy link

iamdlfl commented Jul 26, 2023

Regarding response codes, would a 405 response automatically include the "Allow" header with appropriate methods?

@prochac
Copy link

prochac commented Jul 26, 2023

What is the state of the request's method?

package http

func (*Request) PathValue(wildcardName string) string

I can see that in reference implementation it's part of the ServeMux

package muxpatterns

func (mux *ServeMux) PathValue(r *http.Request, name string) string

IMO, it should stay this way.

Alternatively, if the method is still intended to be a request method, this proposal should make a commitment to allow setting the path values by 3rd party routers.

@jba
Copy link
Contributor Author

jba commented Jul 26, 2023

It's part of ServeMux in the reference implementation because I couldn't see how to modify http.Request without making muxpatterns unusable with net/http.

If it stayed there then every handler would have to have some way to access its ServeMux. But handlers only take ResponseWriters and Requests.

We are leaning to adding a Set method. As was pointed out, it wouldn't be the only bag of values in Request.

@jba
Copy link
Contributor Author

jba commented Jul 26, 2023

@iamdlfl, apparently the spec for a 405 says we MUST do it, so we will. I'm just not sure what to put there if there is a matching pattern that has no method. I guess all the valid HTTP methods?

@iamdlfl
Copy link

iamdlfl commented Jul 27, 2023

Sounds good. And if someone doesn't define the optional method when registering the route, I don't think we'll have to worry about responding with a 405, right? If I'm understanding right, it will respond normally to any method type.

@jba
Copy link
Contributor Author

jba commented Jul 27, 2023

Good point!

@jimmyfrasche
Copy link
Member

Some common related features are naming patterns and taking a pattern and its arguments and outputting a url. This typically gets bundled as something you pass to templates so you can do something like {{ url "user_page" user.id }} so the template doesn't need to be updated if the named pattern changes from /u/{id} to /user/{id} or the like. I don't think this should necessarily be included per se but it would be nice if now or in a later proposal there were enough added so that things of this nature could be built. Sorry if this has come up I haven't been able to pay a great deal of attention to this proposal's discussion.

@nate-anderson
Copy link

nate-anderson commented Jul 31, 2023

I like the idea of richer routing capabilities in the standard library, but I don't love how overloading Handle and HandleFunc to just accept a new string format makes Go programs using the new syntax backwards-incompatible in a way that can't be detected at compile time.

I like @benhoyt's suggestion of infixing Match into new method names, but I would prefer to also see the method+space pattern prefix omitted for two reasons:

  1. Sometimes a route may wish to accept multiple (but not all) methods - does this string format allow this?
  2. We have the HTTP methods conveniently defined as constants in net/http, but they can't be used here without some ugly and awkward string concatenation.

I would suggest these method signatures:

func (s *ServeMux) HandleMatch(pattern string, handler http.Handler, methods ...string)

func (s *ServeMux) HandleMatchFunc(pattern string, handler http.HandlerFunc, methods ...string)

If no methods are passed in the variadic arg, behave exactly like HandleFunc with the addition of wildcard matching, otherwise return a 405 if the request method doesn't match one in the methods slice. The method(s) being a dedicated but optional argument is self-documenting, IMO, compared to putting all that information into a single string.

This will feel very similar to the .Methods() chaining used commonly in gorilla/mux, which itself could be another approach to consider here.

@jub0bs
Copy link

jub0bs commented Oct 12, 2023

@doggedOwl I don't find this approach satisfactory: you then no longer have clean separation of concerns, since you're discriminating between paths both at the router level and within the middleware.

@eliben
Copy link
Member

eliben commented Oct 12, 2023

@doggedOwl I don't find this approach satisfactory: you then no longer have clean separation of concerns, since you're discriminating between routes both at the router level and within the middleware.

Since this discussion is already very long and it's difficult to search, can you elaborate on the solution you propose to this issue? I may have missed it. Is it being able to specify multiple methods in a route (e.g. GET,OPTIONS /path/) or something else?

@atdiar
Copy link

atdiar commented Oct 12, 2023

@jub0bs oh I see, sorry I had misunderstood the issue.
I meant that the OPTION handler applies for all requests.
Then in its logic, it should indeed take into account each request URL.

That can probably be implemented at the handler level by allowing internal state for the OPTION part of the CORS handler to change each time a new CORS response handler is created for a route. (created, not when it's applied which would be too late).

Using a closure to create both the handler for the preflight and a CORS handler constructor in case we want many different CORS policies.

@carlmjohnson
Copy link
Contributor

Another way to solve the problem of /api/ has different CORS from the rest of the site is to use a sub-router and wrap it in middleware:

mux := http.NewServeMux()
mux.Handle("/someroute", ...)

api := http.NewServeMux()
api.Handle("/api/someotherroute", ...)
	
mux.Handle("/api/", corsMiddleware(api))

@jub0bs
Copy link

jub0bs commented Oct 12, 2023

@eliben Sure. See the bottom of this comment:

One glimmer of hope that would minimise duplication and improve developer experience perhaps lies in #61410 (comment):

the door is open to additional syntax, like "GET,POST /foo" to register two methods with the same pattern.

@atdiar No worries. I'm not too sure what your suggestion would look like, though.

@carlmjohnson Promising! Let me think about it.

@c9845
Copy link

c9845 commented Oct 16, 2023

I find the prefixing of the path with "GET ", "POST ", etc. extremely odd and unergonomic. We have http.MethodGet and similar, why not use them? Sure, you can do http.MethodGet + " /path/to/this" (or use fmt.Sprintf and build the string) but that is annoying. I do understand that we don't want to break backward compatibility with the HandleFunc by changing its signature to HandleFunc(method, path string, handler func()), but using "GET /path/to/this/" is just plain ugly.

Why not put the method after the handler func as a variadic argument and, if is is missing, just match on all methods? Something like HandleFunc(path string, handler func(), method... string). Yes, this does get a bit ugly if you use an anonymous func for the handler, but I would wager that style is rarer than defining a func and using it in HandlerFunc.

Or, define HandleFuncWithMethod but that is annoying in other ways.

@zephyrtronium
Copy link
Contributor

Why not put the method after the handler func as a variadic argument and, if is is missing, just match on all methods? Something like HandleFunc(path string, handler func(), method... string).

This is still a breaking change. It would change the interfaces that ServeMux implements.

@doggedOwl
Copy link

doggedOwl commented Oct 16, 2023

Is it being able to specify multiple methods in a route (e.g. GET,OPTIONS /path/)?

I sure hope not.
It's arguably wrong to (try to) handle the preflight requests (OPTIONS) at the same level as the resource request (GET etc).
One of the main reasons is that preflight requests need to preceed and shortcut almost all middleware, especially the ones that filter requests based on headers which are not guaranteed to be sent on preflight requests.

The most wellknown example is the authentication headers that by the CORS standard should not be sent.
Generally the authentication middleware is wrapping all the handlers (or a subset of them like in the sub-router example) so it will fail the preflight requests that are configured at the request handler.
This failure mode is well-known and can be protected against but other less known headers might create the same unpredictable failures.

Options like this (multi-method matching) create the illusion of easy handling and encourage bad patterns and potential for future bugs.

@sentriz
Copy link

sentriz commented Oct 17, 2023

i really like the "stringy matching"

  • it's backwards compatible
  • HTTP methods are loose anyway, consider how WebDAV extends HTTP with verbs like MOVE, COPY, LOCK
  • the argument is already named pattern. maybe later we could also match on scheme, host, query params. why not? method is not the only guy in town

IMO the string is the most flexible. if you're worried about typoing a method. a safe wrapper function is trivial

@jub0bs
Copy link

jub0bs commented Oct 17, 2023

@carlmjohnson Sorry for getting back to you so late. Your approach of declaring a sub-router and wrapping it in the middleware indeed works great, even when using jba/muxpatterns's method-matching functionality:

mux := muxpatterns.NewServeMux()
mux.HandleFunc("GET /hello", ...)

api := muxpatterns.NewServeMux()
api.HandleFunc("GET /api/hello", ...)
api.HandleFunc("PUT /api/helloUpdate", ...)

mux.Handle("/api/", corsMiddleware(api)) // note: method-less pattern

There is duplication of the /api/ path prefix but it's overall much, much cleaner than having to systematically register the same handler for the actual method and the OPTIONS method:

mux.Handle("GET /api/hello", handler)
mux.Handle("OPTIONS /api/hello", handler)

Thanks for that! I'm so relieved! It's so simple and elegant. I curse myself for not thinking about it earlier. I will be recommending your approach to users of my middleware.

@doggedOwl I agree with the general sentiment of your latest comment, and I think Carl's approach completely solves the issue.

@eudore
Copy link

eudore commented Oct 22, 2023

I re-implemented a ServeMux using Radix.

Detailed reference: net/http: reimplementing ServeMux

@carlmjohnson
Copy link
Contributor

The Go team might accept a patch to switch the internal implementation of ServeMux to something faster, but you can’t open a PR with breaking changes and expect it to go anywhere. The Go 1 guarantee means you have to keep the old API working.

@jamietanna
Copy link

I see that there is now a standard for URL Patterns, under WHATWG - is there anything we can adapt from this (or even fully take the pattern structure in board) or are there characteristics of our implementation that may be more appropriate for what Go needs?

I realise that this may not necessarily make sense due to WHATWG's focus, but thought I'd ask!

@carlmjohnson
Copy link
Contributor

carlmjohnson commented Oct 25, 2023

I thought I had previously opened some issue about URL Patterns, but it looks like I just thought about it and didn't do it because I can't find any such issue.

The timing is a little unfortunate because Go is going to add something new and different just as this URL Patterns thing is launching, but I think realistically, you couldn't add URL Pattern support to ServeMux without breaking backwards compatibility anyway, so it would need to be in net/http/v2.

I think the next steps for URL Patterns and Go is to make a third party library that can match a URL to pattern and extract the matching bits, and then just see if anyone uses it. If it's popular, you can make a proposal for either golang.org/x/urlpatterns or net/url/urlpatterns depending on how popular it is. Part of the thing with URL patterns is they have applications outside of just routing, so I think even if it were in ServeMux, you'd want to be able to do matching outside of ServeMux as well.

@gocs
Copy link

gocs commented Nov 3, 2023

http.HandleFunc("GET /", ...
// vs
http.GET("/", ...

If the pattern is inside a string which could not be checked during compile time, what would it's behavior if the given pattern does not match any method like typo, should the go vet analyzer (like printf analyzer or go staticcheck) handle it? And could it be auto-completed?
Using http.GET("/", ... is just convenient, compared to having the methods inside a string.

@jub0bs
Copy link

jub0bs commented Nov 3, 2023

@gocs Note that the set of valid HTTP methods extends beyond the usual GET, POST, PUT, etc. The string approach frees developers to use any valid method they want. If you're worried about typos in method names, you can always write a small wrapper for yourself:

func GET(pathPattern string, h http.Handler) {
    http.Handle(http.MethodGet + " " + pathPattern, h)
}

@c9845
Copy link

c9845 commented Nov 3, 2023

I think asking developers to "write a small wrapper" and define funcs for GET, POST, PUT, etc. is a bit ridiculous (probably a bad word). It isn't like it will just be a few developers that would define/want such wrappers. It would make more sense to have the standard library define http.GET() and so on. I see this as the exact same justification we have strings.Cut over using strings.Index.

@jba
Copy link
Contributor Author

jba commented Nov 3, 2023

Closing this because the proposal has been implemented.

@jba jba closed this as completed Nov 3, 2023
@jub0bs
Copy link

jub0bs commented Nov 3, 2023

@c9845 I understand your concern, but this proposal aims for backwards compatibility and endeavours not to add too much API. The wrappers I mentioned in my earlier comment could be added to net/http later, if needed.

@Kristav-ghoost
Copy link

I tested this and it works great. A well needed add on if you ask me. Is there a benchmark comparing this implementation with other routers?

@jba
Copy link
Contributor Author

jba commented Nov 3, 2023

Here is one that compares routing to the original ServeMux: https://github.com/jba/muxpatterns/blob/main/server_test.go#L331.

But one outcome of the discussion on this issue is that we could find no evidence that routing speed matters, and some evidence that it doesn't. If you have an application where routing speed matters, we'd love to hear about it.

@DeedleFake
Copy link

@c9845

You could also just write a function that'll handle all of them:

func Route(method, path string) string {
  return fmt.Sprintf("%s %s", method, path)
}

Then you can use, for example, http.Handle(Route(http.MethodGet, "/"), ...). It's slightly more error prone, but makes it simpler to use the existing constants. It would be better if the constants had their own type so that the first argument could be type checked, but that would be a backwards incompatible change, so it's not likely.

@c9845
Copy link

c9845 commented Nov 24, 2023

@DeedleFake Thanks for the suggestion. I am kind of just seeing where this whole development/implementation/whatever ends up and then going from there. I imagine a lot of people are in the same boat.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Accepted
Development

No branches or pull requests