Code

Opened 20 months ago

Closed 20 months ago

Last modified 20 months ago

#19349 closed Bug (fixed)

admin/auth/user throws TypeError on save with invalid email address

Reported by: tim.bowden@… Owned by: nobody
Component: contrib.auth Version: 1.5-alpha-1
Severity: Normal Keywords: user email
Cc: antonbaklanov@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When setting user email to "invalid" address, throws:

TypeError at /admin/auth/user/1/

object of type 'NoneType' has no len()

Exception Location: /usr/local/lib/python2.7/dist-packages/django/contrib/auth/hashers.py in identify_hasher, line 135

1.4 returned page with warning and bad field highlighted.

FWIW, @localhost & @127.0.0.x should be considered valid domains.

Attachments (2)

19349.diff (1.5 KB) - added by foonicorn 20 months ago.
Override ReadOnlyPasswordHashField.bound_data to always return initial value
19349_2.diff (2.9 KB) - added by foonicorn 20 months ago.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 20 months ago by aaugustin

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Could you paste the full trace?

@localhost is another issue, it's tracked in #12817.

comment:2 Changed 20 months ago by tim.bowden@…

Environment:


Request Method: POST
Request URL: http://127.0.0.1:8000/admin/auth/user/1/

Django Version: 1.5a1
Python Version: 2.7.1
Installed Applications:
('django.contrib.auth',
 'django.contrib.contenttypes',
 'django.contrib.sessions',
 'django.contrib.sites',
 'django.contrib.messages',
 'django.contrib.staticfiles',
 'django.contrib.admin',
 'django.contrib.admindocs')
Installed Middleware:
('django.middleware.common.CommonMiddleware',
 'django.contrib.sessions.middleware.SessionMiddleware',
 'django.middleware.csrf.CsrfViewMiddleware',
 'django.contrib.auth.middleware.AuthenticationMiddleware',
 'django.contrib.messages.middleware.MessageMiddleware')


Template error:
In template /usr/local/lib/python2.7/dist-packages/django/contrib/admin/templates/admin/includes/fieldset.html, error at line 19
   object of type 'NoneType' has no len()
   9 :             {% for field in line %}
   10 :                 <div{% if not line.fields|length_is:'1' %} class="field-box{% if field.field.name %} field-{{ field.field.name }}{% endif %}{% if not field.is_readonly and field.errors %} errors{% endif %}"{% endif %}>
   11 :                     {% if not line.fields|length_is:'1' and not field.is_readonly %}{{ field.errors }}{% endif %}
   12 :                     {% if field.is_checkbox %}
   13 :                         {{ field.field }}{{ field.label_tag }}
   14 :                     {% else %}
   15 :                         {{ field.label_tag }}
   16 :                         {% if field.is_readonly %}
   17 :                             <p>{{ field.contents }}</p>
   18 :                         {% else %}
   19 :                              {{ field.field }} 
   20 :                         {% endif %}
   21 :                     {% endif %}
   22 :                     {% if field.field.help_text %}
   23 :                         <p class="help">{{ field.field.help_text|safe }}</p>
   24 :                     {% endif %}
   25 :                 </div>
   26 :             {% endfor %}
   27 :         </div>
   28 :     {% endfor %}
   29 : </fieldset>

Traceback:
File "/usr/local/lib/python2.7/dist-packages/django/core/handlers/base.py" in get_response
  141.                     response = response.render()
File "/usr/local/lib/python2.7/dist-packages/django/template/response.py" in render
  105.             self.content = self.rendered_content
File "/usr/local/lib/python2.7/dist-packages/django/template/response.py" in rendered_content
  82.         content = template.render(context)
File "/usr/local/lib/python2.7/dist-packages/django/template/base.py" in render
  140.             return self._render(context)
File "/usr/local/lib/python2.7/dist-packages/django/template/base.py" in _render
  134.         return self.nodelist.render(context)
File "/usr/local/lib/python2.7/dist-packages/django/template/base.py" in render
  830.                 bit = self.render_node(node, context)
File "/usr/local/lib/python2.7/dist-packages/django/template/debug.py" in render_node
  74.             return node.render(context)
