Skip to content

Commit 825621d

Browse files
fix: IPv6 support, memory leak, portal recreation, and validation improvements
- Add IPv6-safe hostname/port parsing with proper regex handling - Fix memory leak by using stable event handlers with useCallback - Implement portal container reuse to prevent recreation overhead - Add proper profileId validation (alphanumeric + length limits) - Remove console.warn calls and gracefully hide unavailable data - Return null when no information is available instead of showing empty tooltip Co-authored-by: modelorona <modelorona@users.noreply.github.com>
1 parent 872554a commit 825621d

File tree

1 file changed

+120
-37
lines changed

1 file changed

+120
-37
lines changed

frontend/src/components/profile-info-tooltip.tsx

Lines changed: 120 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import classNames from "classnames";
2-
import { FC, useState, useRef, useEffect, useCallback } from "react";
2+
import { FC, useState, useRef, useEffect, useCallback, useMemo } from "react";
33
import { createPortal } from "react-dom";
44
import { Icons } from "./icons";
55
import { ClassNames } from "./classes";
@@ -11,31 +11,95 @@ interface ProfileInfoTooltipProps {
1111
className?: string;
1212
}
1313

14-
function getPortFromAdvanced(profile: LocalLoginProfile): string {
14+
// Profile ID validation: only allow alphanumeric, hyphens, underscores, max 64 chars
15+
function isValidProfileId(profileId: string): boolean {
16+
return typeof profileId === 'string' &&
17+
profileId.length > 0 &&
18+
profileId.length <= 64 &&
19+
/^[a-zA-Z0-9_-]+$/.test(profileId);
20+
}
21+
22+
// IPv6-safe hostname/port extraction
23+
function extractPortFromHostname(hostname: string): string {
24+
if (!hostname || typeof hostname !== 'string') {
25+
return 'Default';
26+
}
27+
28+
// Handle IPv6 addresses by checking for square brackets
29+
if (hostname.includes('[')) {
30+
const match = hostname.match(/\]:(\d+)$/);
31+
return match ? match[1] : 'Default';
32+
}
33+
34+
const parts = hostname.split(':');
35+
if (parts.length > 1) {
36+
const port = parts[parts.length - 1];
37+
// Check if the last part is numeric (a port)
38+
if (/^\d+$/.test(port)) {
39+
return port;
40+
}
41+
}
42+
return 'Default';
43+
}
44+
45+
function getPortFromAdvanced(profile: LocalLoginProfile): string | null {
1546
const dbType = profile.Type;
16-
const defaultPort = databaseTypeDropdownItems.find(item => item.id === dbType)!.extra!.Port;
47+
const defaultPortItem = databaseTypeDropdownItems.find(item => item.id === dbType);
48+
49+
if (!defaultPortItem?.extra?.Port) {
50+
return null; // No default port found, hide this info
51+
}
52+
53+
const defaultPort = defaultPortItem.extra.Port;
1754

1855
if (profile.Advanced) {
1956
const portObj = profile.Advanced.find(item => item.Key === 'Port');
2057
return portObj?.Value || defaultPort;
2158
}
2259

60+
// Check if hostname contains port info
61+
if (profile.Host) {
62+
const extractedPort = extractPortFromHostname(profile.Host);
63+
if (extractedPort !== 'Default') {
64+
return extractedPort;
65+
}
66+
}
67+
2368
return defaultPort;
2469
}
2570

