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

Change behaviour of Router to use segments #3139

Merged
merged 1 commit into from Feb 3, 2020
Merged

Change behaviour of Router to use segments #3139

merged 1 commit into from Feb 3, 2020

Conversation

hamnis
Copy link
Contributor

@hamnis hamnis commented Jan 31, 2020

This changes the router to use segment routing instead of string prefix matching.
The previous behaviour was unsafe and would match longer prefixes which may cause unexpected matches.

This will no longer fall-through on partial matches.

  • Add ContextRouter with same routing logic

This commit changes the router to use segment routing instead of string prefix matching.
The previous behaviour was unsafe and would match longer prefixes which may cause unexpected matches.

This will no longer fallthrough on partial matches.

* Add ContextRouter with same routing logic
@rossabaker rossabaker added this to the 0.21.0-RC3 milestone Feb 1, 2020
Copy link
Member

@rossabaker rossabaker left a comment

I remember being guided by the servlet specification with those rules, but looking again after all those years, I think what you did is right:

The container will recursively try to match the longest path-prefix. This is done
by stepping down the path tree a directory at a time, using the ’/’ character as a
path separator. The longest match determines the servlet selected

And whatever the servlet committee thinks, I think what you've done is more intuitive anyway.

@rossabaker rossabaker added the enhancement label Feb 1, 2020
@hamnis hamnis changed the title Change behaviour of Router Change behaviour of Router to use segments Feb 1, 2020
@hamnis hamnis merged commit b431a0e into master Feb 3, 2020
3 checks passed
@hamnis hamnis deleted the context-router branch Feb 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants