Skip to content

Fix 'url' parameter handling in KartonBackend.make_redis#325

Merged
psrok1 merged 1 commit intomasterfrom
fix/url-make-redis
Mar 23, 2026
Merged

Fix 'url' parameter handling in KartonBackend.make_redis#325
psrok1 merged 1 commit intomasterfrom
fix/url-make-redis

Conversation

@psrok1
Copy link
Copy Markdown
Member

@psrok1 psrok1 commented Mar 17, 2026

#292 introduced bug where url argument in [redis] configuration section doesn't work with rediss:// and unix:// URI schemes.

In #292, I've parsed the URL using redis.connection.parse_url to be able to strip credentials when Redis appears to not require authentication. That was bad decision because parse_url is meant for passing parameters to ConnectionPool class and not the Redis class. So e.g. when we use rediss scheme, we get connection_class parameter set to SSLConnection which is not accepted by the Redis client constructor.

So in this PR, I'm using Redis.from_url when url is passed in configuration. I'm also not trying to strip credentials from the URL - if someone relies on our the fallback for smooth transition, they must put the credentials in username and password configuration fields.

I have also noticed that we don't allow for password without username which is incorrect. Redis allows to set authentication both via requirepass option globally (which is not associated with any username) and ACLs (see also https://redis.io/docs/latest/commands/auth/). ACL system was introduced in Redis 6.0, so older Redis may reject the AUTH <username> <password> form. I have removed that requirement in this PR.

@psrok1 psrok1 requested a review from msm-cert March 17, 2026 17:22
@psrok1 psrok1 merged commit bd9936f into master Mar 23, 2026
7 checks passed
@psrok1 psrok1 deleted the fix/url-make-redis branch March 23, 2026 17:56
@psrok1 psrok1 removed the request for review from msm-cert March 23, 2026 17:57
@psrok1 psrok1 mentioned this pull request Mar 23, 2026
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