Skip to content

Fix for issue #975 #976

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.

Sign up for GitHub

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

Closed
fr33mang wants to merge 1 commit into from
Closed

Fix for issue #975 #976

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion connexion/apis/aiohttp_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from aiohttp import web
from aiohttp.web_exceptions import HTTPNotFound, HTTPPermanentRedirect
from aiohttp.web_middlewares import normalize_path_middleware
from aiohttp.web_request import FileField
from connexion.apis.abstract import AbstractAPI
from connexion.exceptions import ProblemException
from connexion.handlers import AuthErrorHandler
Expand Down Expand Up @@ -278,14 +279,17 @@ def get_request(cls, req):
if req.body_exists:
body = yield from req.read()

data = yield from req.post()
Copy link
Contributor

@kornicameister kornicameister Jul 3, 2019

Choose a reason for hiding this comment

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

As far as I can tell, this will never work.
If you post the request, whatever that is because important part is to have something in that, so the condition req_body_exists evaluates to True, you are reading the entire payload from aiohttp.web.Request in line 280 (yield from req.read()). Later on you are attempting to read the payload yet again via req.post(). That's why it fails.

What might have be needed is here is to distinguish between multipart requests and ordinary requests.
I assume that multipart requests might be processed in a way that we extract files and normal fields into separate variables and push'em into ConnexionRequest.

Copy link
Contributor

@prawn-cake prawn-cake Jul 3, 2019

Choose a reason for hiding this comment

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

Good catch!
Indeed, you can't read aiohttp.web.Request payload twice, since it's a StreamReader object and both methods read from it.
Calling different readers based on the content type sounds like a solution.

Copy link
Contributor

@kornicameister kornicameister Jul 3, 2019

Choose a reason for hiding this comment

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

@prawn-cake Give it a whirl then :)

Copy link
Contributor

@kornicameister kornicameister Jul 5, 2019

Choose a reason for hiding this comment

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

@prawn-cake I will give it a try on my own. We're interested in having it fixed as soon as possible.

Copy link
Contributor

@prawn-cake prawn-cake Jul 6, 2019

Choose a reason for hiding this comment

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

Thanks @kornicameister !
Sorry for not being very active here. I'll check the changes

files = {k: v for k, v in data.items() if isinstance(v, FileField)}

return ConnexionRequest(url=url,
method=req.method.lower(),
path_params=dict(req.match_info),
query=query,
headers=headers,
body=body,
json_getter=lambda: cls.jsonifier.loads(body),
files={},
files=files,
context=req)

@classmethod
Expand Down