Repository navigation
fix(ui): accept IPv6 hosts in connection URLs - #6555
RaphaelFakhri wants to merge 2 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a41155a23
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| host: unwrapIpv6Host(host), | ||
| port: port ? parseInt(port, 10) : undefined, | ||
| hostname: port ? `${host}:${port}` : host, |
There was a problem hiding this comment.
When an explicitly ported URL such as redis://[::1]:6379 is saved, these lines produce host = "::1" but name = "[::1]:6379". On a later edit that changes only the host or port, DatabaseService.hasDefaultEndpointName in redisinsight/api/src/modules/database/database.service.ts:119-120 compares that generated name with ${database.host}:${database.port} (::1:6379), classifies it as a custom alias, and therefore skips the synchronization at lines 297-302, leaving the old endpoint displayed as the database name. Use the same IPv6-aware endpoint formatter for both generated aliases and default-name detection/synchronization.
Useful? React with 👍 / 👎.
|
Good catch. The generated name for an IPv6 URL is now |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 449eae6580
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| port: port ? parseInt(port, 10) : undefined, | ||
| hostname: port ? `${host}:${port}` : host, | ||
| // same "host:port" format the API uses for default database names | ||
| hostname: port ? `${unwrappedHost}:${port}` : unwrappedHost, |
There was a problem hiding this comment.
When the newly accepted no-port form redis://[::1] is used, this branch returns hostname: '::1'; AddDatabaseScreen.getPayload then assigns port 6379 (AddDatabaseScreen.tsx:44-46) while preserving that generated name. Since DatabaseService.hasDefaultEndpointName only recognizes ${host}:${port} (database.service.ts:119-120), changing the host or port later treats ::1 as a custom alias and leaves it stale. The earlier explicit-port mismatch is fixed by unwrapping, but the new no-port test at parseRedisUrl.spec.ts:174 shows this path remains; construct the alias with the effective port.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Without a port, main already uses the bare host as the name for every host, so redis://localhost gives localhost the same way redis://[::1] now gives ::1. This PR keeps that behavior. Adding the default port to no-port names would change it for IPv4 hosts and hostnames too, so it belongs in a separate change.
What
Fixes #6554.
parseRedisUrlmatches the host part of a connection URL with[^:\/]+, so a bracketed IPv6 host such asredis://[::1]:6379never matches and the function returnsnull. The Connection URL field on the add database screen then reports the URL as invalid, pasting the URL into the manual form does not fill in host and port, and aredisUrlpassed through theredisinsight://deep link is ignored.This change lets the host be either a bracketed IPv6 address or the existing hostname pattern, in both the
redis[s]://form and the barehost:portform. The brackets are removed fromhost, so the API receives::1, which is what the manual form already accepts for IPv6.hostname, which is used as the default database alias, keeps the brackets ([::1]:6379) so the port stays readable. Hostnames, IPv4 addresses, and credentials that contain@or:are parsed as before.Testing
Added three cases to
redisinsight/ui/src/utils/tests/parseRedisUrl.spec.ts:[::1]:6379,redis://[::1], andrediss://user:pa@ss@[2001:db8::1]:6380/2.parseRedisUrlreturnsnull(3 failed, 17 passed).The add database screen, global URL handler, and home form utility specs also pass (5 suites, 39 tests). ESLint and Prettier pass on both changed files.
Note
Low Risk
Localized URL parsing change with added tests; no auth, persistence, or API contract changes beyond accepting previously failing IPv6 inputs.
Overview
Fixes IPv6 Redis connection URLs being rejected as invalid by extending
parseRedisUrlto recognize bracketed IPv6 hosts in both barehost:portstrings and fullredis[s]://URLs.The parser now matches
\[[^\]]+\]alongside the existing hostname pattern, then strips square brackets from the captured host so downstream connection logic receives addresses like::1(aligned with the manual form).hostname(default database alias) is built from the unwrapped host plus optional port, e.g.::1:6379.Three unit tests cover
[::1]:6379,redis://[::1], and a credentialedrediss://URL with[2001:db8::1]:6380/2. IPv4, DNS names, and passwords containing@or:are unchanged.Reviewed by Cursor Bugbot for commit 449eae6. Bugbot is set up for automated code reviews on this repo. Configure here.