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 the behavior of url dispatching with quote characters. #414

Merged

Conversation

Lothiraldan
Copy link
Contributor

The quoted characters of the URL will not be significant during url dispatching
but will be decoded after the matching.

For example, with this route:

@asyncio.coroutine
def handler(request):
    return web.Response(body=repr(request.match_info))

app = Application(loop=loop)
app.router.add_route('GET', '/{route}', handler)

The following query will generate this match_info:

curl -I http://127.0.0.1:8080/route%2Fslash
HTTP/1.1 200 OK
...
<MatchInfo {'route': 'route/slash'}: <DynamicRoute [*] /{route} -> <function handler at 0x10b888158>>

"""
return self._splitted_path.path

@property
Copy link
Member

Choose a reason for hiding this comment

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

Use @reofy instead of @property

@asvetlov
Copy link
Member

Please fix my inline notes and add doc for unquoted_path property into docs/web_reference.rst.

@Lothiraldan
Copy link
Contributor Author

I'm not sure we want to make this behavior the default one and according to #351 (comment) it's the RFC behavior that we may want to make the default one. I will fix the other notes.

@asvetlov
Copy link
Member

After re-reading RFC I pretty sure that unquoting should be done after finding a match but before pushing match_dict into request.

I mean here is unquoted path but here match_dict should have quoted values.

I believe proposed schema is clean and straightforward.

@asvetlov
Copy link
Member

@kxepal would you join to discussion?

After re-reading RFC I pretty sure that unquoting should be done after finding a match but before pushing match_dict into request.

Does my proposal make sense?

@kxepal
Copy link
Member

kxepal commented Jun 20, 2015

@asvetlov absolutely. +1 for that.

@@ -1,3 +1,11 @@
0.17.0 (XX-XX-XXXX)
Copy link
Member

Choose a reason for hiding this comment

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

You need to modify CHANGES.rst instead. 0.17.0 is an upcoming release, not the archived one.

@Lothiraldan Lothiraldan force-pushed the feature/unquoted-path-url-dispatching branch 3 times, most recently from b59af6e to 003c4ad Compare June 20, 2015 18:06
@Lothiraldan Lothiraldan changed the title Add support for url dispatching using unquoted path. Change the behavior of url dispatching with quote characters. Jun 20, 2015
@Lothiraldan
Copy link
Contributor Author

I've made the changes and update the commit message + PR description.

The quoted characters of the URL will not be significant during url dispatching
but will be decoded after the matching.

For example, with this route:

```
@asyncio.coroutine
def handler(request):
    return web.Response(body=repr(request.match_info))

app = Application(loop=loop)
app.router.add_route('GET', '/{route}', handler)
```

The following query will generate this match_info:

```
curl -I http://127.0.0.1:8080/route%2Fslash
HTTP/1.1 200 OK
...
<MatchInfo {'route': 'route/slash'}: <DynamicRoute [*] /{route} -> <function handler at 0x10b888158>>
```
@Lothiraldan Lothiraldan force-pushed the feature/unquoted-path-url-dispatching branch from 003c4ad to 2f1d33c Compare June 20, 2015 18:53
@asvetlov asvetlov merged commit 2f1d33c into aio-libs:master Jun 21, 2015
@asvetlov
Copy link
Member

Thanks

@lock
Copy link

lock bot commented Oct 30, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Oct 30, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants