canvasapi
canvasapi copied to clipboard
combine_kwargs erroneously parses `file` argument
Describe the bug
In https://github.com/ucfopen/canvasapi/pull/222, we added support for passing the files
dict sent by requests.post
directly:
# Grab file from data.
files = None
for field, value in data:
if field == "file":
if isinstance(value, dict):
files = value
else:
files = {"file": value}
break
# Remove file entry from data.
data[:] = [tup for tup in data if tup[0] != "file"]
return self._session.post(url, headers=headers, data=data, files=files)
Unfortunately, this has a strange interaction with combine_kwargs
, where combine_kwargs
will iterate over the value
and spit out something like this:
('author', 3889181),
('message', 'this is a test'),
('user_can_see_posts', 'true'),
('published', 'true'),
('file[attachment][]', b'#!/usr/bin/env python\n'),
('file[attachment][]', b'import os\n'),
('file[attachment][]', b'import sys\n'),
('file[attachment][]', b'\n'),
('file[attachment][]', b'if __name__ == "__main__":\n'),
('file[attachment][]',
b' os.environ.setdefault("DJANGO_SETTINGS_MODULE", "config.settings.loc'
b'al")\n'),
('file[attachment][]', b'\n'),
('file[attachment][]', b' try:\n'),
('file[attachment][]',
b' from django.core.management import execute_from_command_line\n'),
('file[attachment][]', b' except ImportError:\n'),
('file[attachment][]',
b' # The above import may fail for some other reason. Ensure that t'
b'he\n'),
('file[attachment][]',
b' # issue is really that Django is missing to avoid masking other\n'),
('file[attachment][]', b' # exceptions on Python 2.\n'),
('file[attachment][]', b' try:\n'),
('file[attachment][]', b' import django # noqa\n'),
('file[attachment][]', b' except ImportError:\n'),
('file[attachment][]', b' raise ImportError(\n'),
('file[attachment][]',
b' "Couldn\'t import Django. Are you sure it\'s installed and'
b' "\n'),
('file[attachment][]',
b' "available on your PYTHONPATH environment variable? Did '
b'you "\n'),
('file[attachment][]',
b' "forget to activate a virtual environment?"\n'),
('file[attachment][]', b' )\n'),
('file[attachment][]', b'\n'),
('file[attachment][]', b' raise\n'),
('file[attachment][]', b'\n'),
('file[attachment][]',
b' # This allows easy placement of apps within the interior\n'),
('file[attachment][]', b' # soulpatch directory.\n'),
('file[attachment][]',
b' current_path = os.path.dirname(os.path.abspath(__file__))\n'),
('file[attachment][]',
b' sys.path.append(os.path.join(current_path, "soulpatch"))\n'),
('file[attachment][]', b'\n'),
('file[attachment][]', b' execute_from_command_line(sys.argv)\n')]
To Reproduce
Steps to reproduce the behavior:
-
On any endpoint that accepts a file upload, pass the file handler as part of a dictionary with the keyword argument
file
:create_discussion_topic(file={"attachment": open("some-attachment.txt")})
-
Watch as it doesn't work.
-
Bonus: Enable logging to see the insane output generated by
combine_kwargs
.
Expected behavior
Passing a dictionary named files
should allow me to send the parameter names expected by Canvas with the files to upload. In the case of course.create_discussion_topic()
, the parameter name is attachment
.
Environment information (please complete the following information)
- Python version (
python --version
): 3.8.0a2+ (though it is present in all versions) - CanvasAPI version (
pip show canvasapi
): 0.14 (introduced in 0.12)
Additional context
We may want to consider exposing a keyword argument like _files
or something that will always take the files dictionary, rather than using a name like file
which could potentially conflict with required arguments in various endpoints.