#12404 closed (fixed)
Improve max_length validation
Reported by: | Tim Valenta | Owned by: | josh |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Keywords: | max_length, validation | |
Cc: | Leo Shklovskii | 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
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.
Attachments (2)
Change History (16)
follow-up: 2 comment:1 by , 15 years ago
comment:2 by , 15 years ago
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.
by , 15 years ago
Attachment: | 12404_patch.diff added |
---|
Add CharField max_length validation to mange.py 'validate' command.
comment:3 by , 15 years ago
Has patch: | set |
---|---|
Owner: | changed from | to
Status: | new → assigned |
comment:4 by , 15 years ago
Needs tests: | set |
---|---|
Triage Stage: | Unreviewed → Accepted |
follow-up: 6 comment:5 by , 15 years ago
milestone: | 1.2 |
---|
1.2 is feature-frozen, moving this feature request off the milestone.
comment:6 by , 15 years ago
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 :)
follow-up: 8 comment:7 by , 15 years ago
milestone: | → 1.2 |
---|
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.
comment:8 by , 15 years ago
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.
comment:9 by , 15 years ago
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.
comment:10 by , 15 years ago
Cc: | added |
---|---|
Needs tests: | unset |
Updated patch with tests. I added a check for wrong type argument as well as invalid int.
comment:11 by , 15 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:12 by , 15 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
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?