Skip to content

capture request data with aiohttp multipart/form-data #32543

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 19, 2023

Conversation

kristapratico
Copy link
Member

@kristapratico kristapratico commented Oct 17, 2023

Resolves #32473

If both files and data are passed in the request, only what's in files gets added to aiohttp.FormData in the code below. data gets dropped and is not encoded into request.

def _get_request_data(self, request):
"""Get the request data.
:param request: The request object
:type request: ~azure.core.pipeline.transport.HttpRequest or ~azure.core.rest.HttpRequest
:rtype: bytes or ~aiohttp.FormData
:return: The request data
"""
if request.files:
form_data = aiohttp.FormData()
for form_file, data in request.files.items():
content_type = data[2] if len(data) > 2 else None
try:
form_data.add_field(form_file, data[1], filename=data[0], content_type=content_type)
except IndexError as err:
raise ValueError("Invalid formdata formatting: {}".format(data)) from err
return form_data
return request.data

For example, if I'm doing multipart/form-data with files and data={"response_format": "srt"}, I would expect my request body to include the response_format in the form-data like below:

b'--df13136170d9e03a0e82350af6b2afb7\r\nContent-Disposition: form-data; name="response_format"\r\n\r\nsrt\r\n--df13136170d9e03a0e82350af6b2afb7\r\nContent-Disposition: form-data; name="file"; filename="hello.m4a"\r\nContent-Type: application/octet-stream\r\n\r\n\x00\x00\x00\x18ftypmp42\x00\x00\x00\x00mp41isom\x00\x00\x00(uuid\\\xa7\x08\xfb2\x8eB\x05\xa8ae\x0e\xca\n\x95\x96\x00\x00\x00\x0c10.0.22621.0\x00\

In the current implementation, the request.data (and in this example, response_format) is ignored. This PR captures data and includes/uncomments tests for async.

@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

Copy link
Member

@xiangyan99 xiangyan99 left a comment

Choose a reason for hiding this comment

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

LGTM

@kristapratico
Copy link
Member Author

core, containerregistry, and tables CI failures seem unrelated with the same tests failing on the main build.

@kristapratico
Copy link
Member Author

/check-enforcer override

@kristapratico kristapratico merged commit bf0213f into Azure:main Oct 19, 2023
@kristapratico kristapratico deleted the aiohttp-multipart-formdata branch October 19, 2023 18:46
kristapratico added a commit that referenced this pull request Oct 19, 2023
iscai-msft added a commit to iscai-msft/azure-sdk-for-python that referenced this pull request Oct 19, 2023
…into update_package_json

* 'main' of https://github.com/Azure/azure-sdk-for-python: (136 commits)
  Re-record (Azure#32584)
  Update CODEOWNERS (Azure#32583)
  capture request data with aiohttp multipart/form-data (Azure#32543)
  [Core] Create generic azure-core package: `corehttp` (Azure#32191)
  overwrite .amlignore file (Azure#32546)
  Fix Sphinx error in core (Azure#32417)
  skip pyright for now (Azure#32569)
  Fix identity-broker sdist (Azure#32568)
  Updates to interactions with `pyproject.toml` (Azure#32480)
  fix: fixing local variable's name(last_modified_exising -> last_modified_existing) typo via Cspell (Azure#32556)
  Update feature store related entities to use the GA version (Azure#32541)
  Removing unused RST files (Azure#32547)
  Update python version for virtualenv issues (Azure#32528)
  [AutoRelease] t2-keyvault-2023-10-13-36681(can only be merged by SDK owner) (Azure#32478)
  [Monitor Distro] Update broken link (Azure#32505)
  Bump python stress images and addons (Azure#32554)
  Bump (Azure#32552)
  Increment package version after release of azure-cosmos (Azure#32550)
  repair aggregate report pipeline (Azure#32509)
  update readme and samples (Azure#32548)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[core] aiohttp multipart/form-data drops data when files included
5 participants