Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify path prefix rewrite / redirect docs for HTTPPathModifier #2078

Closed
briantkennedy opened this issue May 30, 2023 · 0 comments · Fixed by #2079
Closed

Clarify path prefix rewrite / redirect docs for HTTPPathModifier #2078

briantkennedy opened this issue May 30, 2023 · 0 comments · Fixed by #2079
Labels
kind/bug Categorizes issue or PR as related to a bug. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@briantkennedy
Copy link
Contributor

What happened:

The ReplacePrefixMatch docstring is a bit confusing since it doesn't provide a value for ReplacePrefixMatch in the example: ReplacePrefixMatch specifies the value with which to replace the prefix match of a request during a rewrite or redirect. For example, a request to "/foo/bar" with a prefix match of "/foo" would be modified to "/bar".

What you expected to happen:

The intro sentence should have the replacement prefix in the example:

For example, a request to "/foo/bar" with a prefix match of "/foo" and a replace prefix of "/xyz" would be modified to "/xyz/bar"

Clarifying trailing slash behavior would ideally be incorporate into a table of incoming request, prefix match value, replace prefix and output, eg:

Request Path Prefix Match Replace Prefix Rewritten
/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 /bar /bar

While making the above table, I noticed that empty string in ReplacePrefixMatch can be problematic since it will have inconsistent behavior when the request path is identical to the prefix match.

Request Path Prefix Match Replace Prefix Rewritten
/foo/bar /foo empty string /bar
/foo /foo empty string / (this should technically be empty string, but that's not a valid path).
@briantkennedy briantkennedy added the kind/bug Categorizes issue or PR as related to a bug. label May 30, 2023
@shaneutt shaneutt added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label May 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants