Opened 13 years ago
Closed 13 years ago
#16157 closed New feature (wontfix)
Make sure that fields that are presented as single-line are validated as such
Reported by: | tkolar | Owned by: | nobody |
---|---|---|---|
Component: | Forms | Version: | dev |
Severity: | Normal | Keywords: | CharField multiline validator |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
This applies to fields like CharField
that are presented as <input type="text">
by default.
As a developer, it is easy to overlook the fact that it is nonetheless possible to submit multiline data to such a field, for instance by creating a custom form, or by manipulating the original form. If the assumption that entries for this field will always be single-line is erroneously made, this is a hard-to-find bug at best, and a vulnerability at worst.
My proposal is to add a validator (for instance single_line
) that checks that an input value doesn't contain a newline, and to add it to all the fields that are presented as <input type="text">
, and (optionally) to add a field option (for instance, allow_multiline
) to override this behavior.
I'm proposing that this become part of django for the following reasons:
- If the user uses the default form field produced by such a field, they cannot enter a multiline value anyway, so my proposal fixes the problem that validation on the server is "weaker" than on the client.
- Although it's a corner case, this could, in fact, create actual vulnerabilities (Use case: a simple protocol that has DSV with the field as the last entry per line).
- People who want multiline will use
TextField
anyway. If someone out there has customizedCharField
to act likeTextField
, they need not complain if they have to fix that. For the other field types, "no multiline" is implicit on their respective validation (haven't checked, but if that isn't the case, that's arguable a bug in itself). Therefore, compatibility is not a problem.
I volunteer to write a patch that implements this if this ticket is accepted.
Change History (3)
comment:1 by , 13 years ago
Component: | Database layer (models, ORM) → Forms |
---|---|
Needs documentation: | set |
Needs tests: | set |
Triage Stage: | Unreviewed → Accepted |
Type: | Bug → New feature |
comment:2 by , 13 years ago
Needs documentation: | unset |
---|---|
Needs tests: | unset |
tl;dr This proposal adds an implicit, non-obvious behavior; and explicit is better than implicit.
I'm skeptical, to say the least.
1) Currently, Django does not apply any restrictions to the data it receives in form fields, and we should keep it consistent. If \n
are undesirable in some POST data, it is the developer's responsibility to validate it at the level of the form.
2) Linking a validation to the fact that the widget will be rendered as <input type="text">
is magic and prone to unexpected side effects. What if Javascript turns the <input>
into a full blow WYSIWYG editor? This makes the proposed change backwards incompatible.
3) If \n
in strings create a vulnerability, it is the developer's responsibility to validate it at the level of the model and . And \n
isn't more dangerous in itself than \r
, \t
, \v
and other hard-to-type characters, depending on the context.
4) (This is debatable.) I don't like the idea of comforting developers in the belief that their app won't receive anything else than data entered by users in form fields. IMO, that helps them to shoot themselves into the foot, from a security point of view. Web apps receive all kinds of crap and should validate external data, period.
comment:3 by , 13 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
Closing as wontfix for all the reasons given by aaugustin. It is also similar to #4960 which was closed as wontfix.
I've marked this as Accepted, but I do have slight misgivings. It seems like your intent is to add the validator to every CharField - but a CharField won't necessarily be rendered as an <input type="text">. I'm also not sure whether you're planning on patching the model field or the form field. Since this is related to what the html input box looks like, it seems more like a form field patch, especially since I don't know that databases would actually have a problem storing newlines as long as the string was short enough.