Special characters break routes

[rauanmayemir]

I had two kinds of routes with 'special' (not sure if it's a valid term) characters and vanguard refused to serve both of them.

1) Method names with : character in them

E.g for a method like this:

  rpc DoSomething (DoSomethingRequest) returns (DoSomethingResponse) {
    option (google.api.http) = {
      post: "/method/objects/{id}:do_something"
    };
  };

sending a POST /method/objects/123:do_something won't work.

2) Methods with resource placeholders that have : character in them:

E.g for a method like this:

  rpc GetSomething (GetSomethingRequest) returns (GetSomethingResponse) {
    option (google.api.http) = {
      get: "/method/objects/{id}"
    };
  };

sending neither GET /method/objects/my:weird:identifier, nor GET /method/objects/my%3Aweird%3Aidentifier will work. (I also tried different type of escaping for : which I can't remember, because 'rawurlencode' skips :, but nothing will work)

However, GET /method/objects?id=my:weird:identifier would work fine if we skip the placeholder in HttpRule.

[jhump]

Colons are "special" because of the grammar for the URI path: https://github.com/googleapis/googleapis/blob/common-protos-1_3_1/google/api/http.proto#L206-L213

Basically, a colon can separate a field value from a "verb" suffix. The convention is that the verb suffix is used with more action-centric RPCs, where the RPC's action doesn't cleanly map to an HTTP method. For more details, see the relevant bits of AEP 136.

So if the first bullet does not work, that must be a bug, because that is expected to work. But the second one currently is expected to not work, specifically due to the first. The path element is being split by the colon, and the matcher would be thinking that "my" is the field value and "weird:identifier" is a custom verb suffix, which of course does not match any of the methods.

We may be able to tweak some of this. We could, for example, at least support field values with colons that aren't in the last path element. (I can't recall if we already support that or not.) We could relax the matcher so that if there is no route that matches an incoming URI's custom verb suffix, we try to combine the suffix with earlier part of path element and match that. But I'd worry that this would be a compatibility/ambiguity issue. If you ever added a method with a custom suffix to this service, then prior URLs would suddenly be interpreted differently.

[rauanmayemir]

Re compatibility, envoy transcoder supports both, so maybe it won’t be as ambiguous.

I acknowledge that doing ‘my:weird:identifier:verb’ is just asking for trouble.