Opened 3 years ago

Closed 3 years ago

Last modified 3 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 Changed 3 years ago by Claude Paroz

Easy pickings: set
Triage Stage: UnreviewedAccepted

Most probably a regression in 1.9 introduced by b64c0d4d613b5cabedbc9b847682fe14877537de

comment:2 Changed 3 years ago by mieciu

Owner: changed from nobody to mieciu
Status: newassigned

comment:3 Changed 3 years ago by mieciu

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 Changed 3 years ago by Claude Paroz

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 Changed 3 years ago by mieciu

Has patch: set

comment:6 Changed 3 years ago by mieciu

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 Changed 3 years ago by Philip James

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

comment:8 Changed 3 years ago by Tim Graham

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 Changed 3 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 19ff5068:

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

comment:10 Changed 3 years ago by Tim Graham <timograham@…>

In 9a204fc:

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

Backport of 19ff506878071ac93de684fe01328707e75e2b3a from master

comment:11 Changed 3 years ago by Tim Graham <timograham@…>

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