URLs using verb no longer work after upgrading to v1.5.0

[orjanbruland]

We tested out the latest 1.5.0 release and noticed it broke our URLs that use colons with a custom verb, like described here: https://cloud.google.com/apis/design/custom_methods

In our case we have a both a "Get" method and a custom method sharing parts of the URL, like this:

  rpc GetDevice(GetDeviceRequest) returns (Device) {
    option (google.api.http) = { get: "/v2/{name=projects/*/devices/*}" };
  }
  rpc StreamDevices(StreamDevicesRequest) returns (stream StreamDevicesResponse) {
      option (google.api.http) = { get: "/v2/{name=projects/*/devices/*}:stream" };
   }

When upgrading from v1.4.1 to v1.5.0 requests no longer seem to reach the StreamDevices method.

Steps you follow to reproduce the error:

By extending the TestMuxServeHTTP test with the following test-case, I'm able to trigger the same issue:

{
			patterns: []stubPattern{
				{
					method: "GET",
					ops:    []int{int(utilities.OpLitPush), 0, int(utilities.OpPush), 0, int(utilities.OpConcatN), 1, int(utilities.OpCapture), 1},
					pool:   []string{"foo", "id"},
				},
				{
					method: "GET",
					ops:    []int{int(utilities.OpLitPush), 0, int(utilities.OpPush), 0, int(utilities.OpConcatN), 1, int(utilities.OpCapture), 1},
					pool:   []string{"foo", "id"},
					verb:   "verb",
				},
			},
			reqMethod: "GET",
			reqPath:   "/foo/bar:verb",
			headers: map[string]string{
				"Content-Type": "application/json",
			},
			respStatus:  http.StatusOK,
			respContent: "GET /foo/{id=*}:verb",
		},

What did you expect to happen instead:

I would expect the test to reach the /foo/{id=*}:verb pattern, or in our case reach the StreamDevices method as in the previous releases.

What's your theory on why it isn't working:

It seems to be a side-effect of fixing #224.

[johanbrandhorst]

Argh, thanks for raising this issue! Terribly sorry to have broken your workflow. It looks like we have to revert #708, is that correct?

[orjanbruland]

One solution could be to introduce a ServeMuxOption to allow Patterns created without a verb to match requests using a verb. This way, using colons in IDs is an opt-in feature instead of breaking backward compatibility.

[johanbrandhorst]

Yeah, backwards compatibility is paramount here, of course. It also seems like the wrong thing to support this tbh, I'm just disappointed we didn't discover it earlier and didn't break users. I'll see what some of the others think but we'll likely just revert #708 and continue the discussion in #224.