Edgewall Software

Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#12028 closed defect (fixed)

Regression in [13888]: `parse_connection_uri` may trim `params` for the returned dictionary

Reported by: pierre-yves@… Owned by: Ryan J Ollos
Priority: normal Milestone: 1.1.5
Component: database backend Version: 1.1.4
Severity: normal Keywords: postgresql
Cc: Branch:
Release Notes:

Fixed regression in [13888] that resulted in KeyError when backing up a PostgreSQL database.

API Changes:
Internal Changes:


13888 is assuming that the dictionary returned by parse_connection_uri contains a params key, replacing the call .setdefault('params', {}) by ['params'].

However, parse_connection_uri trims the returned dictionary from the key-value pairs for values coercible to False. The default value of params (= {}) is coercible to False.

Either parse_connection_uri should not trim such values, either 13888 should be reverted.

This currently makes the backup command unusable under some conditions.

Attachments (0)

Change History (7)

comment:1 by Jun Omae, 8 years ago

Keywords: postgresql added
Milestone: 1.1.5

Okay. You're right. Reproduced using database connection string without parameters.

>>> from trac.test import EnvironmentStub
>>> env = EnvironmentStub()
>>> env.config.set('trac', 'database', 'postgres://user:password@')
>>> env.backup()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "trac/env.py", line 719, in backup
    return DatabaseManager(self).backup(dest)
  File "trac/db/api.py", line 523, in backup
    return connector.backup(dest)
  File "trac/db/postgres_backend.py", line 177, in backup
    db_params = db_prop['params']
KeyError: 'params'

comment:2 by Ryan J Ollos, 8 years ago

Reverting change on line 165 of [13888] seems like the best option. I'm considering whether parse_connection_uri should at least include dictionary entries for parameters that are relevant for a particular scheme, but not sure we should change the behavior of the function.

comment:3 by Ryan J Ollos, 8 years ago

Owner: set to Ryan J Ollos
Status: newassigned

comment:4 by Ryan J Ollos, 8 years ago

Summary: [changeset:13888] is plain wrong - `parse_connection_uri` may trim `params` for the returned dictionaryRegression in [13888]: `parse_connection_uri` may trim `params` for the returned dictionary

comment:5 by Ryan J Ollos, 8 years ago

Regression fixed in [13992]. I'm leaving the ticket open to commit regressions tests, provided I can find time for that before the conclusion of 1.1.5.

comment:6 by Ryan J Ollos, 8 years ago

Resolution: fixed
Status: assignedclosed

comment:7 by Ryan J Ollos, 8 years ago

Release Notes: modified (diff)

Modify Ticket

Change Properties
Set your email in Preferences
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Ryan J Ollos to the specified user.

Add Comment

E-mail address and name can be saved in the Preferences .
Note: See TracTickets for help on using tickets.