File "/usr/local/lib/python2.7/dist-packages/django/template/loader_tags.py" in render
  124.         return compiled_parent._render(context)
File "/usr/local/lib/python2.7/dist-packages/django/template/base.py" in _render
  134.         return self.nodelist.render(context)
File "/usr/local/lib/python2.7/dist-packages/django/template/base.py" in render
  830.                 bit = self.render_node(node, context)
File "/usr/local/lib/python2.7/dist-packages/django/template/debug.py" in render_node
  74.             return node.render(context)
File "/usr/local/lib/python2.7/dist-packages/django/template/loader_tags.py" in render
  124.         return compiled_parent._render(context)
File "/usr/local/lib/python2.7/dist-packages/django/template/base.py" in _render
  134.         return self.nodelist.render(context)
File "/usr/local/lib/python2.7/dist-packages/django/template/base.py" in render
  830.                 bit = self.render_node(node, context)
File "/usr/local/lib/python2.7/dist-packages/django/template/debug.py" in render_node
  74.             return node.render(context)
File "/usr/local/lib/python2.7/dist-packages/django/template/loader_tags.py" in render
  63.             result = block.nodelist.render(context)
File "/usr/local/lib/python2.7/dist-packages/django/template/base.py" in render
  830.                 bit = self.render_node(node, context)
File "/usr/local/lib/python2.7/dist-packages/django/template/debug.py" in render_node
  74.             return node.render(context)
File "/usr/local/lib/python2.7/dist-packages/django/template/loader_tags.py" in render
  63.             result = block.nodelist.render(context)
File "/usr/local/lib/python2.7/dist-packages/django/template/base.py" in render
  830.                 bit = self.render_node(node, context)
File "/usr/local/lib/python2.7/dist-packages/django/template/debug.py" in render_node
  74.             return node.render(context)
File "/usr/local/lib/python2.7/dist-packages/django/template/defaulttags.py" in render
  188.                         nodelist.append(node.render(context))
File "/usr/local/lib/python2.7/dist-packages/django/template/loader_tags.py" in render
  156.         return self.render_template(self.template, context)
File "/usr/local/lib/python2.7/dist-packages/django/template/loader_tags.py" in render_template
  138.         output = template.render(context)
File "/usr/local/lib/python2.7/dist-packages/django/template/base.py" in render
  140.             return self._render(context)
File "/usr/local/lib/python2.7/dist-packages/django/template/base.py" in _render
  134.         return self.nodelist.render(context)
File "/usr/local/lib/python2.7/dist-packages/django/template/base.py" in render
  830.                 bit = self.render_node(node, context)
File "/usr/local/lib/python2.7/dist-packages/django/template/debug.py" in render_node
  74.             return node.render(context)
File "/usr/local/lib/python2.7/dist-packages/django/template/defaulttags.py" in render
  188.                         nodelist.append(node.render(context))
File "/usr/local/lib/python2.7/dist-packages/django/template/defaulttags.py" in render
  188.                         nodelist.append(node.render(context))
File "/usr/local/lib/python2.7/dist-packages/django/template/defaulttags.py" in render
  284.                 return nodelist.render(context)
File "/usr/local/lib/python2.7/dist-packages/django/template/base.py" in render
  830.                 bit = self.render_node(node, context)
File "/usr/local/lib/python2.7/dist-packages/django/template/debug.py" in render_node
  74.             return node.render(context)
File "/usr/local/lib/python2.7/dist-packages/django/template/defaulttags.py" in render
  284.                 return nodelist.render(context)
File "/usr/local/lib/python2.7/dist-packages/django/template/base.py" in render
  830.                 bit = self.render_node(node, context)
File "/usr/local/lib/python2.7/dist-packages/django/template/debug.py" in render_node
  74.             return node.render(context)
File "/usr/local/lib/python2.7/dist-packages/django/template/debug.py" in render
  87.             output = force_text(output)
File "/usr/local/lib/python2.7/dist-packages/django/utils/encoding.py" in force_text
  99.                 s = s.__unicode__()
File "/usr/local/lib/python2.7/dist-packages/django/forms/forms.py" in __str__
  411.         return self.as_widget()
