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

Revert Prevent trailing slash in patterns of proxy rules of backend api configs with a path #1405

Merged
merged 1 commit into from Nov 8, 2019

Conversation

guicassolato
Copy link
Contributor

@guicassolato guicassolato commented Nov 7, 2019

This PR reverts #1394 and introduces new tests to the proxy rule decorator.
It goes in the opposite direction of #1400, which should be closed in case we decide to move on with this one.

Related to THREESCALE-3833.

Closes THREESCALE-3872.

The Jira reported as a bug a scenario where two requests with and without a trailing slash, targeting a mapping rule of a backend whose pattern was / (AKA the "catch-all" mapping rule pattern), were treated differently. According to the description of the bug, one would expect those things to be trated equally. But this is not correct. Requests with and without the trailing slash should be trated as different request.

First reason is that, even though some web servers and web clients may treat those cases as equal, it's also a valid interpretation the a URL that does not end with a slash targets a resource, while the same URL with a slash appended to the end triggers additional semantics to the request. The resource pointed can be a file or a directory or even represent a soft parameter coded in the URL; without a trailing slash, the request is asking for the resource. If a trailing is added, then the resource cannot be a file and if it's a directory can instruct a web server to look for a possible index.html file in that directory, for example.

It also adds some confusion the fact that, following the RFC, user agents (e.g. curl) sometimes add a slash to the path, but that is a leading slash, not a trailing slash. In APIAP terms, when the catch-all mapping rule is defined at the backend level and that backend has a routing path at the product level, then the trailing slash is NOT optional. The user can still make it optional if wanted, by creating a product-level mapping rule with the same pattern as the routing path of the backend and no trailing slash.

Another reason why we should not remove the trailing slash is that system will allow the definition of two distinct mapping rules, where one coincides exactly to the pattern of the other but adds a trailing slash. The users assume those two mapping rules to be used for different cases. Yet after #1394 ans specially if #1400 gets merged, we could be injecting both mapping rules with the exact same pattern.

Finally, as there are some semantics involved behind the meaning of requests with and without a trailing slash, better not to try "fixing" what the customer defined.

@guicassolato guicassolato self-assigned this Nov 7, 2019
[Refactor] ProxyRuleDecorator#pattern without using 'File'

Always remove mapping rule trailing slash

Revert trailing slash removal from mapping rule patterns
@Martouta
Copy link
Contributor

Martouta commented Nov 7, 2019

The accurate Jira issue is THREESCALE-3872

Copy link
Contributor

@Martouta Martouta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description and the code make sense to me 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants