Skip to content

aiohttp multipart/form-data fix applied to corehttp #32581

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

Merged
merged 4 commits into from
Oct 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 12 additions & 0 deletions sdk/core/corehttp/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,17 @@
# Release History

## 1.0.0b2 (Unreleased)

### Features Added

### Breaking Changes

### Bugs Fixed

- Fixed an issue with `multipart/form-data` in the async transport where `data` was not getting encoded into the request body. #32473

### Other Changes

## 1.0.0b1 (2023-10-18)

* Initial Release
2 changes: 1 addition & 1 deletion sdk/core/corehttp/corehttp/_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@
# regenerated.
# --------------------------------------------------------------------------

VERSION = "1.0.0b1"
VERSION = "1.0.0b2"
2 changes: 1 addition & 1 deletion sdk/core/corehttp/corehttp/transport/aiohttp/_aiohttp.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ def _get_request_data(self, request: RestHttpRequest):
:return: The request data
"""
if request._files: # pylint: disable=protected-access
form_data = aiohttp.FormData()
form_data = aiohttp.FormData(request._data or {}) # pylint: disable=protected-access
for form_file, data in request._files.items(): # pylint: disable=protected-access
content_type = data[2] if len(data) > 2 else None
try:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

# NOTE: These tests are heavily inspired from the httpx test suite: https://github.com/encode/httpx/tree/master/tests
# Thank you httpx for your wonderful tests!
import io
import pytest

from corehttp.rest import HttpRequest, AsyncHttpResponse
Expand Down Expand Up @@ -220,14 +221,25 @@ async def test_urlencoded_content(send_request):
)


# @pytest.mark.asyncio
# async def test_multipart_files_content(send_request):
# request = HttpRequest(
# "POST",
# "/multipart/basic",
# files={"fileContent": io.BytesIO(b"<file content>")},
# )
# await send_request(request)
@pytest.mark.asyncio
async def test_multipart_files_content(send_request):
request = HttpRequest(
"POST",
"/multipart/basic",
files={"fileContent": io.BytesIO(b"<file content>")},
)
await send_request(request)


@pytest.mark.asyncio
async def test_multipart_data_and_files_content(send_request):
request = HttpRequest(
"POST",
"/multipart/data-and-files",
data={"message": "Hello, world!"},
files={"fileContent": io.BytesIO(b"<file content>")},
)
await send_request(request)


@pytest.mark.asyncio
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def basic():
assert_with_message("content type", multipart_header_start, request.content_type[: len(multipart_header_start)])
if request.files:
# aiohttp
assert_with_message("content length", 258, request.content_length)
assert_with_message("content length", 228, request.content_length)
assert_with_message("num files", 1, len(request.files))
assert_with_message("has file named fileContent", True, bool(request.files.get("fileContent")))
file_content = request.files["fileContent"]
Expand All @@ -37,7 +37,7 @@ def basic():
)
assert_with_message(
"content disposition",
'form-data; name="fileContent"; filename="fileContent"; filename*=utf-8\'\'fileContent',
'form-data; name="fileContent"; filename="fileContent"',
file_content.headers["Content-Disposition"],
)
elif request.form:
Expand All @@ -51,9 +51,30 @@ def basic():

@multipart_api.route("/data-and-files", methods=["POST"])
def data_and_files():
assert_with_message("content type", multipart_header_start, request.content_type[: len(multipart_header_start)])
assert_with_message("message", "Hello, world!", request.form["message"])
assert_with_message("message", "<file content>", request.form["fileContent"])
if request.files:
# aiohttp
assert_with_message("content type", multipart_header_start, request.content_type[: len(multipart_header_start)])
assert_with_message("has file named fileContent", True, bool(request.files.get("fileContent")))
assert_with_message("message", "Hello, world!", request.form["message"])
file_content = request.files["fileContent"]
assert_with_message("file content type", "application/octet-stream", file_content.content_type)
assert_with_message("file content length", 14, file_content.content_length)
assert_with_message("filename", "fileContent", file_content.filename)
assert_with_message(
"has content disposition header", True, bool(file_content.headers.get("Content-Disposition"))
)
assert_with_message(
"content disposition",
'form-data; name="fileContent"; filename="fileContent"',
file_content.headers["Content-Disposition"],
)
elif request.form:
# requests
assert_with_message("content type", multipart_header_start, request.content_type[: len(multipart_header_start)])
assert_with_message("message", "Hello, world!", request.form["message"])
assert_with_message("message", "<file content>", request.form["fileContent"])
else:
return Response(status=400)
return Response(status=200)


Expand Down