Skip to content

fix(store): merge WithURL fields with WithAuth/WithPassword/WithDB, only error on real conflict - #103

Open
destinyoooo wants to merge 1 commit into
boj:masterfrom
destinyoooo:master
Open

fix(store): merge WithURL fields with WithAuth/WithPassword/WithDB, only error on real conflict#103
destinyoooo wants to merge 1 commit into
boj:masterfrom
destinyoooo:master

Conversation

@destinyoooo

Copy link
Copy Markdown

Background

WithURL interacts silently with WithAuth / WithPassword / WithDB: the URL branch of buildPool only calls redis.DialURL(cfg.url), so any values set via WithAuth("u", "p") or WithDB("5") are completely ignored when WithURL is also set. There is no error, no warning — the user just silently connects with the URL's credentials (or none) instead of the ones they thought they were configuring. In production this is a footgun for multi-tenant Redis setups and ACL-based auth.

Changes

Rather than a blanket "URL and options are mutually exclusive" rule, the fix is per-field conflict detection + field-level merge:

  1. New internal helper parseRedisURL and struct redisURLParts, supporting the redis://, rediss://, and unix:// schemes. For unix:// the database component is forced to "" — the socket path is not a db.
  2. validate(), when cfg.url != "", checks the username / password / db fields one by one. It only returns an error when both the URL and an option set the same field, and the error names the conflicting field.
  3. buildPool() no longer calls redis.DialURL for the URL branch. It parses the URL and merges it with cfg.username / cfg.password / cfg.db using "URL first, options fill the gaps", then routes through dialClient so every value is actually applied to the dial.

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