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

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

Merged
95 changes: 95 additions & 0 deletions debug_toolbar/panels/alerts.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
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 = []

def handle_starttag(self, tag, attrs):
attrs = dict(attrs)
if tag == "form":
self.in_form = True
self.current_form = {
"file_form": False,
"form_attrs": attrs,
"submit_element_attrs": [],
}
elif self.in_form and tag == "input" and attrs.get("type") == "file":
self.current_form["file_form"] = True
elif self.in_form and (
(tag == "input" and attrs.get("type") in {"submit", "image"})
or tag == "button"
):
self.current_form["submit_element_attrs"].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.
"""

title = _("Alerts")

template = "debug_toolbar/panels/alerts.html"

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.issues = []
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if it'd be better to set this in enable_instrumentation instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I trust your call on this, but have to admit that I do not fully understand the enable_instrumentation method. What would the advantage be of setting it there? Looking at the other panels, I commonly see lists being initialized in __init__.


@property
def nav_subtitle(self):
if self.issues:
bkdekoning marked this conversation as resolved.
Show resolved Hide resolved
issue_text = "issue" if len(self.issues) == 1 else "issues"
return f"{len(self.issues)} {issue_text} found"
else:
return ""

def add_issue(self, issue):
self.issues.append(issue)
bkdekoning marked this conversation as resolved.
Show resolved Hide resolved

def check_invalid_file_form_configuration(self, html_content):
bkdekoning marked this conversation as resolved.
Show resolved Hide resolved
parser = FormParser()
parser.feed(html_content)

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
):
form_id = form["form_attrs"].get("id", "no form id")
issue = (
f'Form with id "{form_id}" contains file input but '
"does not have multipart/form-data encoding."
)
self.add_issue({"issue": issue})
return self.issues

def generate_stats(self, request, response):
html_content = response.content.decode(response.charset)
self.check_invalid_file_form_configuration(html_content)

# Further issue checks can go here

# Write all issues to record_stats
self.record_stats({"issues": self.issues})
1 change: 1 addition & 0 deletions debug_toolbar/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ def get_config():


PANELS_DEFAULTS = [
"debug_toolbar.panels.alerts.AlertsPanel",
"debug_toolbar.panels.history.HistoryPanel",
"debug_toolbar.panels.versions.VersionsPanel",
"debug_toolbar.panels.timer.TimerPanel",
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 issues %}
<h4>{% trans "Issues found" %}</h4>
{% for issue in issues %}
<ul>
<li>{{ issue.issue }}</li>
</ul>
{% endfor %}
{% else %}
<p>No issues found.</p>
{% 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``.

Expand Down
1 change: 1 addition & 0 deletions docs/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ included in the toolbar. It works like Django's ``MIDDLEWARE`` setting. The
default value is::

DEBUG_TOOLBAR_PANELS = [
'debug_toolbar.panels.alerts.AlertsPanel',
'debug_toolbar.panels.history.HistoryPanel',
'debug_toolbar.panels.versions.VersionsPanel',
'debug_toolbar.panels.timer.TimerPanel',
Expand Down
9 changes: 9 additions & 0 deletions docs/panels.rst
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,15 @@ 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 issues. Currently, the only
issue it checks for is the encoding of a form that takes a file input not
being set to ``multipart/form-data``.
bkdekoning marked this conversation as resolved.
Show resolved Hide resolved

History
~~~~~~~

Expand Down
57 changes: 57 additions & 0 deletions tests/panels/test_alerts.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
from django.http import HttpResponse
from django.template import Context, Template

from ..base import BaseTestCase


class AlertsPanelTestCase(BaseTestCase):
panel_id = "AlertsPanel"

def test_issue_warning_display(self):
"""
Test that the panel (does not) display[s] a warning when there are
(no) issues.
"""
self.panel.issues = 0
nav_subtitle = self.panel.nav_subtitle
self.assertNotIn("issues found", nav_subtitle)

self.panel.issues = ["Issue 1", "Issue 2"]
nav_subtitle = self.panel.nav_subtitle
self.assertIn("2 issues found", 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 multipart/form-data encoding."
)
self.assertEqual(result[0]["issue"], 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_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(
"Form with id &quot;test-form&quot; contains file input "
"but does not have multipart/form-data encoding.",
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 @@ -67,6 +67,7 @@ def test_urls(self):
@override_settings(DEBUG=True)
class HistoryViewsTestCase(IntegrationTestCase):
PANEL_KEYS = {
"AlertsPanel",
"VersionsPanel",
"TimerPanel",
"SettingsPanel",
Expand Down