Fix openapiv2 path parameter parsing when colon in verb

[johanbrandhorst]

This makes the openapiv2 path parameter parser consistent with our grpc-gateway handler parser. In the future, ideally we'd unify these two parsers (see #2824).

The fix introduced in #2727 caused verb literals with colons in them to be split twice instead of once. Change the logic so that any : immediately terminates parsing with the rest of the string considered part of the verb literal.

CC @oyvindwe

[oyvindwe]

Even within {}?

[johanbrandhorst]

Fair point... should it be allowed as part of a variable name? The grammar is:

// ### Path template syntax
//
//     Template = "/" Segments [ Verb ] ;
//     Segments = Segment { "/" Segment } ;
//     Segment  = "*" | "**" | LITERAL | Variable ;
//     Variable = "{" FieldPath [ "=" Segments ] "}" ;
//     FieldPath = IDENT { "." IDENT } ;
//     Verb     = ":" LITERAL ;

Since this PR is making the assertion that a LITERAL can contain a : (by allowing it after the first : which indicates the start of the verb), I suppose we should also allow it in Segment, which would mean allowing it inside {}. I'll rewrite this and add another test, and check what our other parser does in this case.

[johanbrandhorst]

Thanks for your quick look at this, I really appreciate it. I changed it so we allow colons in variables and added a test to both parsers.

[oyvindwe]

Note that LITERAL is not allowed to contain :, so supporting more than one colon in the verb is supporting paths outside of the HttpRule specification:

The syntax LITERAL matches literal text in the URL path. If the LITERAL
contains any reserved character, such characters should be percent-encoded
before the matching.

As I understand it, 'reserved' here refers to RFC 6570:

reserved       =  gen-delims / sub-delims
gen-delims     =  ":" / "/" / "?" / "#" / "[" / "]" / "@"
sub-delims     =  "!" / "$" / "&" / "'" / "(" / ")"
               /  "*" / "+" / "," / ";" / "="

However, since this is already supported in the the HttpRule parser, I see the point in also supporting it in the OpenAPI generator. I also back the suggestion on using the same parser both places. The question is if the parser should be strict or not.

Does the HttpRule parser also allow other path templates outside the specification? Specifically : before a variable? The reason I am asking is that the the joining of the path in the generator assumes that only the last part can start with :.

[johanbrandhorst]

Hm, we were never able to figure out what characters were reserved when making the amendments to the HttpRule parser in #1947, where did you find the reference to RFC 6570? It probably makes sense but would be good to have for future reference.

Unfortunately, as you surmise, it's probably out of the question for us to change this behavior now, so we're stuck supporting colons in the verb (and by extension, any LITERAL).

Good question on the : preceding a variable, it probably does, I'll add a test and see. In any case, I think that can just be a separate issue from this immediate PR.

[johanbrandhorst]

I've raised #2833 so we don't forget the point you made. I think this PR is still OK to go in as is.

[oyvindwe]

I'm just assuming that it refers to reserved in RFC 6570, as it's not explicitly defined in http.proto, and it makes explicit references to the RFC when it's not followed. I might be wrong though.

I agree it's OK to merge this PR.