search

MagicStack / asyncpg

A fast PostgreSQL Database Client Library for Python/asyncio.
Apache License 2.0
6.88k stars 399 forks source link

WIP: Add Testing Failcase for DSN Parsing #1159

Open ranchodeluxe opened 3 months ago

ranchodeluxe commented 3 months ago

Problem

Crunchydata Postgresql Operator creates passwords by default using the US-ASCII spec. Some of them with special characters in it including @, [, ], / that seem to cause problems for asyncpg host parsing even after running urllib.parse.quote on the password.

The error below shows it return the password assuming it's the port from an example app:

  File "/usr/local/lib/python3.8/site-packages/asyncpg/connect_utils.py", line 633, in _parse_connect_arguments
    addrs, params = _parse_connect_dsn_and_args(
  File "/usr/local/lib/python3.8/site-packages/asyncpg/connect_utils.py", line 288, in _parse_connect_dsn_and_args
    host, port = _parse_hostlist(dsn_hostspec, port, unquote=True)
  File "/usr/local/lib/python3.8/site-packages/asyncpg/connect_utils.py", line 229, in _parse_hostlist
    hostlist_ports.append(int(hostspec_port))
ValueError: invalid literal for int() with base 10: 'a2Vw:yk=)CdSis[fek]tW='

Test output (from this PR) running locally:

――――――――――――――――――――――――――――――――――――――――――――――――― TestConnectParams.test_connect_params ―――――――――――――――――――――――――――――――――――――――――――――――――
Traceback (most recent call last):
  File "/usr/local/Cellar/python@3.11/3.11.8/Frameworks/Python.framework/Versions/3.11/lib/python3.11/unittest/case.py", line 57, in testPartExecutor
    yield
  File "/usr/local/Cellar/python@3.11/3.11.8/Frameworks/Python.framework/Versions/3.11/lib/python3.11/unittest/case.py", line 623, in run
    self._callTestMethod(testMethod)
  File "/usr/local/Cellar/python@3.11/3.11.8/Frameworks/Python.framework/Versions/3.11/lib/python3.11/unittest/case.py", line 579, in _callTestMethod
    if method() is not None:
       ^^^^^^^^
  File "/Users/ranchodeluxe/apps/asyncpg/tests/test_connect.py", line 1186, in test_connect_params
    self.run_testcase(testcase)
  File "/Users/ranchodeluxe/apps/asyncpg/tests/test_connect.py", line 1106, in run_testcase
    addrs, params = connect_utils._parse_connect_dsn_and_args(
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/ranchodeluxe/apps/asyncpg/asyncpg/connect_utils.py", line 296, in _parse_connect_dsn_and_args
    host, port = _parse_hostlist(dsn_hostspec, port, unquote=True)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/ranchodeluxe/apps/asyncpg/asyncpg/connect_utils.py", line 231, in _parse_hostlist
    hostlist_ports.append(int(hostspec_port))
                          ^^^^^^^^^^^^^^^^^^
ValueError: invalid literal for int() with base 10: 'a2Vw:yk=)CdSis[fek]tW='
ranchodeluxe commented 3 months ago

to get this test passing locally using the testing infrastructure I had to:

  1. remove these problematic character/ then urllib.parse.quote worked:

revised passing test

        {
            'name': 'dsn_bad_characters_maybe',
            'dsn': 'postgres://eoapi:a2Vw%3Ayk%3D%29CdSis%5Bfek%5DtW%3D@eoapi-primary.default.svc:5432/eoapi',
            'result': ([('eoapi-primary.default.svc', 5432)], {
                'user': 'eoapi',
                'password': 'a2Vw:yk=)CdSis[fek]tW=',
                'database': 'eoapi',
                'ssl': True,
                'sslmode': SSLMode.prefer
                ,
                'target_session_attrs': 'any'})
        },
ranchodeluxe commented 3 months ago

I see we have a _validate_port_spec.

If passwords don't allow certain characters should we validate and raise too?

ranchodeluxe commented 3 months ago

thanks @elprans : urllib.parse.quote_plus() works, test is up to date

will close this

$ pytest tests/test_connect.py -k TestConnectParams

 tests/test_connect.py ✓✓✓✓✓✓✓✓✓✓                                                                                         100% ██████████

Results (0.09s):
      10 passed
      41 deselected