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
merged 1 commit into from Jun 21, 2015

Conversation

Projects
None yet
3 participants
@Lothiraldan
Contributor

Lothiraldan commented Jun 16, 2015

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

This comment has been minimized.

@asvetlov

asvetlov Jun 17, 2015

Member

Use @reofy instead of @property

HISTORY.rst Outdated
@@ -1,3 +1,9 @@
0.16.6 (XX-XX-XXXX)

This comment has been minimized.

@asvetlov

asvetlov Jun 17, 2015

Member

Actually the PR will land into 0.17.0

@@ -297,14 +297,19 @@ class UrlDispatcher(AbstractRouter, collections.abc.Mapping):
hdrs.METH_GET, hdrs.METH_PUT, hdrs.METH_DELETE,
hdrs.METH_PATCH, hdrs.METH_HEAD, hdrs.METH_OPTIONS}
def __init__(self):
def __init__(self, unquoted_path=False):

This comment has been minimized.

@asvetlov

asvetlov Jun 17, 2015

Member

Parameter is not required

@asyncio.coroutine
def resolve(self, request):
path = request.path
if self.unquoted_path:
path = request.unquoted_path

This comment has been minimized.

@asvetlov

asvetlov Jun 17, 2015

Member

Always use request.unquoted_path

@asvetlov

This comment has been minimized.

Member

asvetlov commented Jun 17, 2015

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

@Lothiraldan

This comment has been minimized.

Contributor

Lothiraldan commented Jun 17, 2015

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

This comment has been minimized.

Member

asvetlov commented Jun 17, 2015

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

This comment has been minimized.

Member

asvetlov commented Jun 20, 2015

@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

This comment has been minimized.

Member

kxepal commented Jun 20, 2015

@asvetlov absolutely. +1 for that.

HISTORY.rst Outdated
@@ -1,3 +1,11 @@
0.17.0 (XX-XX-XXXX)

This comment has been minimized.

@kxepal

kxepal Jun 20, 2015

Member

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

@@ -164,12 +164,21 @@ def _splitted_path(self):
return urlsplit(self._path_qs)
@property
def unquoted_path(self):

This comment has been minimized.

@kxepal

kxepal Jun 20, 2015

Member

I think you actually mean here quoted_path. Since otherwise unquote operation for already unquoted path should makes no sense, but it does. How about raw_path instead?

This comment has been minimized.

@asvetlov

asvetlov Jun 20, 2015

Member

Good catch. IIRC I've suggested unquoted_path name but raw_path is definitely better name.

@kxepal thank you again for advice.

You know, naming and cache invalidation are the most hard things in computer science :)

This comment has been minimized.

@kxepal

kxepal Jun 20, 2015

Member

Haha, indeed (: You're welcome!

@Lothiraldan Lothiraldan force-pushed the Lothiraldan:feature/unquoted-path-url-dispatching branch 3 times, most recently from b59af6e to 003c4ad Jun 20, 2015

@Lothiraldan Lothiraldan changed the title from Add support for url dispatching using unquoted path. to Change the behavior of url dispatching with quote characters. Jun 20, 2015

@Lothiraldan

This comment has been minimized.

Contributor

Lothiraldan commented Jun 20, 2015

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

FELD Boris
Change the behavior of url dispatching with quote characters.
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 Lothiraldan:feature/unquoted-path-url-dispatching branch from 003c4ad to 2f1d33c Jun 20, 2015

@asvetlov asvetlov merged commit 2f1d33c into aio-libs:master Jun 21, 2015

2 checks passed

continuous-integration/appveyor AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@asvetlov

This comment has been minimized.

Member

asvetlov commented Jun 21, 2015

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment