-
-
Notifications
You must be signed in to change notification settings - Fork 945
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
feat(testing): simulate multipart file upload #2141
base: master
Are you sure you want to change the base?
Changes from 45 commits
d37beaf
f89c31c
32daa63
2b48c6b
6b9b198
0a17b79
187aa4f
9d2fe9e
2b9d0e0
835b062
3e28a94
7920cdd
72d54ee
ded1ac0
66d347b
574e667
ffa8d9f
eadb1f3
7493b92
c49e369
235ed3e
94ae979
c9d4f1b
9a9e58d
608efb3
5c68868
f052dca
b527de7
1465241
e4cb4c9
dba5eb9
51ea53e
8a66346
3041583
77f350b
4bc299a
df37dbd
2149b0c
6f8b3ae
632317c
3b2edd8
bfa6a4c
e1d9834
514e936
dc74456
6d8fd96
eeba35d
aef3e0d
3c2b4fc
8665a34
1b7a207
4572ee8
62275cc
5872d3d
6845e5a
93fad43
3af2fcc
6ae4d06
6ede18d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,18 +22,20 @@ | |
import datetime as dt | ||
import inspect | ||
import json as json_module | ||
import os | ||
import time | ||
from typing import Dict | ||
from typing import Optional | ||
from typing import Sequence | ||
from typing import Union | ||
from urllib.parse import urlencode | ||
import warnings | ||
import wsgiref.validate | ||
|
||
from falcon.asgi_spec import ScopeType | ||
from falcon.constants import COMBINED_METHODS | ||
from falcon.constants import MEDIA_JSON | ||
from falcon.errors import CompatibilityError | ||
from falcon.constants import MEDIA_JSON, MEDIA_URLENCODED | ||
from falcon.errors import CompatibilityError, HTTPBadRequest | ||
from falcon.testing import helpers | ||
from falcon.testing.srmock import StartResponseMock | ||
from falcon.util import async_to_sync | ||
|
@@ -95,7 +97,7 @@ class Cookie: | |
or ``None`` if not specified. | ||
max_age (int): The lifetime of the cookie in seconds, or | ||
``None`` if not specified. | ||
secure (bool): Whether or not the cookie may only only be | ||
secure (bool): Whether or not the cookie may only be | ||
transmitted from the client via HTTPS. | ||
http_only (bool): Whether or not the cookie may only be | ||
included in unscripted requests from the client. | ||
|
@@ -437,6 +439,8 @@ def simulate_request( | |
content_type=None, | ||
body=None, | ||
json=None, | ||
files=None, | ||
data=None, | ||
file_wrapper=None, | ||
wsgierrors=None, | ||
params=None, | ||
|
@@ -528,6 +532,30 @@ def simulate_request( | |
overrides `body` and sets the Content-Type header to | ||
``'application/json'``, overriding any value specified by either | ||
the `content_type` or `headers` arguments. | ||
|
||
Note: | ||
Can only be used if data and files are null, otherwise an exception | ||
is thrown. | ||
|
||
files(dict): same as the files parameter in requests, | ||
dictionary of ``'name': file-like-objects`` (or ``{'name': file-tuple}``) | ||
for multipart encoding upload. | ||
``file-tuple``: can be a 2-tuple ``('filename', fileobj)`` or a | ||
3-tuple ``('filename', fileobj, 'content_type')`` | ||
where ``'content-type'`` is a string defining the content | ||
type of the given file. | ||
|
||
Note: | ||
If both data and json are present, an exception is thrown. | ||
To pass additional form-data with files, use data. | ||
|
||
data : list of tuples, dict or (b)string, with additional data | ||
to be passed with files (or alone if files is null), to be treated | ||
as urlencoded form data. | ||
|
||
Note: | ||
If both data and json are present, an exception is thrown. | ||
|
||
file_wrapper (callable): Callable that returns an iterable, | ||
to be used as the value for *wsgi.file_wrapper* in the | ||
WSGI environ (default: ``None``). This can be used to test | ||
|
@@ -575,6 +603,8 @@ def simulate_request( | |
content_type=content_type, | ||
body=body, | ||
json=json, | ||
files=files, | ||
data=data, | ||
params=params, | ||
params_csv=params_csv, | ||
protocol=protocol, | ||
|
@@ -598,6 +628,8 @@ def simulate_request( | |
headers, | ||
body, | ||
json, | ||
files, | ||
data, | ||
extras, | ||
) | ||
|
||
|
@@ -622,7 +654,7 @@ def simulate_request( | |
# NOTE(vytas): Even given the duct tape nature of overriding | ||
# arbitrary environ variables, changing the method can potentially | ||
# be very confusing, particularly when using specialized | ||
# simulate_get/post/patch etc methods. | ||
# simulate_get/post/patch etc. methods. | ||
raise ValueError( | ||
'WSGI environ extras may not override the request method. ' | ||
'Please use the method parameter.' | ||
|
@@ -651,6 +683,8 @@ async def _simulate_request_asgi( | |
content_type=None, | ||
body=None, | ||
json=None, | ||
files=None, | ||
data=None, | ||
params=None, | ||
params_csv=True, | ||
protocol='http', | ||
|
@@ -736,6 +770,29 @@ async def _simulate_request_asgi( | |
overrides `body` and sets the Content-Type header to | ||
``'application/json'``, overriding any value specified by either | ||
the `content_type` or `headers` arguments. | ||
|
||
Note: | ||
Can only be used if data and files are null, otherwise an exception | ||
is thrown. | ||
|
||
files(dict): same as the files parameter in requests, | ||
vytas7 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
dictionary of ``'name': file-like-objects`` (or ``{'name': file-tuple}``) | ||
for multipart encoding upload. | ||
``file-tuple``: can be a 2-tuple ``('filename', fileobj)`` or a | ||
3-tuple ``('filename', fileobj, 'content_type')``, | ||
where ``'content-type'`` is a string defining the content | ||
type of the given file. | ||
|
||
Mote: | ||
If both files and json are present, an exception is thrown. To pass | ||
additional form-data with files, use data. | ||
|
||
data : list of tuples, dict or (b)string with additional data to be passed with | ||
files (or alone if files is null), to be treated as urlencoded form data. | ||
|
||
Note: | ||
If both data and json are present, an exception is thrown. | ||
|
||
host(str): A string to use for the hostname part of the fully | ||
qualified request URL (default: 'falconframework.org') | ||
remote_addr (str): A string to use as the remote IP address for the | ||
|
@@ -774,6 +831,8 @@ async def _simulate_request_asgi( | |
headers, | ||
body, | ||
json, | ||
files, | ||
data, | ||
extras, | ||
) | ||
|
||
|
@@ -2133,8 +2192,163 @@ async def __aexit__(self, exc_type, exc, tb): | |
await self._task_req | ||
|
||
|
||
def _prepare_data_fields(data, boundary=None, urlenc=False): | ||
"""Prepare data fields for request body. | ||
|
||
Args: | ||
data: dict or list of tuples with json data from the request | ||
|
||
Returns: list of 2-tuples (field-name(str), value(bytes)) | ||
|
||
""" | ||
urlresult = [] | ||
body_part = b'' | ||
if isinstance(data, (str, bytes)) or hasattr(data, 'read'): | ||
fields = list(json_module.loads(data).items()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so we are assuming that if provided as string it's a json. this would mean that we cannot replace deprecate body with data. let's see @vytas7 opinion There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, I hadn't really understood what you guys meant by replacing the body... but ok, that should be an easy fix. I'll try to implement it, then you can use whatever works best. |
||
elif isinstance(data, dict): | ||
fields = list(data.items()) | ||
else: | ||
fields = list(data) | ||
|
||
# Append data to the other multipart parts | ||
for field, val in fields: | ||
if isinstance(val, str) or not hasattr(val, '__iter__'): | ||
val = [val] | ||
# if no files are passed, make urlencoded form | ||
if urlenc: | ||
for v in val: | ||
if v: | ||
urlresult.append( | ||
( | ||
field.encode('utf-8') if isinstance(field, str) else field, | ||
v.encode('utf-8') if isinstance(v, str) else v, | ||
) | ||
) | ||
# if files and data are passed, concat data to files body like in requests | ||
else: | ||
for v in val: | ||
body_part += ( | ||
f'Content-Disposition: form-data; name={field}; ' | ||
f'\r\n\r\n'.encode() | ||
) | ||
if v: | ||
if not isinstance(v, bytes): | ||
v = str(v) | ||
body_part += v.encode('utf-8') if isinstance(v, str) else v | ||
body_part += b'\r\n--' + boundary.encode() + b'\r\n' | ||
else: | ||
body_part += b'\r\n--' + boundary.encode() + b'\r\n' | ||
|
||
return body_part if not urlenc else urlencode(urlresult, doseq=True) | ||
|
||
|
||
def _prepare_files(k, v): | ||
"""Prepare file attributes for body of request form. | ||
|
||
Args: | ||
k: (str), file-name | ||
v: fileobj or tuple (filename, data, content_type?) | ||
|
||
Returns: file_name, file_data, file_content_type | ||
|
||
""" | ||
file_content_type = None | ||
if not v: | ||
raise ValueError(f'No file provided for {k}') | ||
if isinstance(v, (tuple, list)): | ||
if len(v) == 2: | ||
file_name, file_data = v | ||
else: | ||
file_name, file_data, file_content_type = v | ||
TigreModerata marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if ( | ||
len(v) == 3 | ||
and file_content_type | ||
and file_content_type.startswith('multipart/mixed') | ||
): | ||
file_data, new_header = _encode_files(json_module.loads(file_data.decode())) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if we need to allow this custom implementation that allows for nested file send. This is assuming quite a bit of things, in particular that file_data is a byte string that represent json, this is then de-serialized and re-encoded as files. seems quite complex and custom. Maybe a better could be to have a way of creating a multipart body that can be embedded into the main request as plain bytes? so you would pass a nested body as something like this files = {
'x': 'other stuff',
'nested': (None, make_multipart(...), 'multipart/mixed')
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we should support nested multipart forms at all, at least not in this issue. There is a FAQ item on how to parse them that probably already gives too much attention to the topic, it is a super legacy edge case. One can construct the payload in other ways; if they have such legacy code, probably there is already some existing test to create such a payload. |
||
file_content_type = 'multipart/mixed; ' + ( | ||
new_header['Content-Type'].split('; ')[1] | ||
) | ||
else: | ||
# if v is not a tuple or iterable it has to be a filelike obj | ||
name = getattr(v, 'name', None) | ||
if name and isinstance(name, str) and name[0] != '<' and name[-1] != '>': | ||
file_name = os.path.basename(name) | ||
else: | ||
file_name = k | ||
file_data = v | ||
if hasattr(file_data, 'read'): | ||
file_data = file_data.read() | ||
return file_name, file_data, file_content_type | ||
|
||
|
||
def _make_boundary(): | ||
""" | ||
Create random boundary to be used in multipart/form-data with files. | ||
""" | ||
boundary = os.urandom(16).hex() | ||
return boundary | ||
|
||
|
||
def _encode_files(files, data=None): | ||
"""Build the body for a multipart/form-data request. | ||
|
||
Will successfully encode files when passed as a dict or a list of | ||
tuples. ``data`` fields are added first. | ||
The tuples may be 2-tuples (filename, fileobj) or | ||
3-tuples (filename, fileobj, contentype). | ||
Allows for content_type = ``multipart/mixed`` for submission of nested files | ||
|
||
Returns: (encoded body string, headers dict) | ||
""" | ||
boundary = _make_boundary() | ||
body_string = b'--' + boundary.encode() + b'\r\n' | ||
header = {'Content-Type': 'multipart/form-data; boundary=' + boundary} | ||
|
||
# Deal with the files tuples | ||
if not isinstance(files, (dict, list)): | ||
raise ValueError('cannot encode objects that are not 2-tuples') | ||
elif isinstance(files, dict): | ||
files = list(files.items()) | ||
|
||
for (k, v) in files: | ||
file_name, file_data, file_content_type = _prepare_files(k, v) | ||
if not file_data: | ||
continue | ||
|
||
body_string += f'Content-Disposition: form-data; name={k}; '.encode() | ||
body_string += ( | ||
f'filename={file_name}\r\n'.encode() if file_name else '\r\n'.encode() | ||
) | ||
body_string += ( | ||
f'Content-Type: {file_content_type or "text/plain"}\r\n\r\n'.encode() | ||
) | ||
body_string += ( | ||
file_data.encode('utf-8') if isinstance(file_data, str) else file_data | ||
) | ||
body_string += b'\r\n--' + boundary.encode() + b'\r\n' | ||
|
||
# Handle whatever json data gets passed along with files | ||
if data: | ||
body_string += _prepare_data_fields(data, boundary) | ||
|
||
body_string = body_string[:-2] + b'--\r\n' | ||
|
||
return body_string, header | ||
|
||
|
||
def _prepare_sim_args( | ||
path, query_string, params, params_csv, content_type, headers, body, json, extras | ||
path, | ||
query_string, | ||
params, | ||
params_csv, | ||
content_type, | ||
headers, | ||
body, | ||
json, | ||
files, | ||
data, | ||
extras, | ||
): | ||
if not path.startswith('/'): | ||
raise ValueError("path must start with '/'") | ||
|
@@ -2163,7 +2377,19 @@ def _prepare_sim_args( | |
headers = headers or {} | ||
headers['Content-Type'] = content_type | ||
|
||
if json is not None: | ||
if files or data: | ||
if json: | ||
raise HTTPBadRequest( | ||
description='Cannot process both json and (files or data) args' | ||
) | ||
elif files: | ||
body, headers = _encode_files(files, data) | ||
else: | ||
body = _prepare_data_fields(data, None, True) | ||
headers = headers or {} | ||
headers['Content-Type'] = MEDIA_URLENCODED | ||
|
||
elif json is not None: | ||
body = json_module.dumps(json, ensure_ascii=False) | ||
headers = headers or {} | ||
headers['Content-Type'] = MEDIA_JSON | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,3 +5,4 @@ pytest | |
pyyaml | ||
requests | ||
ujson | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any particular reason behind this newline removal?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know what happened... was that me?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it must have been you, yes. It is coming from you changeset, but it might be some artefact of merging things back and forth on your side, not necessarily something you actively did.