-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fixed #1682 -- alert user when using file field without proper encoding #1933
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
tim-schilling
merged 8 commits into
django-commons:main
from
bkdekoning:issue_1682_alerts_panel
Jul 4, 2024
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
97fa2e5
Fixed issue #1682 -- alert user when using file field without proper …
bkdekoning 00dc765
Changed issues to alerts, added docstring to check_invalid_... functi…
bkdekoning 0bbb66d
Update AlertsPanel documentation to list pre-defined alert cases
bkdekoning c332a6f
added check for file inputs that directly reference a form, including…
bkdekoning d89ee73
Update the alert messages to be on the panel as a map.
tim-schilling 11872eb
Expose a page in the example app that triggers the alerts panel.
tim-schilling e8dd720
Move the alerts panel below the more used panels.
tim-schilling 28e15a7
Missed a period.
tim-schilling File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,148 @@ | ||
from html.parser import HTMLParser | ||
|
||
from django.utils.translation import gettext_lazy as _ | ||
|
||
from debug_toolbar.panels import Panel | ||
|
||
|
||
class FormParser(HTMLParser): | ||
""" | ||
HTML form parser, used to check for invalid configurations of forms that | ||
take file inputs. | ||
""" | ||
|
||
def __init__(self): | ||
super().__init__() | ||
self.in_form = False | ||
self.current_form = {} | ||
self.forms = [] | ||
self.form_ids = [] | ||
self.referenced_file_inputs = [] | ||
|
||
def handle_starttag(self, tag, attrs): | ||
attrs = dict(attrs) | ||
if tag == "form": | ||
self.in_form = True | ||
form_id = attrs.get("id") | ||
if form_id: | ||
self.form_ids.append(form_id) | ||
self.current_form = { | ||
"file_form": False, | ||
"form_attrs": attrs, | ||
"submit_element_attrs": [], | ||
} | ||
elif ( | ||
self.in_form | ||
and tag == "input" | ||
and attrs.get("type") == "file" | ||
and (not attrs.get("form") or attrs.get("form") == "") | ||
): | ||
self.current_form["file_form"] = True | ||
elif ( | ||
self.in_form | ||
and ( | ||
(tag == "input" and attrs.get("type") in {"submit", "image"}) | ||
or tag == "button" | ||
) | ||
and (not attrs.get("form") or attrs.get("form") == "") | ||
): | ||
self.current_form["submit_element_attrs"].append(attrs) | ||
elif tag == "input" and attrs.get("form"): | ||
self.referenced_file_inputs.append(attrs) | ||
|
||
def handle_endtag(self, tag): | ||
if tag == "form" and self.in_form: | ||
self.forms.append(self.current_form) | ||
self.in_form = False | ||
|
||
|
||
class AlertsPanel(Panel): | ||
""" | ||
A panel to alert users to issues. | ||
""" | ||
|
||
messages = { | ||
"form_id_missing_enctype": _( | ||
'Form with id "{form_id}" contains file input, but does not have the attribute enctype="multipart/form-data".' | ||
), | ||
"form_missing_enctype": _( | ||
'Form contains file input, but does not have the attribute enctype="multipart/form-data".' | ||
), | ||
"input_refs_form_missing_enctype": _( | ||
'Input element references form with id "{form_id}", but the form does not have the attribute enctype="multipart/form-data".' | ||
), | ||
} | ||
|
||
title = _("Alerts") | ||
|
||
template = "debug_toolbar/panels/alerts.html" | ||
|
||
def __init__(self, *args, **kwargs): | ||
super().__init__(*args, **kwargs) | ||
self.alerts = [] | ||
|
||
@property | ||
def nav_subtitle(self): | ||
alerts = self.get_stats()["alerts"] | ||
if alerts: | ||
alert_text = "alert" if len(alerts) == 1 else "alerts" | ||
return f"{len(alerts)} {alert_text}" | ||
else: | ||
return "" | ||
|
||
def add_alert(self, alert): | ||
self.alerts.append(alert) | ||
|
||
def check_invalid_file_form_configuration(self, html_content): | ||
""" | ||
Inspects HTML content for a form that includes a file input but does | ||
not have the encoding type set to multipart/form-data, and warns the | ||
user if so. | ||
""" | ||
parser = FormParser() | ||
parser.feed(html_content) | ||
|
||
# Check for file inputs directly inside a form that do not reference | ||
# any form through the form attribute | ||
for form in parser.forms: | ||
if ( | ||
form["file_form"] | ||
and form["form_attrs"].get("enctype") != "multipart/form-data" | ||
and not any( | ||
elem.get("formenctype") == "multipart/form-data" | ||
for elem in form["submit_element_attrs"] | ||
) | ||
tim-schilling marked this conversation as resolved.
Show resolved
Hide resolved
|
||
): | ||
if form_id := form["form_attrs"].get("id"): | ||
alert = self.messages["form_id_missing_enctype"].format( | ||
form_id=form_id | ||
) | ||
else: | ||
alert = self.messages["form_missing_enctype"] | ||
self.add_alert({"alert": alert}) | ||
|
||
# Check for file inputs that reference a form | ||
form_attrs_by_id = { | ||
form["form_attrs"].get("id"): form["form_attrs"] for form in parser.forms | ||
} | ||
|
||
for attrs in parser.referenced_file_inputs: | ||
form_id = attrs.get("form") | ||
if form_id and attrs.get("type") == "file": | ||
form_attrs = form_attrs_by_id.get(form_id) | ||
if form_attrs and form_attrs.get("enctype") != "multipart/form-data": | ||
alert = self.messages["input_refs_form_missing_enctype"].format( | ||
form_id=form_id | ||
) | ||
self.add_alert({"alert": alert}) | ||
|
||
return self.alerts | ||
|
||
def generate_stats(self, request, response): | ||
html_content = response.content.decode(response.charset) | ||
self.check_invalid_file_form_configuration(html_content) | ||
|
||
# Further alert checks can go here | ||
|
||
# Write all alerts to record_stats | ||
self.record_stats({"alerts": self.alerts}) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
{% load i18n %} | ||
|
||
{% if alerts %} | ||
<h4>{% trans "Alerts found" %}</h4> | ||
{% for alert in alerts %} | ||
<ul> | ||
<li>{{ alert.alert }}</li> | ||
</ul> | ||
{% endfor %} | ||
{% else %} | ||
<h4>{% trans "No alerts found" %}</h4> | ||
{% endif %} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
{% load cache %} | ||
<!DOCTYPE html> | ||
<html> | ||
<head> | ||
<meta http-equiv="content-type" content="text/html; charset=utf-8"> | ||
<title>Bad form</title> | ||
</head> | ||
<body> | ||
<h1>Bad form test</h1> | ||
<form> | ||
<input type="file" name="file" /> | ||
</form> | ||
</body> | ||
</html> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
from django.http import HttpResponse | ||
from django.template import Context, Template | ||
|
||
from ..base import BaseTestCase | ||
|
||
|
||
class AlertsPanelTestCase(BaseTestCase): | ||
panel_id = "AlertsPanel" | ||
|
||
def test_alert_warning_display(self): | ||
""" | ||
Test that the panel (does not) display[s] an alert when there are | ||
(no) problems. | ||
""" | ||
self.panel.record_stats({"alerts": []}) | ||
self.assertNotIn("alerts", self.panel.nav_subtitle) | ||
|
||
self.panel.record_stats({"alerts": ["Alert 1", "Alert 2"]}) | ||
self.assertIn("2 alerts", self.panel.nav_subtitle) | ||
|
||
def test_file_form_without_enctype_multipart_form_data(self): | ||
""" | ||
Test that the panel displays a form invalid message when there is | ||
a file input but encoding not set to multipart/form-data. | ||
""" | ||
test_form = '<form id="test-form"><input type="file"></form>' | ||
result = self.panel.check_invalid_file_form_configuration(test_form) | ||
expected_error = ( | ||
'Form with id "test-form" contains file input, ' | ||
'but does not have the attribute enctype="multipart/form-data".' | ||
) | ||
self.assertEqual(result[0]["alert"], expected_error) | ||
self.assertEqual(len(result), 1) | ||
|
||
def test_file_form_no_id_without_enctype_multipart_form_data(self): | ||
""" | ||
Test that the panel displays a form invalid message when there is | ||
a file input but encoding not set to multipart/form-data. | ||
|
||
This should use the message when the form has no id. | ||
""" | ||
test_form = '<form><input type="file"></form>' | ||
result = self.panel.check_invalid_file_form_configuration(test_form) | ||
expected_error = ( | ||
"Form contains file input, but does not have " | ||
'the attribute enctype="multipart/form-data".' | ||
) | ||
self.assertEqual(result[0]["alert"], expected_error) | ||
self.assertEqual(len(result), 1) | ||
|
||
def test_file_form_with_enctype_multipart_form_data(self): | ||
test_form = """<form id="test-form" enctype="multipart/form-data"> | ||
<input type="file"> | ||
</form>""" | ||
result = self.panel.check_invalid_file_form_configuration(test_form) | ||
|
||
self.assertEqual(len(result), 0) | ||
|
||
def test_file_form_with_enctype_multipart_form_data_in_button(self): | ||
test_form = """<form id="test-form"> | ||
<input type="file"> | ||
<input type="submit" formenctype="multipart/form-data"> | ||
</form>""" | ||
result = self.panel.check_invalid_file_form_configuration(test_form) | ||
|
||
self.assertEqual(len(result), 0) | ||
|
||
def test_referenced_file_input_without_enctype_multipart_form_data(self): | ||
test_file_input = """<form id="test-form"></form> | ||
<input type="file" form = "test-form">""" | ||
result = self.panel.check_invalid_file_form_configuration(test_file_input) | ||
|
||
expected_error = ( | ||
'Input element references form with id "test-form", ' | ||
'but the form does not have the attribute enctype="multipart/form-data".' | ||
) | ||
self.assertEqual(result[0]["alert"], expected_error) | ||
self.assertEqual(len(result), 1) | ||
|
||
def test_referenced_file_input_with_enctype_multipart_form_data(self): | ||
test_file_input = """<form id="test-form" enctype="multipart/form-data"> | ||
</form> | ||
<input type="file" form = "test-form">""" | ||
result = self.panel.check_invalid_file_form_configuration(test_file_input) | ||
|
||
self.assertEqual(len(result), 0) | ||
|
||
def test_integration_file_form_without_enctype_multipart_form_data(self): | ||
t = Template('<form id="test-form"><input type="file"></form>') | ||
c = Context({}) | ||
rendered_template = t.render(c) | ||
response = HttpResponse(content=rendered_template) | ||
|
||
self.panel.generate_stats(self.request, response) | ||
|
||
self.assertIn("1 alert", self.panel.nav_subtitle) | ||
self.assertIn( | ||
"Form with id "test-form" contains file input, " | ||
"but does not have the attribute enctype="multipart/form-data".", | ||
self.panel.content, | ||
) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.