Regression in Forms: KeyError if trying to access nonvalid input from required field

[jonescb]

I discovered that I'm getting a KeyError when I try to access a blank required field from the clean() method of the Form.

Something like this will trigger the error:

class TestForm(forms.Form)
    name = forms.CharField()

    def clean(self):
        super(forms.Form, self).clean()
        name = self.cleaned_data['name'] # this is the offending line
        return self.cleaned_data

If you display that form in a template and the user enters nothing into the field, it causes a KeyError which leads to a 500 error. This same code used to work in Django 1.2, but it doesn't work in 1.3 anymore.

If you set the field to required=False, it does work.
The obvious workaround is to wrap the offending line in a try/except.

The previous functionality was that Django would display an error to the user telling them that the field is required if you configured your templates to display the errors. It still displays that error if you don't try to access keys from self.cleaned_data.

This also happens with EmailField if you enter an invalid email address. I didn't test any other fields though.

[jonescb]

Sorry, I messed up the formatting of the code in clean(), and I can't edit it.
I'll just redo it.

class TestForm(forms.Form)
    name = forms.CharField?()
    def clean(self):
        super(forms.Form, self).clean()
        name = self.cleaned_data['name'] # this is the offending line 
        return self.cleaned_data

[Karen Tracey]

I notice you say "something like this" which implies you have simplified your actual case. The code you have posted doesn't work in 1.2 either...1.2 raises a KeyError on exactly the line you note. Fields that have not successfully passed individual field validation have never been available in cleaned_data (what would be there...the data was determined to be invalid already?) when the form-level clean is called. You'll need to get an example closer to the actual case you have encountered that works on 1.2 and fails on 1.3 for someone to help identify what may have changed.

[jonescb]

Ok, you're right 1.2 behaves the same way, so it's not a regression, but I think it could be handled better. At least for CharFields and fields that derive CharField, cleaned_data could default to an empty string. Other fields could possibly default to other values. AFAIK, a blank string won't raise an exception during any string processing just because it's blank. If the field is set to required, Django should set the form as being invalid so that a custom clean() written by an amateur like myself that wasn't expecting an empty field won't end up with a 500 error and it can be handled more gracefully. Custom clean()'s might have to call super.clean() to do this I believe.

Django already does this for required=False. Blank CharFields yield an empty string in cleaned_data.
To me it seems like a strange behavior for Django to give a nice error message for invalid fields, but if your clean() method has to process a field, it will crash unless you catch the exception. Maybe I'm just rationalizing my mistake in not catching the exception, but I think it would be more consistent and make life easier for developers if Django set a blank or default value.

I'll leave the ticket as closed because I doubt anyone would agree with me.

[Karen Tracey]

Note it's quite easy to avoid a KeyError in the case of a required field that has not been specified: ​http://docs.python.org/library/stdtypes.html#dict.get You do not have to riddle your code which try/excepts to handle these cases gracefully in your custom clean method.