[Django] #22234: Special characters in database password are not escaped correctly on Windows platforms
Django
Windows platforms
----------------------------------------------+--------------------
Reporter: djangoproject.com@… | Owner: nobody
Type: Bug | Status: new
Component: Database layer (models, ORM) | Version: 1.6
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
https://github.com/django/django/blob/master/django/db/backends/mysql/client.py#L38
On Windows platforms, the database client command line is composed as a
string, but no escaping whatsoever is performed on the parameters.
Database configuration can contain special characters (especially the
password), which would require escaping.
Real-life case: http://stackoverflow.com/questions/22280126/sign-in-
database-password-in-django
--
Ticket URL: <https://code.djangoproject.com/ticket/22234>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Windows platforms
Reporter: djangoproject.com@… | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by djangoproject.com@…):
* needs_better_patch: => 0
* has_patch: 0 => 1
* version: 1.6 => master
* needs_tests: => 0
* needs_docs: => 0
Comment:
Proposed fix:
https://github.com/django/django/pull/2412
--
Ticket URL: <https://code.djangoproject.com/ticket/22234#comment:1>
Django
Windows platforms
Reporter: djangoproject.com@… | Owner: nobody
Type: Bug | Status: new
commands) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by bmispelon):
* component: Database layer (models, ORM) => Core (Management commands)
* stage: Unreviewed => Accepted
Comment:
Hi,
It seems that the different codepath on windows was added as a result of
#10357.
Looking at the discussion (especially comment number 3 [1]), it seems that
using `subprocess` was considered at the time but it wasn't used because
it was incompatible with Python 2.3 which was supported at the time.
The patch looks good and cleans things up nicely but I don't have access
to a windows box to make sure it actually works.
I'm also concerned about a potential for subtle differences between
`os.execp` and `subprocess.call`. The documentation of `os.execp` [2] is a
bit hard to read and as a result, I can't really tell exactly what
differences there are between the two.
Still, I'll mark this ticket as `accepted`, if only on the basis that
using `os.system(" ".join(args))` is clearly wrong.
Thanks.
[1] https://code.djangoproject.com/ticket/10357#comment:3
[2] http://docs.python.org/2/library/os.html?highlight=os.execvp#os.execvp
--
Ticket URL: <https://code.djangoproject.com/ticket/22234#comment:2>
Django
Windows platforms
Reporter: djangoproject.com@… | Owner: nobody
Type: Bug | Status: new
commands) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by lanzz):
From the [http://docs.python.org/2/library/subprocess.html#subprocess.call
subprocess.call docs]:
The full function signature is the same as that of the `Popen`
constructor - this functions passes all supplied arguments directly
through to that interface.
From the
[http://docs.python.org/2/library/subprocess.html#subprocess.Popen
subprocess.Popen docs]:
Execute a child program in a new process. On Unix, the class uses
`os.execvp()`-like behavior to execute the child program. On Windows, the
class uses the Windows `CreateProcess()` function.
Sounds to me `subprocess.call` and `os.execvp` are explicitly meant to be
compatible on Unix, and on Windows `os.execvp` isn't an option anyway.
--
Ticket URL: <https://code.djangoproject.com/ticket/22234#comment:3>
Django
Windows platforms
Reporter: djangoproject.com@… | Owner: nobody
Type: Bug | Status: new
commands) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* cc: anubhav9042@… (added)
Comment:
If only you could add a test to your PR, we can test the new
implementation.
--
Ticket URL: <https://code.djangoproject.com/ticket/22234#comment:4>
Django
Windows platforms
Reporter: djangoproject.com@… | Owner: nobody
Type: Bug | Status: new
commands) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by lanzz):
I would, but I'm not really sure how to approach testing this change. The
fact that there are zero existing tests for the DatabaseClient
implementations does not help either — it seems historical changes to
`backends.*.client.DatabaseClient` have never included a testcase.
--
Ticket URL: <https://code.djangoproject.com/ticket/22234#comment:5>
Django
Windows platforms
Reporter: djangoproject.com@… | Owner: nobody
Type: Bug | Status: closed
Component: Core (Management | Version: master
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"bf5382c6e549d78da9f7029cd86686459b52eaed"]:
{{{
#!CommitTicketReference repository=""
revision="bf5382c6e549d78da9f7029cd86686459b52eaed"
Fixed #22234 -- Replaced OS-specific code with subprocess.call() in
dbshell.
This fixes escaping of special characters on Windows.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22234#comment:6>