Improve max_length validation

[Tim Valenta]

Absentmindedly I wrote a line like the following in a model, where I accidentally contracted the definition of max_length and help_text into a single keyword arg:

    field = models.CharField(max_length="This field is really cool. Not to mention this message.")

My accidental value for max_length is not being validated to check and see if it is indeed an integer for use in a MySQL varchar field definition. The SQL created during ./manage.py syncdb is as follows, which causes the error also following:

    `field` varchar(This field is really cool. Not to mention this message.) NOT NULL

    _mysql_exceptions.ProgrammingError: (1064, "You have an error in your SQL syntax;
    check the manual that corresponds to your MySQL server version for the right
    syntax to use near 'This field is really cool. Not to mention this message.) NOT NULL\n)'
    at line 4")

Improved validation might simply try casting the value of max_length to an integer before proceeding.

[Oroku Saki]

If I have a max_length=5 in a Field, and I use firebug to disable the HTML maxlength='5' attribute on the page, and enter 10 characters, then hit save, it'll give me a "KeyError" of some sort, instead of using application level validation to see if the text is indeed 5 characters long. You shouldn't have a server error just because some nerd with Firebug hacked your page, right?

[Luke Plant]

Replying to orokusaki:

If I have a max_length=5 in a Field, and I use firebug to disable the HTML maxlength='5' attribute on the page, and enter 10 characters, then hit save, it'll give me a "KeyError" of some sort, instead of using application level validation to see if the text is indeed 5 characters long.

FieldTests.test_charfield_2 in tests/regressiontests/forms/fields.py indicates that you are incorrect about that issue, or you are not using the application level validation that is available. This bug is about another issue anyway.

[josh]

Add CharField max_length validation to mange.py 'validate' command.

[James Bennett]

1.2 is feature-frozen, moving this feature request off the milestone.

[Tim Valenta]

Replying to ubernostrum:

1.2 is feature-frozen, moving this feature request off the milestone.

Funny, I see this as a bug, not a feature addition. But okay. I submitted it a while back for this very reason :)

[Russell Keith-Magee]

I agree - this is a bug that should be fixed. Chalk this one up to friendly fire.

Patch still requires tests. See the invalid_models regression test for how to do it.

[Tim Valenta]

Replying to russellm:

Patch still requires tests. See the invalid_models regression test for how to do it.

For my own future reference, does the fact that there's an assigned user suggest who the author of the test should be? I've kept my distance tentatively since it was picked up by someone the day after I submitted it. I haven't seen direct reference to such a question in the documentation on contributing, unless I missed that somehow.

[Russell Keith-Magee]

Ultimately, we'll accept a patch from whoever submits it, regardless of whether they have officially claimed the ticket.

The ticket assignment just exists as an attempt to minimize duplicated effort. During a sprint, assigning a bug helps ensure that multiple people aren't working on the same issue; on larger features, it announces that someone is working on the problem.

@josh claimed this ticket over a month ago, and hasn't updated the patch since. Given that this is a small bug with a simple fix, it's probably safe to assume he isn't actively working on it.

The documentation (such as it is) for this process is ​here.

[Leo Shklovskii]

updated patch with tests

[Leo Shklovskii]

Updated patch with tests. I added a check for wrong type argument as well as invalid int.

[Russell Keith-Magee]

(In [12768]) Fixed #12404 -- Improved model validation for CharField and DecimalField. Thanks to tiliv for the report, josh for the patch, and Leo for the tests.

[Russell Keith-Magee]

(In [12769]) [1.1.X] Fixed #12404 -- Improved model validation for CharField and DecimalField. Thanks to tiliv for the report, josh for the patch, and Leo for the tests.

Backport of r12768 from trunk.

[Jacob]

Milestone 1.2 deleted