mirror of
https://github.com/lobehub/lobehub.git
synced 2026-03-26 13:19:34 +07:00
♻️ refactor: replace window.open interception with setWindowOpenHandler in main process
- Use Electron's setWindowOpenHandler in Browser.ts to intercept external links - Remove window.open override from preload script - Remove pushState/replaceState interception as no longer needed - Update tests to reflect changes This moves external link handling to the main process for better security and simplifies the preload script.
This commit is contained in:
@@ -4,7 +4,7 @@ import { join } from 'node:path';
|
||||
import { APP_WINDOW_MIN_SIZE } from '@lobechat/desktop-bridge';
|
||||
import type { MainBroadcastEventKey, MainBroadcastParams } from '@lobechat/electron-client-ipc';
|
||||
import type { BrowserWindowConstructorOptions } from 'electron';
|
||||
import { BrowserWindow, ipcMain, screen, session as electronSession } from 'electron';
|
||||
import { BrowserWindow, ipcMain, screen, session as electronSession, shell } from 'electron';
|
||||
|
||||
import { preloadDir, resourcesDir } from '@/const/dir';
|
||||
import { isMac } from '@/const/env';
|
||||
@@ -163,6 +163,9 @@ export default class Browser {
|
||||
|
||||
// Setup event listeners
|
||||
this.setupEventListeners(browserWindow);
|
||||
|
||||
// Setup external link handler (prevents opening new windows in renderer)
|
||||
this.setupWindowOpenHandler(browserWindow);
|
||||
}
|
||||
|
||||
private initiateContentLoading(): void {
|
||||
@@ -187,6 +190,26 @@ export default class Browser {
|
||||
this.setupContextMenu(browserWindow);
|
||||
}
|
||||
|
||||
/**
|
||||
* Setup window open handler to intercept external links
|
||||
* Prevents opening new windows in renderer and uses system browser instead
|
||||
*/
|
||||
private setupWindowOpenHandler(browserWindow: BrowserWindow): void {
|
||||
logger.debug(`[${this.identifier}] Setting up window open handler for external links`);
|
||||
|
||||
browserWindow.webContents.setWindowOpenHandler(({ url }) => {
|
||||
logger.info(`[${this.identifier}] Intercepted window open for URL: ${url}`);
|
||||
|
||||
// Open external URL in system browser
|
||||
shell.openExternal(url).catch((error) => {
|
||||
logger.error(`[${this.identifier}] Failed to open external URL: ${url}`, error);
|
||||
});
|
||||
|
||||
// Deny creating new window in renderer
|
||||
return { action: 'deny' };
|
||||
});
|
||||
}
|
||||
|
||||
private setupWillPreventUnloadListener(browserWindow: BrowserWindow): void {
|
||||
logger.debug(`[${this.identifier}] Setting up 'will-prevent-unload' event listener.`);
|
||||
browserWindow.webContents.on('will-prevent-unload', (event) => {
|
||||
|
||||
@@ -41,6 +41,7 @@ const { mockBrowserWindow, mockNativeTheme, mockIpcMain, mockScreen, MockBrowser
|
||||
},
|
||||
},
|
||||
on: vi.fn(),
|
||||
setWindowOpenHandler: vi.fn(),
|
||||
},
|
||||
};
|
||||
|
||||
|
||||
@@ -39,52 +39,6 @@ describe('setupRouteInterceptors', () => {
|
||||
document.body.innerHTML = '';
|
||||
});
|
||||
|
||||
describe('window.open interception', () => {
|
||||
it('should intercept external URL and invoke openExternalLink', () => {
|
||||
setupRouteInterceptors();
|
||||
|
||||
const externalUrl = 'https://google.com';
|
||||
const result = window.open(externalUrl, '_blank');
|
||||
|
||||
expect(invoke).toHaveBeenCalledWith('system.openExternalLink', externalUrl);
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
it('should intercept URL object for external link', () => {
|
||||
setupRouteInterceptors();
|
||||
|
||||
const externalUrl = new URL('https://github.com');
|
||||
const result = window.open(externalUrl, '_blank');
|
||||
|
||||
expect(invoke).toHaveBeenCalledWith('system.openExternalLink', 'https://github.com/');
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
it('should allow internal link to proceed with original window.open', () => {
|
||||
setupRouteInterceptors();
|
||||
|
||||
const originalWindowOpen = window.open;
|
||||
const internalUrl = 'http://localhost:3000/settings';
|
||||
|
||||
// We can't fully test the original behavior in happy-dom, but we can verify invoke is not called
|
||||
window.open(internalUrl);
|
||||
|
||||
expect(invoke).not.toHaveBeenCalledWith('system.openExternalLink', expect.anything());
|
||||
});
|
||||
|
||||
it('should handle relative URL that resolves as internal link', () => {
|
||||
setupRouteInterceptors();
|
||||
|
||||
// In happy-dom, 'invalid-url' is resolved relative to window.location.href
|
||||
// So it becomes 'http://localhost:3000/invalid-url' which is internal
|
||||
const relativeUrl = 'invalid-url';
|
||||
window.open(relativeUrl);
|
||||
|
||||
// Since it's internal, it won't call invoke for external link
|
||||
expect(invoke).not.toHaveBeenCalledWith('system.openExternalLink', expect.anything());
|
||||
});
|
||||
});
|
||||
|
||||
describe('link click interception', () => {
|
||||
it('should intercept external link clicks', async () => {
|
||||
setupRouteInterceptors();
|
||||
@@ -189,184 +143,6 @@ describe('setupRouteInterceptors', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('history.pushState interception', () => {
|
||||
it('should intercept pushState for matched routes', () => {
|
||||
setupRouteInterceptors();
|
||||
|
||||
const matchedRoute = {
|
||||
description: 'Developer Tools',
|
||||
enabled: true,
|
||||
pathPrefix: '/desktop/devtools',
|
||||
targetWindow: 'devtools',
|
||||
};
|
||||
vi.mocked(findMatchingRoute).mockReturnValue(matchedRoute);
|
||||
|
||||
const originalLength = history.length;
|
||||
history.pushState({}, '', '/desktop/devtools');
|
||||
|
||||
expect(findMatchingRoute).toHaveBeenCalledWith('/desktop/devtools');
|
||||
expect(invoke).toHaveBeenCalledWith('windows.interceptRoute', {
|
||||
path: '/desktop/devtools',
|
||||
source: 'push-state',
|
||||
url: 'http://localhost:3000/desktop/devtools',
|
||||
});
|
||||
// Ensure navigation was prevented
|
||||
expect(history.length).toBe(originalLength);
|
||||
});
|
||||
|
||||
it('should not intercept if already on target page', () => {
|
||||
setupRouteInterceptors();
|
||||
|
||||
vi.stubGlobal('location', {
|
||||
href: 'http://localhost:3000/desktop/devtools/console',
|
||||
origin: 'http://localhost:3000',
|
||||
pathname: '/desktop/devtools/console',
|
||||
});
|
||||
|
||||
const matchedRoute = {
|
||||
description: 'Developer Tools',
|
||||
enabled: true,
|
||||
pathPrefix: '/desktop/devtools',
|
||||
targetWindow: 'devtools',
|
||||
};
|
||||
vi.mocked(findMatchingRoute).mockReturnValue(matchedRoute);
|
||||
|
||||
history.pushState({}, '', '/desktop/devtools/network');
|
||||
|
||||
expect(consoleLogSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Skip pushState interception'),
|
||||
);
|
||||
});
|
||||
|
||||
it('should allow pushState for non-matched routes', () => {
|
||||
setupRouteInterceptors();
|
||||
|
||||
vi.mocked(findMatchingRoute).mockReturnValue(undefined);
|
||||
|
||||
history.pushState({}, '', '/chat/new');
|
||||
|
||||
expect(invoke).not.toHaveBeenCalledWith('windows.interceptRoute', expect.anything());
|
||||
});
|
||||
|
||||
it('should handle pushState errors gracefully', () => {
|
||||
setupRouteInterceptors();
|
||||
|
||||
vi.mocked(findMatchingRoute).mockImplementation(() => {
|
||||
throw new Error('Route matching error');
|
||||
});
|
||||
|
||||
history.pushState({}, '', '/some/path');
|
||||
|
||||
expect(consoleErrorSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('pushState interception error'),
|
||||
expect.any(Error),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('history.replaceState interception', () => {
|
||||
it('should intercept replaceState for matched routes', () => {
|
||||
setupRouteInterceptors();
|
||||
|
||||
const matchedRoute = {
|
||||
description: 'Developer Tools',
|
||||
enabled: true,
|
||||
pathPrefix: '/desktop/devtools',
|
||||
targetWindow: 'devtools',
|
||||
};
|
||||
vi.mocked(findMatchingRoute).mockReturnValue(matchedRoute);
|
||||
|
||||
history.replaceState({}, '', '/desktop/devtools');
|
||||
|
||||
expect(findMatchingRoute).toHaveBeenCalledWith('/desktop/devtools');
|
||||
expect(invoke).toHaveBeenCalledWith('windows.interceptRoute', {
|
||||
path: '/desktop/devtools',
|
||||
source: 'replace-state',
|
||||
url: 'http://localhost:3000/desktop/devtools',
|
||||
});
|
||||
});
|
||||
|
||||
it('should not intercept if already on target page', () => {
|
||||
setupRouteInterceptors();
|
||||
|
||||
vi.stubGlobal('location', {
|
||||
href: 'http://localhost:3000/desktop/devtools/console',
|
||||
origin: 'http://localhost:3000',
|
||||
pathname: '/desktop/devtools/console',
|
||||
});
|
||||
|
||||
const matchedRoute = {
|
||||
description: 'Developer Tools',
|
||||
enabled: true,
|
||||
pathPrefix: '/desktop/devtools',
|
||||
targetWindow: 'devtools',
|
||||
};
|
||||
vi.mocked(findMatchingRoute).mockReturnValue(matchedRoute);
|
||||
|
||||
history.replaceState({}, '', '/desktop/devtools/network');
|
||||
|
||||
expect(consoleLogSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Skip replaceState interception'),
|
||||
);
|
||||
});
|
||||
|
||||
it('should allow replaceState for non-matched routes', () => {
|
||||
setupRouteInterceptors();
|
||||
|
||||
vi.mocked(findMatchingRoute).mockReturnValue(undefined);
|
||||
|
||||
history.replaceState({}, '', '/chat/session-123');
|
||||
|
||||
expect(invoke).not.toHaveBeenCalledWith('windows.interceptRoute', expect.anything());
|
||||
});
|
||||
});
|
||||
|
||||
describe('error event interception', () => {
|
||||
it('should prevent navigation errors for prevented paths', () => {
|
||||
setupRouteInterceptors();
|
||||
|
||||
// First trigger a route interception to add path to preventedPaths
|
||||
const matchedRoute = {
|
||||
description: 'Developer Tools',
|
||||
enabled: true,
|
||||
pathPrefix: '/desktop/devtools',
|
||||
targetWindow: 'devtools',
|
||||
};
|
||||
vi.mocked(findMatchingRoute).mockReturnValue(matchedRoute);
|
||||
history.pushState({}, '', '/desktop/devtools');
|
||||
|
||||
// Now trigger an error event with navigation in the message
|
||||
const errorEvent = new ErrorEvent('error', {
|
||||
bubbles: true,
|
||||
cancelable: true,
|
||||
message: 'navigation error occurred',
|
||||
});
|
||||
const preventDefaultSpy = vi.spyOn(errorEvent, 'preventDefault');
|
||||
|
||||
window.dispatchEvent(errorEvent);
|
||||
|
||||
expect(preventDefaultSpy).toHaveBeenCalled();
|
||||
expect(consoleLogSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Captured possible routing error'),
|
||||
);
|
||||
});
|
||||
|
||||
it('should not prevent non-navigation errors', () => {
|
||||
setupRouteInterceptors();
|
||||
|
||||
const errorEvent = new ErrorEvent('error', {
|
||||
bubbles: true,
|
||||
cancelable: true,
|
||||
message: 'some other error',
|
||||
});
|
||||
const preventDefaultSpy = vi.spyOn(errorEvent, 'preventDefault');
|
||||
|
||||
window.dispatchEvent(errorEvent);
|
||||
|
||||
expect(preventDefaultSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('interceptRoute helper', () => {
|
||||
it('should handle successful route interception', async () => {
|
||||
vi.mocked(invoke).mockResolvedValue(undefined);
|
||||
@@ -381,13 +157,18 @@ describe('setupRouteInterceptors', () => {
|
||||
};
|
||||
vi.mocked(findMatchingRoute).mockReturnValue(matchedRoute);
|
||||
|
||||
history.pushState({}, '', '/desktop/devtools');
|
||||
const link = document.createElement('a');
|
||||
link.href = 'http://localhost:3000/desktop/devtools';
|
||||
document.body.append(link);
|
||||
|
||||
const clickEvent = new MouseEvent('click', { bubbles: true, cancelable: true });
|
||||
link.dispatchEvent(clickEvent);
|
||||
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
|
||||
expect(invoke).toHaveBeenCalledWith('windows.interceptRoute', {
|
||||
path: '/desktop/devtools',
|
||||
source: 'push-state',
|
||||
source: 'link-click',
|
||||
url: 'http://localhost:3000/desktop/devtools',
|
||||
});
|
||||
});
|
||||
@@ -406,12 +187,17 @@ describe('setupRouteInterceptors', () => {
|
||||
};
|
||||
vi.mocked(findMatchingRoute).mockReturnValue(matchedRoute);
|
||||
|
||||
history.pushState({}, '', '/desktop/devtools');
|
||||
const link = document.createElement('a');
|
||||
link.href = 'http://localhost:3000/desktop/devtools';
|
||||
document.body.append(link);
|
||||
|
||||
const clickEvent = new MouseEvent('click', { bubbles: true, cancelable: true });
|
||||
link.dispatchEvent(clickEvent);
|
||||
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
|
||||
expect(consoleErrorSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Route interception (push-state) call failed'),
|
||||
expect.stringContaining('Route interception (link-click) call failed'),
|
||||
error,
|
||||
);
|
||||
});
|
||||
|
||||
@@ -2,11 +2,7 @@ import { findMatchingRoute } from '~common/routes';
|
||||
|
||||
import { invoke } from './invoke';
|
||||
|
||||
const interceptRoute = async (
|
||||
path: string,
|
||||
source: 'link-click' | 'push-state' | 'replace-state',
|
||||
url: string,
|
||||
) => {
|
||||
const interceptRoute = async (path: string, source: 'link-click', url: string) => {
|
||||
console.log(`[preload] Intercepted ${source} and prevented default behavior:`, path);
|
||||
|
||||
// Use electron-client-ipc's dispatch method
|
||||
@@ -16,43 +12,13 @@ const interceptRoute = async (
|
||||
console.error(`[preload] Route interception (${source}) call failed`, e);
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* Route interceptor - Responsible for capturing and intercepting client-side route navigation
|
||||
*/
|
||||
export const setupRouteInterceptors = function () {
|
||||
console.log('[preload] Setting up route interceptors');
|
||||
|
||||
// Store prevented paths to avoid pushState duplicate triggers
|
||||
const preventedPaths = new Set<string>();
|
||||
|
||||
// Override window.open method to intercept JavaScript calls
|
||||
const originalWindowOpen = window.open;
|
||||
window.open = function (url?: string | URL, target?: string, features?: string) {
|
||||
if (url) {
|
||||
try {
|
||||
const urlString = typeof url === 'string' ? url : url.toString();
|
||||
const urlObj = new URL(urlString, window.location.href);
|
||||
|
||||
// Check if it's an external link
|
||||
if (urlObj.origin !== window.location.origin) {
|
||||
console.log(`[preload] Intercepted window.open for external URL:`, urlString);
|
||||
// Call main process to handle external link
|
||||
invoke('system.openExternalLink', urlString);
|
||||
return null; // Return null to indicate no window was opened
|
||||
}
|
||||
} catch (error) {
|
||||
// Handle invalid URL or special protocol
|
||||
console.error(`[preload] Intercepted window.open for special protocol:`, url);
|
||||
console.error(error);
|
||||
invoke('system.openExternalLink', typeof url === 'string' ? url : url.toString());
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
// For internal links, call original window.open
|
||||
return originalWindowOpen.call(window, url, target, features);
|
||||
};
|
||||
|
||||
// Intercept all a tag click events - For Next.js Link component
|
||||
document.addEventListener(
|
||||
'click',
|
||||
@@ -112,101 +78,5 @@ export const setupRouteInterceptors = function () {
|
||||
true,
|
||||
);
|
||||
|
||||
// Intercept history API (for capturing Next.js useRouter().push/replace etc.)
|
||||
const originalPushState = history.pushState;
|
||||
const originalReplaceState = history.replaceState;
|
||||
|
||||
// Override pushState
|
||||
history.pushState = function () {
|
||||
const url = arguments[2];
|
||||
if (typeof url === 'string') {
|
||||
try {
|
||||
// Only handle relative paths or current domain URLs
|
||||
const parsedUrl = new URL(url, window.location.origin);
|
||||
|
||||
// Use shared config to check if interception is needed
|
||||
const matchedRoute = findMatchingRoute(parsedUrl.pathname);
|
||||
|
||||
// Check if this navigation needs interception
|
||||
if (matchedRoute) {
|
||||
// Check if current page is already under target path, if so don't intercept
|
||||
const currentPath = window.location.pathname;
|
||||
const isAlreadyInTargetPage = currentPath.startsWith(matchedRoute.pathPrefix);
|
||||
|
||||
// If already in target page, don't intercept, let default navigation continue
|
||||
if (isAlreadyInTargetPage) {
|
||||
console.log(
|
||||
`[preload] Skip pushState interception for ${parsedUrl.pathname} because already in target page ${matchedRoute.pathPrefix}`,
|
||||
);
|
||||
return Reflect.apply(originalPushState, this, arguments);
|
||||
}
|
||||
|
||||
// Add this path to prevented set
|
||||
preventedPaths.add(parsedUrl.pathname);
|
||||
|
||||
interceptRoute(parsedUrl.pathname, 'push-state', parsedUrl.href);
|
||||
|
||||
// Don't execute original pushState operation, prevent navigation
|
||||
// But return undefined to avoid errors
|
||||
return;
|
||||
}
|
||||
} catch (err) {
|
||||
console.error('[preload] pushState interception error:', err);
|
||||
}
|
||||
}
|
||||
return Reflect.apply(originalPushState, this, arguments);
|
||||
};
|
||||
|
||||
// Override replaceState
|
||||
history.replaceState = function () {
|
||||
const url = arguments[2];
|
||||
if (typeof url === 'string') {
|
||||
try {
|
||||
const parsedUrl = new URL(url, window.location.origin);
|
||||
|
||||
// Use shared config to check if interception is needed
|
||||
const matchedRoute = findMatchingRoute(parsedUrl.pathname);
|
||||
|
||||
// Check if this navigation needs interception
|
||||
if (matchedRoute) {
|
||||
// Check if current page is already under target path, if so don't intercept
|
||||
const currentPath = window.location.pathname;
|
||||
const isAlreadyInTargetPage = currentPath.startsWith(matchedRoute.pathPrefix);
|
||||
|
||||
// If already in target page, don't intercept, let default navigation continue
|
||||
if (isAlreadyInTargetPage) {
|
||||
console.log(
|
||||
`[preload] Skip replaceState interception for ${parsedUrl.pathname} because already in target page ${matchedRoute.pathPrefix}`,
|
||||
);
|
||||
return Reflect.apply(originalReplaceState, this, arguments);
|
||||
}
|
||||
|
||||
// Add to prevented set
|
||||
preventedPaths.add(parsedUrl.pathname);
|
||||
|
||||
interceptRoute(parsedUrl.pathname, 'replace-state', parsedUrl.href);
|
||||
|
||||
// Prevent navigation
|
||||
return;
|
||||
}
|
||||
} catch (err) {
|
||||
console.error('[preload] replaceState interception error:', err);
|
||||
}
|
||||
}
|
||||
return Reflect.apply(originalReplaceState, this, arguments);
|
||||
};
|
||||
|
||||
// Listen and intercept routing errors - Sometimes Next.js tries to recover navigation on routing errors
|
||||
window.addEventListener(
|
||||
'error',
|
||||
function (e) {
|
||||
if (e.message && e.message.includes('navigation') && preventedPaths.size > 0) {
|
||||
console.log('[preload] Captured possible routing error, preventing default behavior');
|
||||
e.preventDefault();
|
||||
}
|
||||
},
|
||||
true,
|
||||
);
|
||||
|
||||
console.log('[preload] Route interceptors setup completed');
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user