Skip to content

Preserve explicit port zero in connection URLs - #4325

Open
niukanen1 wants to merge 2 commits into
redis:masterfrom
niukanen1:fix/preserve-url-port-zero
Open

niukanen1 wants to merge 2 commits into
redis:masterfrom
niukanen1:fix/preserve-url-port-zero

Conversation

@niukanen1

@niukanen1 niukanen1 commented Sep 13, 2026

Copy link
Copy Markdown

Description of change

An explicit zero port in a connection URL was treated as if the port had been omitted:

ConnectionPool.from_url("redis://localhost:0").connection_kwargs.get("port")
# before: None
# after:  0

Both URL parsers now distinguish 0 from None, so the pool preserves the supplied port instead of silently falling back to port 6379. Async cluster initialization also preserves integer zero when constructing startup nodes; previously rejected values such as an empty string remain invalid.

Regression tests cover omitted, ordinary, and zero ports through synchronous and asyncio parsing and pool construction, as well as public async cluster construction and invalid direct-constructor values.

Validation:

  • The 14 new parser, pool-construction, and async-cluster regression cases passed without a Redis server.
  • pytest -q tests/test_connection.py tests/test_asyncio/test_connection.py -k parse_url — 14 passed.
  • invoke linters — passed.
  • The full Redis integration matrix was not run locally; these focused checks do not require a Redis server.

Pull Request check-list

  • Focused tests and repository linters pass with this change.
  • CI tests pass with this change.
  • The changed parsing and cluster-startup behavior has regression tests.
  • Documentation update — not applicable; no new API.
  • Example in the examples folder — not applicable.

Note

Low Risk
Behavior change is limited to edge-case port parsing and async cluster startup validation, with targeted regression tests and no auth or data-path changes.

Overview
Fixes URL and cluster client handling so an explicit port 0 is no longer dropped as “missing.”

URL parsing (parse_url in sync and asyncio connection.py) now sets port when parsed.port is not None instead of truthiness checks, so redis://localhost:0 keeps port=0 rather than omitting it and falling back to 6379.

Async RedisCluster uses port_is_provided = bool(port) or type(port) is int for startup-node validation and construction, so from_url("redis://localhost:0") builds a startup node with port 0 while still rejecting invalid falsy ports (None, "", False, 0.0).

Regression tests cover omitted, normal, and zero ports for parse_url/ConnectionPool.from_url (both clients) and for RedisCluster.from_url / constructor validation.

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

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 13, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-13T18:26:11.675742Z e6af331 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

Reviewed by Cursor Bugbot for commit 615ff45. Configure here.

Comment thread redis/asyncio/connection.py
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