Opened 87 minutes ago
Last modified 83 minutes ago
#37433 assigned Cleanup/optimization
Escaped database error messages logged by `Q.check()`
| Reported by: | Natalia Bidart | Owned by: | Natalia Bidart |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | dev |
| Severity: | Normal | Keywords: | not-security |
| Cc: | Jake Howard | Triage Stage: | Accepted |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
Q.check(), used by CheckConstraint.validate() and by UniqueConstraint.validate() with a condition, catches DatabaseError and logs it to the django.db.models logger:
logger.warning("Got a database error calling check() on %r: %s", self, e)
Database error messages can include the values being validated, which may originate from user input (for example through ModelForm validation). These messages are currently logged as-is. For defense in depth, and for consistency with other logging call sites, Q.check() should log the message through django.utils.log.log_message() so that its content is escaped, and trim it to a reasonable length.
The trimming logic for large values is currently duplicated in django/views/debug.py and django/contrib/gis/gdal/raster/source.py. A small trim_value() helper in django.utils.text could be shared by those two places and by Q.check().
Separately, the fact that BaseConstraint.validate() treats a constraint as satisfied when evaluating it raises a database error (logging a warning and leaving the error to surface on save()) is not documented. A short note in the constraints reference would help developers understand why a condition that can fail to evaluate does not act as validation.
Change History (2)
comment:1 by , 86 minutes ago
| Cc: | added |
|---|---|
| Keywords: | not-security added |
comment:2 by , 83 minutes ago
| Triage Stage: | Unreviewed → Accepted |
|---|