Semantics of url rewrite prefix replace - clarifications required

[LiorLieberman]

This is a followup to a surprisingly long thread on slack.

Our spec has a very detailed table of expectations for ReplacePrefixMatch field. However it does not cover the case where the Prefix Match(not the replacement) is "/".

Adding the spec table here for convenience:

// Request Path | Prefix Match | Replace Prefix | Modified Path
// -------------|--------------|----------------|----------
// /foo/bar     | /foo         | /xyz           | /xyz/bar
// /foo/bar     | /foo         | /xyz/          | /xyz/bar
// /foo/bar     | /foo/        | /xyz           | /xyz/bar
// /foo/bar     | /foo/        | /xyz/          | /xyz/bar
// /foo         | /foo         | /xyz           | /xyz
// /foo/        | /foo         | /xyz           | /xyz/
// /foo/bar     | /foo         | <empty string> | /bar
// /foo/        | /foo         | <empty string> | /
// /foo         | /foo         | <empty string> | /
// /foo/        | /foo         | /              | /
// /foo         | /foo         | /              | /

The ambiguity mostly comes down to the case where Prefix Match is "/". Below are a few examples:

// Request Path | Prefix Match | Replace Prefix | Modified Path
// -------------|--------------|----------------|----------
// /bar         | /            | /xyz           | /xyz/bar (AND NOT "/xyzbar")
// /bar         | /            | /xyz/          | /xyz/bar (AND NOT "/xyz//bar")
// /            | /            | /xyz           | /xyz (AND NOT "/xyz/")
// /bar         | /            | <empty string> | /bar (And NOT "bar")
// /            | /            | <empty string> | / (AND NOT "")
// /            | /            | /              | /

AND NOT is a language I used to reflect the proposed standardization. You can also be read it as OR if you have a different opinion.

Although there is no easy way in envoy to achieve this behavior (other non-envoy implementations, please shout if this is easily possible with your proxies), @howardjohn came up with a regex that makes this possible (ref https://github.com/istio/istio/pull/54939/files#diff-a0e8831b6aefb0ef9b2cd269fcd726b26fd1104120950952b5217a7b104ba153R1668-R1669)

Hoping this thread would result in a change to our spec to explicitly iron it out.

/cc @mikemorris @arkodg @robscott @howardjohn @kflynn

[kflynn]

I made this suggestion on Slack:

I’m wondering how well it would work to simply say that:

• if the rewrite prefix starts with a /, make sure the rewriting pattern does too, and
• if the rewrite prefix ends with a /, make sure the rewriting pattern does too.

That would imply that your “rewrite prefix / to /xyz” would become “rewrite prefix / to /xyz/“, which I think is likely to always be what people expect. 🤔 It still leaves the door open for “rewrite /foo to /bar”, but maybe that’s a necessary evil…

I think that making that change in validation could resolve this ambiguity, and even without validation, making this change to the spec would let implementations refuse rewrites that are likely to cause trouble. Anyone else have thoughts?

[howardjohn]

I think that making that change in validation could resolve this ambiguity, and even without validation, making this change to the spec would let implementations refuse rewrites that are likely to cause trouble. Anyone else have thoughts?

I don't really understand the suggestion. What is a rewrite prefix and rewrite pattern? Might help to give some examples and use the same terms are the tables above (Request Path | Prefix Match | Replace Prefix | Modified Path) so we don't get things mixed up?

[snorwin]

IMO all cases, including edge cases (i.e, / as replace prefix or match prefix), should be explicitly defined in the Gateway API specification and also covered by the conformance tests.

Additionally, the behavior should be intuitive for users rather than easy to be implemented. For instance, it is not obvious to me that a match prefix / replaced with /xyz for a request like /foo/bar would result in /xyzfoo/bar rather than /xyz/foo/bar.

[snorwin]

According to the current specification, trailing slashes in both the prefix match and replace prefix are ignored. Incorporating this rule into the documented behavior, I've constructed the following table to illustrate how various request paths are transformed under different prefix configurations:

Request Path	Prefix Match	Replace Prefix	Modified Path
/	/	/xyz(/)?	/xyz
/foo	/	/xyz(/)?	/xyz/foo
/foo/	/	/xyz(/)?	/xyz/foo/
/foo	/foo(/)?	/xyz(/)?	/xyz
/foo/	/foo(/)?	/xyz(/)?	/xyz/
/foo/bar	/foo(/)?	/xyz(/)?	/xyz/bar
/foo/bar/	/foo(/)?	/xyz(/)?	/xyz/bar/
/foo	/foo(/)?	(/)?	/
/foo/	/foo(/)?	(/)?	/
/foo/bar	/foo(/)?	(/)?	/bar
/foo/bar/	/foo(/)?	(/)?	/bar/

Note: The notation /foo(/)? is used to represent both /foo and /foo/.

@LiorLieberman does this interpretation align with your interpretation of the spec?

[youngnick]

I agree that whatever we end up agreeing, it should be explicitly laid out in the table in the spec, and also tested with conformance tests.

[Miciah]

The ReplacePrefixMatch godoc says that the replacement operates on full path elements:

gateway-api/apis/v1/httproute_types.go

Lines 1095 to 1097 in 5dd3155

	// Note that this matches the behavior of the PathPrefix match type. This
	// matches full path elements. A path element refers to the list of labels
	// in the path split by the `/` separator. When specified, a trailing `/` is

So I think it is clear that a request path /bar with prefix match / and replacement prefix /xyz does not result in /xyzbar, which would be treating things as strings to be concatenated rather than as full path elements. However, it is ambiguous whether a replacement could introduce an empty path element. Could the ambiguity be eliminated just by saying that the replacement cannot introduce empty path elements, including adding a trailing slash or producing an empty path?