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 swagger console backslash redirect for aiohttp (thanks @pando85) #843

Merged
merged 4 commits into from
Mar 25, 2019

Conversation

dtkav
Copy link
Collaborator

@dtkav dtkav commented Jan 8, 2019

Fixes #831

Changes proposed in this pull request:

  • redirect ui -> ui/ for aiohttp

@jmcs
Copy link
Contributor

jmcs commented Jan 14, 2019

Won't this introduce a mismatch between this endpoint and all the others?

@dtkav
Copy link
Collaborator Author

dtkav commented Jan 14, 2019

@jmcs good point. I guess the best path forward is to look into enabling this for all routes on aiohttp to make the behavior similar to flask?

@dtkav dtkav changed the title fix swagger console backslash redirect for aiohttp (thanks @pando85) WIP: fix swagger console backslash redirect for aiohttp (thanks @pando85) Jan 14, 2019
@dtkav
Copy link
Collaborator Author

dtkav commented Jan 24, 2019

It turns out we have to manually add a redirect for this route specifically because AioHttp removes the trailing slash from the static_files route (ui/*).

See https://github.com/aio-libs/aiohttp/blob/master/aiohttp/web_urldispatcher.py#L1049

I'll also add the middleware for redirecting all other routes to trailing slashes.

@dtkav
Copy link
Collaborator Author

dtkav commented Jan 24, 2019

Note, this depends on #823

@dtkav
Copy link
Collaborator Author

dtkav commented Jan 24, 2019

Wow this was a real rabbit hole to go down. 🐰
It turns out that some clients will turn POST requests into GET requests on a (301|302|303) redirect!
This is working nicely now with 308 redirect (https://tools.ietf.org/html/rfc7538).

@dtkav dtkav changed the title WIP: fix swagger console backslash redirect for aiohttp (thanks @pando85) Fix swagger console backslash redirect for aiohttp (thanks @pando85) Jan 24, 2019
@dtkav
Copy link
Collaborator Author

dtkav commented Jan 25, 2019

I've also put in a PR with aiohttp to change the default redirect code to 308 in the normalize_path_middleware.
aio-libs/aiohttp#3578

@jmcs
Copy link
Contributor

jmcs commented Feb 4, 2019

@dtkav can you figure out why the flake8 started failing?

@dtkav dtkav force-pushed the aiohttp_redirect branch 4 times, most recently from 151eafb to e822b0f Compare February 4, 2019 12:41
@jmcs
Copy link
Contributor

jmcs commented Mar 25, 2019

👍

@jmcs jmcs merged commit 1e6aead into spec-first:master Mar 25, 2019
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.

3 participants