Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#26698 closed Bug (fixed)

dbshell crashes on Postgres backend with empty database name

Reported by: Ilya Semenov Owned by: mieciu
Component: Core (Management commands) Version: 1.9
Severity: Normal Keywords: dbshell
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

Django database backend postgresql_psycopg2 works perfectly fine without specifying database NAME (falling back to the user name for the database name) until you run manage.py dbshell, when it crashes.

Please see the test case below.

root@3a46382913a8:/# /app/src/manage.py shell
>>> from django.conf import settings
>>> pprint(settings.DATABASES['default'])
{'ATOMIC_REQUESTS': False,
 'AUTOCOMMIT': True,
 'CONN_MAX_AGE': 0,
 'ENGINE': 'django.db.backends.postgresql_psycopg2',
 'HOST': '172.17.0.3',
 'NAME': None,
 'OPTIONS': {},
 'PASSWORD': 'xxx',
 'PORT': '5432',
 'TEST': {'CHARSET': None, 'COLLATION': None, 'MIRROR': None, 'NAME': None},
 'TIME_ZONE': None,
 'USER': 'postgres'}
>>> from django.db import connection
>>> c=connection.cursor(); c.execute("SELECT 1"); print(c.fetchone())
(1,)

but:

root@3a46382913a8:/# /app/src/manage.py dbshell
Traceback (most recent call last):
  File "/app/src/manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/usr/local/lib/python3.5/dist-packages/django/core/management/__init__.py", line 353, in execute_from_command_line
    utility.execute()
  File "/usr/local/lib/python3.5/dist-packages/django/core/management/__init__.py", line 345, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/usr/local/lib/python3.5/dist-packages/django/core/management/base.py", line 348, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/usr/local/lib/python3.5/dist-packages/django/core/management/base.py", line 399, in execute
    output = self.handle(*args, **options)
  File "/usr/local/lib/python3.5/dist-packages/django/core/management/commands/dbshell.py", line 19, in handle
    connection.client.runshell()
  File "/usr/local/lib/python3.5/dist-packages/django/db/backends/postgresql/client.py", line 66, in runshell
    DatabaseClient.runshell_db(self.connection.settings_dict)
  File "/usr/local/lib/python3.5/dist-packages/django/db/backends/postgresql/client.py", line 46, in runshell_db
    _escape_pgpass(name) or '*',
  File "/usr/local/lib/python3.5/dist-packages/django/db/backends/postgresql/client.py", line 13, in _escape_pgpass
    return txt.replace('\\', '\\\\').replace(':', '\\:')
AttributeError: 'NoneType' object has no attribute 'replace'

Change History (11)

comment:1 by Claude Paroz, 8 years ago

Easy pickings: set
Triage Stage: UnreviewedAccepted

Most probably a regression in 1.9 introduced by b64c0d4d613b5cabedbc9b847682fe14877537de

comment:2 by mieciu, 8 years ago

Owner: changed from nobody to mieciu
Status: newassigned

comment:3 by mieciu, 8 years ago

It's something related to postgresql's DatabaseClient class and temporary .pgpass file creation.

IMO omitting such a vital parameter database name is a crucial mistake in configuration and nobody should rely here on any fallback mechanism...

comment:4 by Claude Paroz, 8 years ago

If we want Django refusing the default database name with Postgres, that should be the subject of a separate ticket (and maybe a discussion on the django-developers mailing list). But clearly, the _escape_pgpass should not crash when receiving a None value.

comment:5 by mieciu, 8 years ago

Has patch: set

comment:6 by mieciu, 8 years ago

I totally agree.

However, I've found a way to actually have the fallback mechanism working for dbshell, so at least we're consistent here :)

PR is already waiting.

comment:7 by Philip James, 8 years ago

Should a test be added to account for the case where 'NAME' == None?

comment:8 by Tim Graham, 8 years ago

Triage Stage: AcceptedReady for checkin

I don't see an easy way to add a regression test for this since the current tests bypass DatabaseClient.runshell() which is what's changed to fix the issue.

comment:9 by Tim Graham <timograham@…>, 8 years ago

Resolution: fixed
Status: assignedclosed

In 19ff5068:

Fixed #26698 -- Fixed PostgreSQL dbshell crash on an empty database name.

comment:10 by Tim Graham <timograham@…>, 8 years ago

In 9a204fc:

[1.10.x] Fixed #26698 -- Fixed PostgreSQL dbshell crash on an empty database name.

Backport of 19ff506878071ac93de684fe01328707e75e2b3a from master

comment:11 by Tim Graham <timograham@…>, 8 years ago

In 5d60eb8b:

[1.9.x] Fixed #26698 -- Fixed PostgreSQL dbshell crash on an empty database name.

Backport of 19ff506878071ac93de684fe01328707e75e2b3a from master

Note: See TracTickets for help on using tickets.
Back to Top