Skip to content

Commit 5b338a0

Browse files
bkdekoningbkdekoning
committed
Fixed django-commons#1682 -- alert user when using file field without proper encoding
1 parent 9c4eb67 commit 5b338a0

File tree

3 files changed

+118
-33
lines changed

3 files changed

+118
-33
lines changed

‎debug_toolbar/panels/templates/panel.py

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
from contextlib import contextmanager
2+
from html.parser import HTMLParser
23
from os.path import normpath
34
from pprint import pformat, saferepr
45

@@ -58,6 +59,31 @@ def _request_context_bind_template(self, template):
5859
RequestContext.bind_template = _request_context_bind_template
5960

6061

62+
class FormParser(HTMLParser):
63+
"""
64+
HTML form parser, used to check for invalid configurations
65+
"""
66+
67+
def __init__(self):
68+
super().__init__()
69+
self.in_form = False
70+
self.current_form = {}
71+
self.forms = []
72+
73+
def handle_starttag(self, tag, attrs):
74+
attrs = dict(attrs)
75+
if tag == "form":
76+
self.in_form = True
77+
self.current_form = {"attrs": attrs, "file_inputs": []}
78+
elif self.in_form and tag == "input" and attrs.get("type") == "file":
79+
self.current_form["file_inputs"].append(attrs)
80+
81+
def handle_endtag(self, tag):
82+
if tag == "form" and self.in_form:
83+
self.forms.append(self.current_form)
84+
self.in_form = False
85+
86+
6187
class TemplatesPanel(Panel):
6288
"""
6389
A panel that lists all templates used during processing of a response.
@@ -177,6 +203,25 @@ def process_context_list(self, context_layers):
177203

178204
return context_list
179205

206+
def check_invalid_file_form_configuration(self, html_content):
207+
parser = FormParser()
208+
parser.feed(html_content)
209+
210+
invalid_forms = []
211+
for form in parser.forms:
212+
if (
213+
form["file_inputs"]
214+
and form["attrs"].get("enctype") != "multipart/form-data"
215+
):
216+
form_id = form["attrs"].get("id", "no form id")
217+
error_message = (
218+
f'Form with id "{form_id}" contains file input but '
219+
f'missing enctype="multipart/form-data".'
220+
)
221+
invalid_forms.append({"form": form, "error_message": error_message})
222+
223+
return invalid_forms
224+
180225
def generate_stats(self, request, response):
181226
template_context = []
182227
for template_data in self.templates:
@@ -211,10 +256,18 @@ def generate_stats(self, request, response):
211256
context_processors = None
212257
template_dirs = []
213258

259+
html_content = response.content.decode(response.charset)
260+
invalid_file_form_configs = self.check_invalid_file_form_configuration(
261+
html_content
262+
)
263+
214264
self.record_stats(
215265
{
216266
"templates": template_context,
217267
"template_dirs": [normpath(x) for x in template_dirs],
218268
"context_processors": context_processors,
269+
"invalid_file_form_configs": [
270+
issue["error_message"] for issue in invalid_file_form_configs
271+
],
219272
}
220273
)

‎debug_toolbar/templates/debug_toolbar/panels/templates.html

Lines changed: 43 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,58 @@
11
{% load i18n %}
22
<h4>{% blocktrans count template_dirs|length as template_count %}Template path{% plural %}Template paths{% endblocktrans %}</h4>
33
{% if template_dirs %}
4-
<ol>
5-
{% for template in template_dirs %}
6-
<li>{{ template }}</li>
7-
{% endfor %}
8-
</ol>
4+
<ol>
5+
{% for template in template_dirs %}
6+
<li>{{ template }}</li>
7+
{% endfor %}
8+
</ol>
99
{% else %}
10-
<p>{% trans "None" %}</p>
10+
<p>{% trans "None" %}</p>
1111
{% endif %}
1212

1313
<h4>{% blocktrans count templates|length as template_count %}Template{% plural %}Templates{% endblocktrans %}</h4>
1414
{% if templates %}
15-
<dl>
16-
{% for template in templates %}
17-
<dt><strong><a class="remoteCall toggleTemplate" href="{% url 'djdt:template_source' %}?template={{ template.template.name }}&amp;template_origin={{ template.template.origin_hash }}">{{ template.template.name|addslashes }}</a></strong></dt>
18-
<dd><samp>{{ template.template.origin_name|addslashes }}</samp></dd>
19-
{% if template.context %}
20-
<dd>
21-
<details>
22-
<summary>{% trans "Toggle context" %}</summary>
23-
<code class="djTemplateContext">{{ template.context }}</code>
24-
</details>
25-
</dd>
26-
{% endif %}
27-
{% endfor %}
28-
</dl>
15+
<dl>
16+
{% for template in templates %}
17+
<dt><strong><a class="remoteCall toggleTemplate" href="{% url 'djdt:template_source' %}?template={{ template.template.name }}&amp;template_origin={{ template.template.origin_hash }}">{{ template.template.name|addslashes }}</a></strong></dt>
18+
<dd><samp>{{ template.template.origin_name|addslashes }}</samp></dd>
19+
{% if template.context %}
20+
<dd>
21+
<details>
22+
<summary>{% trans "Toggle context" %}</summary>
23+
<code class="djTemplateContext">{{ template.context }}</code>
24+
</details>
25+
</dd>
26+
{% endif %}
27+
{% endfor %}
28+
</dl>
2929
{% else %}
30-
<p>{% trans "None" %}</p>
30+
<p>{% trans "None" %}</p>
3131
{% endif %}
3232

3333
<h4>{% blocktrans count context_processors|length as context_processors_count %}Context processor{% plural %}Context processors{% endblocktrans %}</h4>
3434
{% if context_processors %}
35-
<dl>
36-
{% for key, value in context_processors.items %}
37-
<dt><strong>{{ key|escape }}</strong></dt>
38-
<dd>
39-
<details>
40-
<summary>{% trans "Toggle context" %}</summary>
41-
<code class="djTemplateContext">{{ value|escape }}</code>
42-
</details>
43-
</dd>
44-
{% endfor %}
45-
</dl>
35+
<dl>
36+
{% for key, value in context_processors.items %}
37+
<dt><strong>{{ key|escape }}</strong></dt>
38+
<dd>
39+
<details>
40+
<summary>{% trans "Toggle context" %}</summary>
41+
<code class="djTemplateContext">{{ value|escape }}</code>
42+
</details>
43+
</dd>
44+
{% endfor %}
45+
</dl>
46+
{% else %}
47+
<p>{% trans "None" %}</p>
48+
{% endif %}
49+
50+
{% if invalid_file_form_configs %}
51+
<h4>{% blocktrans count invalid_file_form_configs|length as invalid_file_form_configs_count %}Invalid file form configuration{% plural %}Invalid file form configurations{% endblocktrans %}</h4>
52+
<dl>
53+
{% for invalid_file_form_config in invalid_file_form_configs %}
54+
<dt>{{ invalid_file_form_config | escape }}</dt>
55+
{% endfor %}
56+
</dl>
4657
{% else %}
47-
<p>{% trans "None" %}</p>
4858
{% endif %}

‎tests/panels/test_template.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,28 @@ def test_object_with_non_ascii_repr_in_context(self):
6868
self.panel.generate_stats(self.request, response)
6969
self.assertIn("nôt åscíì", self.panel.content)
7070

71+
def test_file_form_without_enctype_multipart_form_data(self):
72+
"""
73+
Test that the panel displays a form invalid message when there is
74+
a file input but encoding not set to multipart/form-data.
75+
"""
76+
test_form = '<form id="test-form"><input type="file"></form>'
77+
result = self.panel.check_invalid_file_form_configuration(test_form)
78+
79+
expected_error = (
80+
'Form with id "test-form" contains file input '
81+
'but missing enctype="multipart/form-data".'
82+
)
83+
self.assertEqual(result[0]["error_message"], expected_error)
84+
85+
def test_file_form_with_enctype_multipart_form_data(self):
86+
test_form = """<form id="test-form" enctype="multipart/form-data">
87+
<input type="file">
88+
</form>"""
89+
result = self.panel.check_invalid_file_form_configuration(test_form)
90+
91+
self.assertEqual(len(result), 0)
92+
7193
def test_insert_content(self):
7294
"""
7395
Test that the panel only inserts content after generate_stats and

0 commit comments

Comments
 (0)