26-
function getLastAccessedTime(profileId: string): string {
71+
function getLastAccessedTime(profileId: string): string | null {
72+
if (!isValidProfileId(profileId)) {
73+
return null; // Invalid profile ID, hide this info
74+
}
75+
2776
try {
2877
const lastAccessed = localStorage.getItem(`whodb_profile_last_accessed_${profileId}`);
2978
if (lastAccessed) {
3079
const date = new Date(lastAccessed);
80+
if (isNaN(date.getTime())) {
81+
return null; // Invalid date, hide this info
82+
}
3183
const timeZone = Intl.DateTimeFormat().resolvedOptions().timeZone;
3284
const formattedTimeZone = timeZone.replace(/_/g, ' ').split('/').join(' / ');
3385
return `${date.toLocaleDateString()} ${date.toLocaleTimeString([], { hour: '2-digit', minute: '2-digit' })} (${formattedTimeZone})`;
3486
}
3587
} catch (error) {
36-
console.warn('Failed to get last accessed time:', error);
88+
// Silently fail - return null to hide this info
3789
}
38-
return 'Never';
90+
return null;
91+
}
92+
93+
// Portal container reuse - create once and reuse
94+
let tooltipPortalContainer: HTMLDivElement | null = null;
95+
96+
function getTooltipPortalContainer(): HTMLDivElement {
97+
if (!tooltipPortalContainer) {
98+
tooltipPortalContainer = document.createElement('div');
99+
tooltipPortalContainer.id = 'whodb-tooltip-portal';
100+
document.body.appendChild(tooltipPortalContainer);
101+
}
102+
return tooltipPortalContainer;
39103
}
40104

41105
export const ProfileInfoTooltip: FC<ProfileInfoTooltipProps> = ({ profile, className }) => {
@@ -46,6 +110,12 @@ export const ProfileInfoTooltip: FC<ProfileInfoTooltipProps> = ({ profile, class
46110
const port = getPortFromAdvanced(profile);
47111
const lastAccessed = getLastAccessedTime(profile.Id);
48112

113+
// If no information is available, don't render the component
114+
const hasInfo = port !== null || lastAccessed !== null;
115+
if (!hasInfo) {
116+
return null;
117+
}
118+
49119
// Show tooltip to the right of the icon
50120
const showTooltip = useCallback(() => {
51121
if (btnRef.current) {
@@ -63,30 +133,35 @@ export const ProfileInfoTooltip: FC<ProfileInfoTooltipProps> = ({ profile, class
63133
setIsVisible(false);
64134
}, []);
65135

66-
// Click-away logic
67-
useEffect(() => {
68-
if (!isVisible) return;
69-
function handleClick(event: MouseEvent) {
70-
if (
71-
btnRef.current &&
72-
!btnRef.current.contains(event.target as Node)
73-
) {
74-
setIsVisible(false);
75-
}
136+
// Memoized event handlers to prevent recreation
137+
const handleClickAway = useCallback((event: MouseEvent) => {
138+
if (
139+
btnRef.current &&
140+
!btnRef.current.contains(event.target as Node)
141+
) {
142+
setIsVisible(false);
76143
}
77-
document.addEventListener("mousedown", handleClick);
78-
return () => document.removeEventListener("mousedown", handleClick);
79-
}, [isVisible]);
144+
}, []);
145+
146+
const handleKeyDown = useCallback((event: KeyboardEvent) => {
147+
if (event.key === "Escape") setIsVisible(false);
148+
}, []);
80149

81-
// Keyboard accessibility: close on Escape
150+
// Optimized event listeners - only add when visible, use stable handlers
82151
useEffect(() => {
83152
if (!isVisible) return;
84-
function handleKey(event: KeyboardEvent) {
85-
if (event.key === "Escape") setIsVisible(false);
86-
}
87-
document.addEventListener("keydown", handleKey);
88-
return () => document.removeEventListener("keydown", handleKey);
89-
}, [isVisible]);
153+
154+
document.addEventListener("mousedown", handleClickAway);
155+
document.addEventListener("keydown", handleKeyDown);
156+
157+
return () => {
158+
document.removeEventListener("mousedown", handleClickAway);
159+
document.removeEventListener("keydown", handleKeyDown);
160+
};
161+
}, [isVisible, handleClickAway, handleKeyDown]);
162+
163+
// Memoize portal container to prevent recreation
164+
const portalContainer = useMemo(() => getTooltipPortalContainer(), []);
90165

91166
const tooltip = isVisible && tooltipPos
92167
? createPortal(
@@ -106,14 +181,18 @@ export const ProfileInfoTooltip: FC<ProfileInfoTooltipProps> = ({ profile, class
106181
}}
107182
>
108183
<div className="space-y-1">
109-
<div className="flex justify-between">
110-
<span className="text-gray-600 dark:text-gray-400">Port:</span>
111-
<span className={ClassNames.Text}>{port}</span>
112-
</div>
113-
<div className="flex justify-between">
114-
<span className="text-gray-600 dark:text-gray-400">Last Logged In:&nbsp;</span>
115-
<span className={ClassNames.Text}>{lastAccessed}</span>
116-
</div>
184+
{port !== null && (
185+
<div className="flex justify-between">
186+
<span className="text-gray-600 dark:text-gray-400">Port:</span>
187+
<span className={ClassNames.Text}>{port}</span>
188+
</div>
189+
)}
190+
{lastAccessed !== null && (
191+
<div className="flex justify-between">
192+
<span className="text-gray-600 dark:text-gray-400">Last Logged In:&nbsp;</span>
193+
<span className={ClassNames.Text}>{lastAccessed}</span>
194+
</div>
195+
)}
117196
</div>
118197
<div
119198
className="absolute top-1/2 left-0 -translate-x-full -translate-y-1/2"
@@ -122,7 +201,7 @@ export const ProfileInfoTooltip: FC<ProfileInfoTooltipProps> = ({ profile, class
122201
<div className="w-0 h-0 border-t-4 border-b-4 border-r-4 border-t-transparent border-b-transparent border-r-gray-200 dark:border-r-white/20"></div>
123202
</div>
124203
</div>,
125-
document.body
204+
portalContainer
126205
)
127206
: null;
128207

@@ -144,11 +223,15 @@ export const ProfileInfoTooltip: FC<ProfileInfoTooltipProps> = ({ profile, class
144223
);
145224
};
146225

147-
// Utility function to update last accessed time
226+
// Utility function to update last accessed time with validation
148227
export function updateProfileLastAccessed(profileId: string): void {
228+
if (!isValidProfileId(profileId)) {
229+
return; // Silently fail for invalid profile IDs
230+
}
231+
149232
try {
150233
localStorage.setItem(`whodb_profile_last_accessed_${profileId}`, new Date().toISOString());
151234
} catch (error) {
152-
console.warn('Failed to update last accessed time:', error);
235+
// Silently fail - localStorage may be full or disabled
153236
}
154237
}

0 commit comments

Comments
 (0)