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

custom regex of router should support \s of the pattern #6619

Closed
1 task done
NewUserHa opened this issue Feb 15, 2022 · 4 comments · Fixed by #8898
Closed
1 task done

custom regex of router should support \s of the pattern #6619

NewUserHa opened this issue Feb 15, 2022 · 4 comments · Fixed by #8898
Labels

Comments

@NewUserHa
Copy link
Contributor

NewUserHa commented Feb 15, 2022

Describe the bug

router like /{fn:\w+ \d+} or /{fn:\w+\s\d+} can't match path like /abc 123,
because of the coroutine resolve(request) uses raw_path, which is /abc%20123 and needs pattern like /{fn:\w+%20\d+}.

Usually, users will write or \s for a pattern instead of %20

Python Version

$ python --version
Python 3.8.10

aiohttp Version

$ python -m pip show aiohttp
Version: 3.8.1

Related component

Server

Additional context

No response

Code of Conduct

  • I agree to follow the aio-libs Code of Conduct
@NewUserHa NewUserHa added the bug label Feb 15, 2022
@NewUserHa NewUserHa changed the title custom regex should support \s of the pattern custom regex of router should support \s of the pattern Feb 15, 2022
@NewUserHa
Copy link
Contributor Author

NewUserHa commented Feb 15, 2022

Also, if the URL has special (like % {space}) and non-ascii (other language characters) characters and is urlencoded, the custom regex can't work at all. (i.e. the users have to write the pattern with the urlencoded raw path)

@Dreamsorcerer
Copy link
Member

I think spaces are typically discouraged in URLs, which is why this probably hasn't come up before. If you can come with an easy fix though, I'm sure we can consider merging it.

@NewUserHa
Copy link
Contributor Author

NewUserHa commented Feb 15, 2022

say if the url is a totally valid file path and contains like [ ] ( ) @, then the regex of the route say like @routes.get(r'/{fn:.+\[.+\].+) then it can't ever work. (or checking containing other language characters in the route can't work too.)

How about coroutine resolve(request) use request.path instead of request.raw_path?

@Dreamsorcerer
Copy link
Member

I don't have time to take a look at the moment. But, I'd suggest trying the change and running the test suite, that will likely tell you if there are problems with the solution.

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

Successfully merging a pull request may close this issue.

2 participants