#25705: Parameters are not adapted or quoted in Query.__str__
-------------------------------------+-------------------------------------
Reporter: Dmitry Dygalo | Owner: Alex
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Simon Charette):
* cc: Simon Charette (added)
Comment:
@Carlton I think the request for this feature is a symptom of a different
problem so maybe we could re-purpose this ticket instead?
----
@Alex
> I wouldn't consider someone doing str(qs.query) to then pass in to raw()
(which the docs already warn of SQL injections) a real use case that
anyone would do.
I don't agree on that point. From helping users through various forums in
most cases the reason why they were bringing up the issue of improper
quoting was when they tried executing the query as is. The duplicates of
this ticket seem to support this position (see #17741 and #25092
[https://stackoverflow.com/questions/31287184/django-orm-magic-bug-
or-a-feature/31287932#31287932 SO]) and I don't think would be hard to
find other examples.
It might not be your personal use case as you are well versed into why
this a bad idea but I think it would be irresponsible to our user base
from an API design (newcomers footgun) and maintenance burden perspective
(think of the security and bug reports about `mogrify` not producing the
exact same SQL as backend escaping normally does).
> Also my recommended choice was not trying to emulate parameter quoting,
but to use the database library mogrify method in 3 of the 5 supported
backends.
You second alternative mentioned
> Use the mogrify function in the first three backends, and manually quote
the parameters in the other two.
I ''think'' that would qualify as a form of emulation or best-effort
wouldn't it? What guarantees do we have that the quoting is appropriate
for all datatypes that these backends support as parameters? Are we
feeling confident, knowing that users will attempt to pipe `str(qs.query)`
into `raw` and that `Query.__str__` always use `DEFAULT_DB_ALIAS` for
compilation (even if the proper backend can often only be determined as
execution time) that we are exposing the right tools to users and we can
commit to them being safe for the years to come?
IMO the usage of `raw(str(qs.query))`, and the main motive for this
ticket, is a symptom of a lack of documented way for safely building and
executing the SQL and parameters from a `QuerySet` object which makes me
believe the focus should be on documenting `queryset.qs.sql_with_params()`
first instead?
As for the printing and copy-pasting into a shell use case we've reached
an agreement that the only way to see the query with the parameters binded
correctly is after executing it. Knowing that, and the fact
`Query.__str__` is still pretty legible even without the exact mogrifying
that only leaves the copy-pasting into a shell use case. Should we make it
easier to retrieve the exact SQL associated with a particular queryset
instead? Something like `QuerySet.executed_query: str | None` that is only
present on ''evaluated'' querysets.
These appears like solutions that prevent misuse of `Query.__str__` and
avoid maintaining correct and safe x-backend mogrifying solutions?
--
Ticket URL: <https://code.djangoproject.com/ticket/25705#comment:16>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/01070190b76fe942-e2be3220-50d0-4964-b5f8-ff4032b262b8-000000%40eu-central-1.amazonses.com.
