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 unquoting #165

Closed
wants to merge 1 commit into from
Closed

Fix unquoting #165

wants to merge 1 commit into from

Conversation

lucasts
Copy link

@lucasts lucasts commented Jan 16, 2018

use yarl public api to unquote

@@ -295,7 +295,7 @@ def modify_request(self, request):
"""
Apply common path conventions eg. / > /index.html, /foobar > /foobar.html
"""
filename = unquote(request.match_info['filename'])
filename = URL(request.match_info['filename'], encoded=True).path
Copy link
Member

Choose a reason for hiding this comment

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

URL.build(path=...) is even better.

Copy link
Author

Choose a reason for hiding this comment

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

Done.

Copy link
Member

Choose a reason for hiding this comment

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

Looks like not :)

@codecov
Copy link

codecov bot commented Jan 17, 2018

Codecov Report

Merging #165 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #165   +/-   ##
======================================
  Coverage    94.9%   94.9%           
======================================
  Files          12      12           
  Lines         765     765           
  Branches       93      93           
======================================
  Hits          726     726           
  Misses         25      25           
  Partials       14      14
Impacted Files Coverage Δ
aiohttp_devtools/runserver/serve.py 94.29% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 33ef2dc...ab52ea4. Read the comment docs.

@lucasts
Copy link
Author

lucasts commented Jan 17, 2018

updated travis.yml as well to test against current aiohttp version

@vadim-su
Copy link

Please, merge it :)

- use yarl public api to unquote
- update travis env to use latest aiohttp compatible version(2.3.9)
@samuelcolvin
Copy link
Member

please fix and I'll merge

@lucasts
Copy link
Author

lucasts commented Jan 18, 2018

not sure what to do with the failing aiohttp 2.0 test(using yarl 0.10.3)
any directions @samuelcolvin / @asvetlov ?

@samuelcolvin
Copy link
Member

as per #164 I think we're happy to drop 2.0.0, shall we support only >2.3.0?

samuelcolvin pushed a commit that referenced this pull request Jan 18, 2018
- use yarl public api to unquote
- update travis env to use latest aiohttp compatible version(2.3.9)
@samuelcolvin
Copy link
Member

I'm replacing this with #170 as the changed required conflicted and I want to release an update.

samuelcolvin added a commit that referenced this pull request Jan 18, 2018
* when not serving static files, don't mess the apps static_root_url
tests updated
changed middleware to aiohttp's newer decorator middleware

* change aiohttp versions supported

* Fix unquoting, picked from #165

- use yarl public api to unquote
- update travis env to use latest aiohttp compatible version(2.3.9)

* require aiohttp>=2.3.9
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.

None yet

4 participants