Skip to content

fix: parse IPv6 Sentinel URL nodes independently - #781

Open
Claire-ovo wants to merge 1 commit into
redis:mainfrom
Claire-ovo:fix/sentinel-ipv6-parsing
Open

Claire-ovo wants to merge 1 commit into
redis:mainfrom
Claire-ovo:fix/sentinel-ipv6-parsing

Conversation

@Claire-ovo

@Claire-ovo Claire-ovo commented Oct 23, 2026 •

Copy link
Copy Markdown

Motivation

Sentinel URLs split each node on :, so [::1]:26380 becomes ('[', 26379) rather than ('::1', 26380). Parsing the entire comma-separated authority with urlparse also rejects mixed hostname/IPv6 lists on current Python versions.

Reproduce without a running server:

from redisvl.redis.connection import RedisConnectionFactory

RedisConnectionFactory._parse_sentinel_url(
    "redis+sentinel://[::1]:26380/mymaster/0"
)

Changes

Separate the node list before URL parsing, then use the standard-library hostname/port parser for each node. Keep credential decoding, service/database selection, and the default Sentinel port; decode percent-encoded hostnames, including IPv6 scope identifiers.

Add parser regressions and parameter-forwarding coverage through sync, current async, and legacy async factories. Document bracketed IPv6 syntax. No dependency or query-parameter behavior changes.

Release Notes

Sentinel URLs accept bracketed IPv6 addresses and mixed hostname/IPv4/IPv6 lists. Empty nodes, unbracketed IPv6 and out-of-range ports now raise ValueError during parsing. Empty explicit ports and nonnumeric ports are also rejected, including on IPv6 nodes.

Review record

Reviewed at a4a835eadda2a665a9bb2618cf1b8ba9e5f4d446 by Codex with pre-pr-review. Prepared with AI assistance; model identifier is not independently exposed by this execution environment.
Lenses: correctness (parser/callers); security (connection URLs and credentials); design (shared factory behavior and legacy async compatibility); tests (new regressions); docs (IPv6 syntax). Lens passes were delegated; correctness/tests and design/docs reused their respective reviewer contexts. Final challenge ran in a fresh independent context. Skipped: Redis behaviour (no server command, reply or server-default change; 26379 is the existing client default).

  • Two defect candidates reviewed; one kept. Two additional nonblocking test suggestions were accepted.
  • Fixed: percent-encoded IPv6 zone identifiers now decode before connection (redisvl/redis/connection.py:1067); added encoded/raw scope assertions. Added explicit port-zero and query/fragment boundary regressions.
  • Rejected: additional malformed-bracket parsing machinery; the proposed failure was not reproduced on the tested Python versions, which already reject it through urlparse.
  • Open for the reviewer: none identified.

Verified:

  • Initial regression run against unchanged production code: 17 failed, 18 passed. Four boundary cases were subsequently added; no claim that all added cases failed on the old code.
  • Python 3.12.14, redis-py 8.1.0: pytest tests/unit/test_sentinel_url.py tests/unit/test_connection_protocol.py tests/unit/test_connection_normalization.py -q -o log_cli=false --tb=short: 79 passed with the repository's Redis 8.4 Docker fixture enabled.
  • Python 3.10.21: isolated pytest --noconftest tests/unit/test_sentinel_url.py -q -o log_cli=false --tb=short: 39 passed (pure parser/factory tests; no Docker fixture).
  • Live Redis/Sentinel 8.4.7, Linux Python 3.12.3, redis-py 8.1.0: six discovery and SET/GET/DEL round trips passed. Sync/current-async/legacy-async factories each connected through [::1]:26380 and through 127.0.0.1:26381,[::1]:26380 (first node deliberately unavailable). Sentinel and the discovered primary both bound to IPv6 loopback in an isolated container network namespace. This does not exercise failover.
  • make format, make check-types (120 source files), pre-commit run code-quality-checks --all-files, and git diff --check: passed.
  • make docs-build: succeeded with 12 warnings in unchanged pages/dependency docstrings. Notebooks were not executed.
  • Full pre-commit was attempted: the code-quality hook passed, but the pinned codespell hook rewrote the pre-existing word “Unparseable” outside this change. That unrelated edit was reverted; a targeted codespell check with that existing word ignored passed. Full pre-commit is not reported as passing.

Not checked: full repository suite, all Python/redis-py versions, Sentinel failover, TLS, authenticated live Sentinel, link-local interface connectivity, or Cluster behavior. Parser and mocked-factory tests do not prove those deployment scenarios.


Note

Medium Risk
Changes connection URL parsing for Sentinel deployments; incorrect parsing could block HA connections, though scope is limited to _parse_sentinel_url and stricter validation may surface previously silent misconfigurations.

Overview
Fixes redis+sentinel:// parsing so comma-separated Sentinel nodes are handled correctly when IPv6 literals use bracket notation or when hostnames and IPv6 addresses are mixed.

_parse_sentinel_url no longer splits the authority on : or runs the whole list through a single urlparse. It isolates the host list, parses each node with the stdlib hostname/port logic, keeps default port 26379, and decodes percent-encoded hostnames (including IPv6 zone IDs). Malformed nodes (unbracketed IPv6, empty segments, invalid ports) now raise ValueError at parse time. Query strings and fragments are stripped from the authority so they cannot be mistaken for hosts or path segments.

Unit tests cover valid IPv6/mixed lists, invalid inputs, and that sync, async, and legacy async factories still pass the parsed hosts, credentials, service name, and DB into Sentinel / AsyncSentinel. Installation docs now describe bracketed IPv6 Sentinel URLs.

Reviewed by Cursor Bugbot for commit a4a835e. Bugbot is set up for automated code reviews on this repo. Configure here.

This branch has not been deployed

No deployments
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