fix: support for IPv6 address literals in HOST env var#2614
fix: support for IPv6 address literals in HOST env var#2614kouellette wants to merge 3 commits intoseerr-team:developfrom
Conversation
When pages would build URLs for API requests, process.env.HOST would be directly concatenated with process.env.PORT (e.g., `0.0.0.0:5055`). This works for IPv4 address literals but does not work for IPv6 address literals because `:::5055` for example is invalid. This commit adds a check if process.env.HOST contains a colon (:) and if so, wraps the host in square brackets. E.g., `[::]:5055`, which is valid. This allows for users to set the HOST environment variable to IPv6 address literals. fix seerr-team#2612
📝 WalkthroughWalkthroughCentralized host:port resolution added via Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/utils/urlHelper.ts`:
- Around line 3-5: The current host-wrapping logic in src/utils/urlHelper.ts
blindly brackets any host containing ':' which double-brackets already-bracketed
IPv6 literals; update the guard in the host handling (the block that checks
host.includes(':')) to first detect if host already startsWith('[') and
endsWith(']') and only wrap with `[...]` when those brackets are absent, leaving
already-bracketed values unchanged.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/pages/_app.tsxsrc/pages/collection/[collectionId]/index.tsxsrc/pages/movie/[movieId]/index.tsxsrc/pages/tv/[tvId]/index.tsxsrc/utils/urlHelper.ts
| if (host.includes(':')) { | ||
| // If host includes a colon, it's an IPv6 literal and needs to be placed in square brackets | ||
| host = `[${host}]`; |
There was a problem hiding this comment.
Avoid double-bracketing pre-bracketed IPv6 values.
If HOST is already configured as a bracketed literal (e.g. [::1]), Line 5 produces [[::1]], which breaks URL construction. Add a guard before wrapping.
💡 Proposed fix
export function getHostAndPort(): string {
let host = process.env.HOST || 'localhost';
- if (host.includes(':')) {
+ if (
+ host.includes(':') &&
+ !(host.startsWith('[') && host.endsWith(']'))
+ ) {
// If host includes a colon, it's an IPv6 literal and needs to be placed in square brackets
host = `[${host}]`;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (host.includes(':')) { | |
| // If host includes a colon, it's an IPv6 literal and needs to be placed in square brackets | |
| host = `[${host}]`; | |
| if ( | |
| host.includes(':') && | |
| !(host.startsWith('[') && host.endsWith(']')) | |
| ) { | |
| // If host includes a colon, it's an IPv6 literal and needs to be placed in square brackets | |
| host = `[${host}]`; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/urlHelper.ts` around lines 3 - 5, The current host-wrapping logic
in src/utils/urlHelper.ts blindly brackets any host containing ':' which
double-brackets already-bracketed IPv6 literals; update the guard in the host
handling (the block that checks host.includes(':')) to first detect if host
already startsWith('[') and endsWith(']') and only wrap with `[...]` when those
brackets are absent, leaving already-bracketed values unchanged.
There was a problem hiding this comment.
Pull request overview
Adds a shared helper for constructing internal HOST:PORT strings so server-side page requests can correctly target IPv6 address literals (by bracketing them) when building URLs for local API calls.
Changes:
- Introduced
getHostAndPort()utility for consistent internal host/port formatting (including IPv6 bracketing). - Updated server-side Axios calls in
_appand several detail pages to use the new helper instead of inlineprocess.env.HOST/process.env.PORTstring concatenation.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/urlHelper.ts | Adds centralized host/port formatting used for internal API URL construction. |
| src/pages/_app.tsx | Switches internal settings/user Axios requests to use the shared host/port helper. |
| src/pages/movie/[movieId]/index.tsx | Uses the shared host/port helper for server-side movie detail API calls. |
| src/pages/tv/[tvId]/index.tsx | Uses the shared host/port helper for server-side TV detail API calls. |
| src/pages/collection/[collectionId]/index.tsx | Uses the shared host/port helper for server-side collection detail API calls. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Maybe overkill but this lib could help : https://www.npmjs.com/package/ip-address |
Yeah probably overkill. Worst case you don't properly configure the host address and the app crashes at startup? |
Correct. However, I noticed this error wasn't being caught and logged so I updated the PR for this. Example logs when |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/index.ts (1)
281-284: Consider using structured logging for consistency.The error handler works correctly. For consistency with other logging calls in this file (e.g., lines 270, 276), consider using the structured format with a
labelproperty.♻️ Suggested improvement
httpServer.on('error', (err) => { - logger.error('Listen error:', err); + logger.error('Failed to start server', { + label: 'Server', + message: err.message, + }); process.exit(1); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/index.ts` around lines 281 - 284, Update the httpServer error handler to use the same structured logging shape as other calls: replace the freeform logger.error('Listen error:', err) in the httpServer.on('error', (err) => { ... }) callback with a single structured call that passes an object including a label (e.g., label: 'server:listen') and the error (e.g., err) so the log matches the format used at lines like the other logger.error/logger.info calls; ensure you only modify the logger.error invocation inside the httpServer.on('error') handler to emit the label and error fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@server/index.ts`:
- Around line 281-284: Update the httpServer error handler to use the same
structured logging shape as other calls: replace the freeform
logger.error('Listen error:', err) in the httpServer.on('error', (err) => { ...
}) callback with a single structured call that passes an object including a
label (e.g., label: 'server:listen') and the error (e.g., err) so the log
matches the format used at lines like the other logger.error/logger.info calls;
ensure you only modify the logger.error invocation inside the
httpServer.on('error') handler to emit the label and error fields.
Description
When pages would build URLs for API requests, process.env.HOST would be directly concatenated with process.env.PORT (e.g.,
0.0.0.0:5055). This works for IPv4 address literals but does not work for IPv6 address literals because:::5055for example is invalid. This commit adds a check if process.env.HOST contains a colon (:) and if so, wraps the host in square brackets. E.g.,[::]:5055, which is valid. This allows for users to set the HOST environment variable to IPv6 address literals.How Has This Been Tested?
This was tested on a Debian 13 VM running kernel 6.12.48+deb13-amd64 for the server and a 13-inch 2022 MacBook Pro with the Arc browser Version 1.135.0 (75383), Chromium Engine Version 145.0.7632.110.
Tested by setting
HOSTenv var to::and another IPv6 literal and ensured pages loaded as expected and the error observed in the ticket was no longer occurring.Screenshots / Logs (if applicable)
Checklist:
pnpm buildpnpm i18n:extractSummary by CodeRabbit