net/http: allow multiple spaces between method and path in mux patterns

[egtann]

Proposal Details

When testing the muxpatterns implementation which is landing in the stdlib Mux in Go 1.22, I was surprised to find that I could not add extra whitespace between the method and path. For instance, this does not work:

r.Handle("GET  /my-route", handler) // note the 2nd space!

Allowing extra spaces is useful when you have many routes and want to keep the code neatly aligned, so it's easy to scan the routes visually. As a small example:

r.Handle("GET    /my-route", handler1)
r.Handle("POST   /my-route", handler2)
r.Handle("DELETE /my-route", handler3)

[adonovan]

"Be strict in what you accept" is the usual (contemporary) advice for interface design, and the API documentation is clear that it must be a single space. Perhaps @jba can offer more insight.

[jba]

I can't see any reason to enforce the single space, and aligning patterns seems like a good reason to drop it.
Any arguments against allowing [ \t]+ between the two? (In English, at least one character of horizontal whitespace.)

[adonovan]

Any arguments against allowing [ \t]+ between the two? (In English, at least one character of horizontal whitespace.)

This is consistent with HTTP itself, which also allows [ \t]+ between GET and the URL in the syntax for a (single-line) request.

[earthboundkid]

Seems fine to me since the verbs can't have spaces anyway.

[AlexanderYastrebov]

This is consistent with HTTP itself, which also allows [ \t]+ between GET and the URL in the syntax for a (single-line) request.

Is it? From https://datatracker.ietf.org/doc/html/rfc9112#name-request-line

A request-line begins with a method token, followed by a single space (SP), the request-target, and another single space (SP), and ends with the protocol version.
request-line = method SP request-target SP HTTP-version

[adonovan]

Is it? From https://datatracker.ietf.org/doc/html/rfc9112#name-request-line

A request-line begins with a method token, followed by a single space (SP), the request-target, and another single space (SP), and ends with the protocol version.
request-line = method SP request-target SP HTTP-version

Right, but the definition of SP in 2.2 Basic Rules also says:

HTTP/1.1 header field values can be folded onto multiple lines if the continuation line begins with a space or horizontal tab. All linear white space, including folding, has the same semantics as SP. A recipient MAY replace any linear white space with a single SP before interpreting the field value or forwarding the message downstream.

which I took to mean that multiple spaces are allowed in the first line of the request. Perhaps I misread it, and this rule applies only to line continuations.

Empirically some servers do permit multiple spaces---not that that is a reliable guide.

$ { echo "GET    /    HTTP/1.1"; echo; } | nc google.com 80 | head -n 1 
HTTP/1.1 200 OK

[rsc]

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

[TheCoreMan]

My gut says this is not a great idea, so I tried to back that up with some concrete cases where this will be a bad idea. And this is what I came up with:

1. Gives room to "argue" in CRs on whether we should or shouldn't use spaces - better to keep it unambiguous
2. The moment you have two routes with the same path and different verbs, you ought to extract the path to a constant anyways (even with one path it makes sense for usage in clients, tests, etc.). So there isn't a ton of style benefit in lining them up
3. Autogenerators will want to update the spaces to allow lining them up, causing unrelated changes when adding a new verb, making code review harder (see this example; the GET change is unrelated to the actual verb we added):

- r.Handle("GET /my-route", handler1)
+ r.Handle("GET  /my-route", handler1)
+ r.Handle("POST /my-route", handler2)

[rsc]

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

The proposal is to accept multiple spaces in patterns like “GET /foo”.

[rsc]

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

The proposal is to accept multiple spaces in patterns like “GET /foo”.

[mibk]

Based on the emoji votes of #64910 (comment) and #64910 (comment), is there really a consensus?

[Malix-Labs]

@rsc in #64910 (comment):

Based on the discussion above, this proposal seems like a likely accept.

The discussion above from @TheCoreMan was about

some concrete cases where this will be a bad idea

Thus, the upvotes are to highlight the cases where this proposal would be a bad idea, not to upvote the proposal.

@rsc in #64910 (comment):

consensus

Where?

There is no consensus I could find,
But if there were some, it is more towards a decline.

[Malix-Labs]

In my opinion, it's not a bad idea if it doesn't slow down compilation parsing (would there be any way to test that?), and if gofmt and gofumpt apply the same rule to them at they apply for extra spaces in general.

[Malix-Labs]

@earthboundkid may I ask why you downvoted #64910 (comment)?
Did I miss something?

[mvdan]

I don't think we should worry about parse performance at all. What matters is the performance for routing and serving HTTP requests, and that's not affected at all, as far as I can tell.

Personally I find the argument at #64910 (comment) compelling in terms of allowing any whitespace. I'm not as worried in terms of consistent formatting noise, to be entirely honest - the same arguments could be made against the way gofmt already aligns var () or const () blocks, for example. Another existing example is struct field tags, which also allow extra spaces like `json:"foo" yaml:"bar"`, and which can also be used for some form of alignment if a user wishes to.

Perhaps consider diff tools which support ignoring whitespace in some way. For example, GitHub already has a knob to ignore all whitespace changes like git diff -w, and Gerrit has similar settings as well. This is also useful when not wanting to see alignment noise due to gofmt, for example. Personally, I've just learned to visually skip over formatting changes relatively quickly, so I don't mind.

When it comes to the reactions or votes, my impression is they area helpful signal, but they shouldn't be used directly to measure consensus or reach a decision for a proposal. I personally value the opinion of the package maintainers above reactions, and in this case, both Jonathan and Russ (Neil hasn't commented) appear to lean in favor, and I'd tend to agree with them.

[Malix-Labs]

@mvdan in #64910 (comment):

I don't think we should worry about parse performance at all

If it's marginal, then there is nothing to worry about.

@mvdan in #64910 (comment):

Personally I find the argument at #64910 (comment) compelling in terms of allowing any whitespace

Same.

@mvdan in #64910 (comment):

⋯reactions or votes ⋯ shouldn't be used directly to measure consensus or reach a decision for a proposal ⋯ I personally value the opinion of the package maintainers above reactions ⋯

Understood.

[earthboundkid]

@Malix-off, you edited your comment, but before it said,

However, I surely hope gofmt and gofumpt strip-out those space

I think gofmt editing strings is a bad idea.

I don't really understand the opposition to this proposal, tbh. Seems harmless to me. I don't think I will do it, but I also wouldn't change a codebase I encounter that does.

[gopherbot]

Change https://go.dev/cl/565916 mentions this issue: net/http: allow multiple spaces between method and path in mux patterns