File "/usr/local/lib/python2.7/dist-packages/django/forms/forms.py" in as_widget
  458.         return widget.render(name, self.value(), attrs=attrs)
File "/usr/local/lib/python2.7/dist-packages/django/contrib/auth/forms.py" in render
  34.                 hasher = identify_hasher(encoded)
File "/usr/local/lib/python2.7/dist-packages/django/contrib/auth/hashers.py" in identify_hasher
  135.     if len(encoded) == 32 and '$' not in encoded:

Exception Type: TypeError at /admin/auth/user/1/
Exception Value: object of type 'NoneType' has no len()
Last edited 20 months ago by lrekucki (previous) (diff)

comment:3 Changed 20 months ago by bak1an

it's reproducible on 1.6.dev 20121123121254 too

comment:4 Changed 20 months ago by bak1an

Looks like this patch solves the issue.
https://github.com/bak1an/django/commit/5050e5d928426a6742b4b58d9040b80da8bf8850.patch

We have no access to initial value when rendering widget within bound form, so lets store it within hidden field.

BTW, in django 1.4 this bug is kind of reproducible too.
No stack traces, but we will get 'None' instead of password hash when submit incorrect email.

comment:5 Changed 20 months ago by bak1an

Here is new patch https://github.com/bak1an/django/commit/ed36d62193db166fe11bb2f9088afa31c4664352.patch
Old one was quick and ugly.

And i'm wondering what should be done with this ticket now?

comment:6 Changed 20 months ago by bak1an

  • Cc antonbaklanov@… added

comment:7 Changed 20 months ago by foonicorn

  • Needs tests set
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

While writing a test for this, I noticed that the whole hash is now in the hidden field. This doesn't seem right since the widget is there to obfuscate the hash.

Now the question is: Should we return to 1.4 behavior where 'None' is displayed on bound forms or find a way to get the hash into the widget without revealing it in html?

Changed 20 months ago by foonicorn

Override ReadOnlyPasswordHashField.bound_data to always return initial value

comment:8 Changed 20 months ago by foonicorn

Here is my first try: Override the field's bound_data so the widget always gets the initial value. Test runs on master and 1.5.x

comment:9 Changed 20 months ago by bak1an

Looks good for me.
Maybe we still need to add something like that to prevent ReadOnlyPasswordHashWidget from throwing exceptions when no value provided:

@@ -27,7 +27,7 @@ class ReadOnlyPasswordHashWidget(forms.Widget):
         encoded = value
         final_attrs = self.build_attrs(attrs)
 
-        if encoded == '' or encoded == UNUSABLE_PASSWORD:
+        if not encoded or encoded == UNUSABLE_PASSWORD:
             summary = mark_safe("<strong>%s</strong>" % ugettext("No password set."))
         else:
             try:

Anyway - problem is fixed now, patch is ok and ready for commit.

Changed 20 months ago by foonicorn

comment:10 Changed 20 months ago by foonicorn

  • Has patch set
  • Needs tests unset
  • Patch needs improvement unset

You are right. I've added this to the patch.

comment:11 Changed 20 months ago by bak1an

  • Triage Stage changed from Accepted to Ready for checkin

it's ready

comment:12 Changed 20 months ago by jezdez

Yup, LGTM

comment:13 Changed 20 months ago by Claude Paroz <claude@…>

  • Resolution set to fixed
  • Status changed from new to closed

In a0cd6dd11e81cd0e229d6d109524c8a7e52bc9e1:

Fixed #19349 -- Fixed re-rendering of ReadOnlyPasswordHashWidget

Thanks tim.bowden at mapforge.com.au for the report, Andreas Hug
for the patch and Anton Baklanov for the review.

comment:14 Changed 20 months ago by Claude Paroz <claude@…>

In 676e4d5497955bd16b06411c02875683bc312621:

[1.5.x] Fixed #19349 -- Fixed re-rendering of ReadOnlyPasswordHashWidget

Thanks tim.bowden at mapforge.com.au for the report, Andreas Hug
for the patch and Anton Baklanov for the review.
Backport of a0cd6dd11 from master.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.