Skip to content

Commit

Permalink
Fixed #1682 -- alert user when using file field without proper encodi…
Browse files Browse the repository at this point in the history
…ng (#1933)

* Fixed issue #1682 -- alert user when using file field without proper encoding

* Changed issues to alerts, added docstring to check_invalid_... function, changed call in nav_subtitle to get_stats

* Update AlertsPanel documentation to list pre-defined alert cases

* added check for file inputs that directly reference a form, including tests; also added tests for setting encoding in submit input type

* Update the alert messages to be on the panel as a map.

This also explicitly mentions what attribute the form needs in
the message.

* Expose a page in the example app that triggers the alerts panel.

---------

Co-authored-by: Tim Schilling <[email protected]>
  • Loading branch information
bkdekoning and tim-schilling authored Jul 4, 2024
1 parent 325ea19 commit d0f09b3
Show file tree
Hide file tree
Showing 11 changed files with 297 additions and 0 deletions.
148 changes: 148 additions & 0 deletions debug_toolbar/panels/alerts.py
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"]
)
):
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})
1 change: 1 addition & 0 deletions debug_toolbar/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ def get_config():
"debug_toolbar.panels.sql.SQLPanel",
"debug_toolbar.panels.staticfiles.StaticFilesPanel",
"debug_toolbar.panels.templates.TemplatesPanel",
"debug_toolbar.panels.alerts.AlertsPanel",
"debug_toolbar.panels.cache.CachePanel",
"debug_toolbar.panels.signals.SignalsPanel",
"debug_toolbar.panels.redirects.RedirectsPanel",
Expand Down
12 changes: 12 additions & 0 deletions debug_toolbar/templates/debug_toolbar/panels/alerts.html
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 %}
2 changes: 2 additions & 0 deletions docs/changes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ Change log
Pending
-------

* Added alert panel with warning when form is using file fields
without proper encoding type.
* Fixed overriding font-family for both light and dark themes.
* Restored compatibility with ``iptools.IpRangeList``.
* Limit ``E001`` check to likely error cases when the
Expand Down
1 change: 1 addition & 0 deletions docs/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ default value is::
'debug_toolbar.panels.sql.SQLPanel',
'debug_toolbar.panels.staticfiles.StaticFilesPanel',
'debug_toolbar.panels.templates.TemplatesPanel',
'debug_toolbar.panels.alerts.AlertsPanel',
'debug_toolbar.panels.cache.CachePanel',
'debug_toolbar.panels.signals.SignalsPanel',
'debug_toolbar.panels.redirects.RedirectsPanel',
Expand Down
11 changes: 11 additions & 0 deletions docs/panels.rst
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,17 @@ Default built-in panels

The following panels are enabled by default.

Alerts
~~~~~~~

.. class:: debug_toolbar.panels.alerts.AlertsPanel

This panel shows alerts for a set of pre-defined cases:

- Alerts when the response has a form without the
``enctype="multipart/form-data"`` attribute and the form contains
a file input.

History
~~~~~~~

Expand Down
14 changes: 14 additions & 0 deletions example/templates/bad_form.html
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>
1 change: 1 addition & 0 deletions example/templates/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ <h1>Index of Tests</h1>
<li><a href="/prototype/">Prototype 1.7.3.0</a></li>
<li><a href="{% url 'turbo' %}">Hotwire Turbo</a></li>
<li><a href="{% url 'htmx' %}">htmx</a></li>
<li><a href="{% url 'bad_form' %}">Bad form</a></li>
</ul>
<p><a href="/admin/">Django Admin</a></p>
{% endcache %}
Expand Down
5 changes: 5 additions & 0 deletions example/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@

urlpatterns = [
path("", TemplateView.as_view(template_name="index.html"), name="home"),
path(
"bad-form/",
TemplateView.as_view(template_name="bad_form.html"),
name="bad_form",
),
path("jquery/", TemplateView.as_view(template_name="jquery/index.html")),
path("mootools/", TemplateView.as_view(template_name="mootools/index.html")),
path("prototype/", TemplateView.as_view(template_name="prototype/index.html")),
Expand Down
101 changes: 101 additions & 0 deletions tests/panels/test_alerts.py
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 &quot;test-form&quot; contains file input, "
"but does not have the attribute enctype=&quot;multipart/form-data&quot;.",
self.panel.content,
)
1 change: 1 addition & 0 deletions tests/panels/test_history.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ class HistoryViewsTestCase(IntegrationTestCase):
"SQLPanel",
"StaticFilesPanel",
"TemplatesPanel",
"AlertsPanel",
"CachePanel",
"SignalsPanel",
"ProfilingPanel",
Expand Down

0 comments on commit d0f09b3

Please sign in to comment.