Skip to content
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

Submit an empty file when leaving a file input blank #270

Merged
merged 2 commits into from
Apr 3, 2019
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/ChangeLog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ Bug fixes
``disabled`` attribute from the element in the :class:`~mechanicalsoup.Form`
object directly.
[`#248 <https://github.com/MechanicalSoup/MechanicalSoup/issues/248>`__]
* Upon submitting a form containing a file input field without uploading one,
an empty filename & content will be sent in compliance with regular web
browser behavior.
[`#250 <https://github.com/MechanicalSoup/MechanicalSoup/issues/250>`__]

Version 0.11
============
Expand Down
13 changes: 8 additions & 5 deletions mechanicalsoup/browser.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,11 +180,14 @@ def _request(self, form, url=None, **kwargs):
# in browsers, file upload only happens if the form
# (or submit button) enctype attribute is set to
# "multipart/form-data". We don't care, simplify.
if not value:
continue
if isinstance(value, string_types):
value = open(value, "rb")
files[name] = value
filename = value
if filename != "" and isinstance(filename, string_types):
content = open(filename, "rb")
else:
content = ""
# If value is the empty string, we still pass it for
# consistency with browsers (see #250).
files[name] = (filename, content)
else:
data.append((name, value))

Expand Down
38 changes: 29 additions & 9 deletions tests/test_browser.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import sys
from bs4 import BeautifulSoup
import tempfile
import os
from requests.cookies import RequestsCookieJar
import pytest

Expand Down Expand Up @@ -56,7 +57,6 @@ def test__request(httpbin):
<p><input type=checkbox name="topping" value="onion" checked>Onion</p>
<p><input type=checkbox name="topping" value="mushroom">Mushroom</p>
</fieldset>
<input name="pic" type="FiLe">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Every commit must pass tests (bisectable history), so this change should come before the commit that actually changes the behavior.

About the commit message: no line longer than 80 characters (preferably <= 72 characters) please.

<select name="shape">
<option value="round">Round</option>
<option value="square" selected>Square</option>
Expand All @@ -81,20 +81,25 @@ def test__request(httpbin):
"Content-Type"]


def test__request_file(httpbin):
@pytest.mark.parametrize('expected_content, set_value', [
(b":-)", True),
(b"", False)
])
senabIsShort marked this conversation as resolved.
Show resolved Hide resolved
def test__request_file(httpbin, expected_content, set_value):
form_html = """
<form method="post" action="{}/post">
<input name="pic" type="file" />
</form>
""".format(httpbin.url)
form = BeautifulSoup(form_html, "lxml").form

# create a temporary file for testing file upload
pic_path = tempfile.mkstemp()[1]
with open(pic_path, "w") as f:
f.write(":-)")
if set_value:
# create a temporary file for testing file upload
pic_filedescriptor, pic_path = tempfile.mkstemp()
os.write(pic_filedescriptor, expected_content)
os.close(pic_filedescriptor)

form.find("input", {"name": "pic"})["value"] = pic_path
form.find("input", {"name": "pic"})["value"] = pic_path

browser = mechanicalsoup.Browser()
response = browser._request(form)
Expand All @@ -103,14 +108,29 @@ def test__request_file(httpbin):
found = False
for key, value in response.json().items():
if key == "files":
assert value["pic"] == ":-)"
found = True
if "pic" in value:
if value["pic"].encode() == expected_content:
# If pic is found twice, an error will occur
assert not found
found = True
# One would expect to find "pic" in files, but as of writing,
# httpbin puts it in form when the filename is empty:
elif key == "form":
if "pic" in value:
if value["pic"].encode() == expected_content:
assert not found
found = True
assert b"filename=\"\"" in response.request.body
else:
assert (value is None) or ("pic" not in value)

assert found
assert "multipart/form-data" in response.request.headers["Content-Type"]

# In case we created a file for upload, we need to close & delete it
if set_value:
os.remove(pic_path)


def test__request_select_none(httpbin):
"""Make sure that a <select> with no options selected
Expand Down