Skip to content

Comments

Accept ssl_mode in DSN URI query parameters#1595

Open
rolandwalker wants to merge 1 commit intomainfrom
RW/change-uri-query-parameter-to-ssl-mode
Open

Accept ssl_mode in DSN URI query parameters#1595
rolandwalker wants to merge 1 commit intomainfrom
RW/change-uri-query-parameter-to-ssl-mode

Conversation

@rolandwalker
Copy link
Contributor

Description

  • deprecate ssl in DSN query parameters (as in CLI options)
  • accept ssl_mode in DSN query parameters
  • refactor implicit SSL-enable logic to use ssl_mode

Incidentally

  • fix CLI option deprecation warning to refer to default_ssl_mode (adding prefix "default_")
  • update tests which did not account for SSL becoming on by default

Checklist

  • I added this contribution to the changelog.md file.
  • I added my name to the AUTHORS file (or it's already there).
  • To lint and format the code, I ran
    uv run ruff check && uv run ruff format && uv run mypy --install-types .

 * deprecate "ssl" in DSN query parameters (as in CLI options)
 * accept "ssl_mode" in DSN query parameters
 * refactor implicit SSL-enable logic to use "ssl_mode"

Incidentally

 * fix CLI option deprecation warning to refer to default_ssl_mode
   (adding prefix "default_")
 * update tests which did not account for SSL becoming on by default
@rolandwalker rolandwalker self-assigned this Feb 21, 2026
@github-actions
Copy link

Findings

  1. Correctness: Deprecated ssl=true can override explicit ssl_mode (CLI or DSN).
    In mycli/main.py the ssl DSN param sets ssl_mode = 'on' unconditionally. If the user passes --ssl-mode=off (or DSN includes ssl_mode=off) and the DSN also contains ssl=true, the deprecated flag wins. That contradicts the intended “explicit ssl_mode wins” behavior and the updated test premise.
    File: mycli/main.py:1971-1984
    Fix: Only set ssl_mode='on' if it is not already set, e.g. if params[0].lower() == 'true' and ssl_mode is None: ssl_mode = 'on'. Also consider ignoring ssl when ssl_mode is present.

  2. Correctness: DSN ssl_mode is not validated or normalized, can silently disable SSL.
    ssl_mode from DSN is used verbatim. If user supplies ssl_mode=ON or any invalid value, it won’t match ("auto","on") and SSL will be disabled without warning.
    File: mycli/main.py:1982-1984, mycli/main.py:2020-2031
    Fix: Normalize to lowercase and validate against allowed values, warning and ignoring invalid values (similar to config validation).

Missing Tests

  1. Deprecated ssl precedence vs ssl_mode
    Add a test where DSN contains ssl=true and CLI --ssl-mode=off (or DSN ssl_mode=off) to assert explicit ssl_mode wins.
    Related: test/test_main.py:882-907

  2. Invalid or mixed-case ssl_mode in DSN
    Add tests for ssl_mode=ON and ssl_mode=invalid to ensure normalization or warning and expected behavior.

I didn’t run tests. If you want, I can add the tests and propose the small logic fix.

@rolandwalker
Copy link
Contributor Author

Correctness: DSN ssl_mode is not validated or normalized

I plan to address this comprehensively once a series is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant