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

Fix overshadowing of overlapped subapp prefixes (#3701). #3708

Merged

Conversation

nikie
Copy link
Contributor

@nikie nikie commented Apr 22, 2019

What do these changes do?

Fix for issue #3701 - overshadowing of overlapped subapp prefixes.

The fix allows different subapps to have prefixes with overlapping beginning. The following didn't work properly before - the second subapp prefix was hidden by the first one as subapp prefixes matching was performed by startswith only, so the first added app won:

    app.add_subapp('/qwe/', create_subapp01())
    app.add_subapp('/qwerty/', create_subapp02())

Are there changes in behavior for the user?

Subapps with overlapping prefixes can now be used without unexpected overshadowing.

Related issue number

#3701

Notes

  • I have considered storing the value of self._prefix + '/' in an attribute for optimization but was not sure to do that because it adds complexity. Please let me know if such optimization is desired - I can dig further.

  • Without adding request.url.raw_path != self._prefix to the comparison the existing behavior of matching empty and root subapp routes would have changed. That didn't break any existing test though, so I have added 2 more tests to make sure this works as before - test_urldispatch.py/test_prefixed_subapp_empty_route and test_urldispatch.py/test_prefixed_subapp_root_route

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes.
    Please let me know if any documentation change is required.
  • If you provide code modification, please add yourself to CONTRIBUTORS.txt
  • Add a new news fragment into the CHANGES folder

@asvetlov
Copy link
Member

Thanks!
Sorry for the long delay

@asvetlov asvetlov merged commit 2aaf111 into aio-libs:master Aug 30, 2019
asvetlov pushed a commit that referenced this pull request Aug 30, 2019
(cherry picked from commit 2aaf111)

Co-authored-by: Eugene Nikolaiev <[email protected]>
asvetlov added a commit that referenced this pull request Aug 31, 2019
@nikie nikie deleted the bugfix/3701-overshadow-of-overlapped-subapps branch August 31, 2019 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants