diff --git a/readme.hints.md b/readme.hints.md index e7deae1..f7d9ad3 100644 --- a/readme.hints.md +++ b/readme.hints.md @@ -640,4 +640,25 @@ export function setupBidirectionalForwarding( - **Neutral**: Half-open connections still available when needed (opt-in) ### Migration Notes -No configuration changes needed. The fix applies to all proxy chains automatically. \ No newline at end of file +No configuration changes needed. The fix applies to all proxy chains automatically. + +## Socket Cleanup Handler Deprecation (v19.5.15+) + +### Issue +The deprecated `createSocketCleanupHandler()` function was still being used in forwarding handlers, despite being marked as deprecated. + +### Solution +Updated all forwarding handlers to use the new centralized socket utilities: +1. **Replaced `createSocketCleanupHandler()`** with `setupBidirectionalForwarding()` in: + - `https-terminate-to-https-handler.ts` + - `https-terminate-to-http-handler.ts` +2. **Removed deprecated function** from `socket-utils.ts` + +### Benefits +- Consistent socket handling across all handlers +- Proper cleanup in proxy chains (no half-open connections by default) +- Better backpressure handling with the centralized implementation +- Reduced code duplication + +### Migration Notes +No user-facing changes. All forwarding handlers now use the same robust socket handling as the main SmartProxy connection handler. \ No newline at end of file diff --git a/ts/core/utils/socket-utils.ts b/ts/core/utils/socket-utils.ts index 421d3c6..86cae3c 100644 --- a/ts/core/utils/socket-utils.ts +++ b/ts/core/utils/socket-utils.ts @@ -67,37 +67,6 @@ export function cleanupSocket( }); } -/** - * Create a cleanup handler for paired sockets (client and server) - * @param clientSocket The client socket - * @param serverSocket The server socket (optional) - * @param onCleanup Optional callback when cleanup is done - * @returns A cleanup function that can be called multiple times safely - * @deprecated Use createIndependentSocketHandlers for better half-open support - */ -export function createSocketCleanupHandler( - clientSocket: plugins.net.Socket | plugins.tls.TLSSocket, - serverSocket?: plugins.net.Socket | plugins.tls.TLSSocket | null, - onCleanup?: (reason: string) => void -): (reason: string) => void { - let cleanedUp = false; - - return (reason: string) => { - if (cleanedUp) return; - cleanedUp = true; - - // Cleanup both sockets (old behavior - too aggressive) - cleanupSocket(clientSocket, 'client', { immediate: true }); - if (serverSocket) { - cleanupSocket(serverSocket, 'server', { immediate: true }); - } - - // Call cleanup callback if provided - if (onCleanup) { - onCleanup(reason); - } - }; -} /** * Create independent cleanup handlers for paired sockets that support half-open connections @@ -278,19 +247,6 @@ export function setupBidirectionalForwarding( return { cleanupClient, cleanupServer }; } -/** - * Pipe two sockets together with proper cleanup on either end - * @param socket1 First socket - * @param socket2 Second socket - */ -export function pipeSockets( - socket1: plugins.net.Socket | plugins.tls.TLSSocket, - socket2: plugins.net.Socket | plugins.tls.TLSSocket -): void { - socket1.pipe(socket2); - socket2.pipe(socket1); -} - /** * Create a socket with immediate error handling to prevent crashes * @param options Socket creation options diff --git a/ts/forwarding/handlers/https-terminate-to-http-handler.ts b/ts/forwarding/handlers/https-terminate-to-http-handler.ts index 094e43f..4b8b180 100644 --- a/ts/forwarding/handlers/https-terminate-to-http-handler.ts +++ b/ts/forwarding/handlers/https-terminate-to-http-handler.ts @@ -2,7 +2,7 @@ import * as plugins from '../../plugins.js'; import { ForwardingHandler } from './base-handler.js'; import type { IForwardConfig } from '../config/forwarding-types.js'; import { ForwardingHandlerEvents } from '../config/forwarding-types.js'; -import { createSocketCleanupHandler, setupSocketHandlers, createSocketWithErrorHandler } from '../../core/utils/socket-utils.js'; +import { setupSocketHandlers, createSocketWithErrorHandler, setupBidirectionalForwarding } from '../../core/utils/socket-utils.js'; /** * Handler for HTTPS termination with HTTP backend @@ -100,19 +100,30 @@ export class HttpsTerminateToHttpHandler extends ForwardingHandler { let backendSocket: plugins.net.Socket | null = null; let dataBuffer = Buffer.alloc(0); let connectionEstablished = false; + let forwardingSetup = false; - // Create cleanup handler for all sockets - const handleClose = createSocketCleanupHandler(tlsSocket, backendSocket, (reason) => { - this.emit(ForwardingHandlerEvents.DISCONNECTED, { - remoteAddress, - reason - }); - dataBuffer = Buffer.alloc(0); - connectionEstablished = false; - }); + // Set up initial error handling for TLS socket + const tlsCleanupHandler = (reason: string) => { + if (!forwardingSetup) { + // If forwarding not set up yet, emit disconnected and cleanup + this.emit(ForwardingHandlerEvents.DISCONNECTED, { + remoteAddress, + reason + }); + dataBuffer = Buffer.alloc(0); + connectionEstablished = false; + + if (!tlsSocket.destroyed) { + tlsSocket.destroy(); + } + if (backendSocket && !backendSocket.destroyed) { + backendSocket.destroy(); + } + } + // If forwarding is setup, setupBidirectionalForwarding will handle cleanup + }; - // Set up error handling with our cleanup utility - setupSocketHandlers(tlsSocket, handleClose, undefined, 'tls'); + setupSocketHandlers(tlsSocket, tlsCleanupHandler, undefined, 'tls'); // Set timeout const timeout = this.getTimeout(); @@ -123,7 +134,7 @@ export class HttpsTerminateToHttpHandler extends ForwardingHandler { remoteAddress, error: 'TLS connection timeout' }); - handleClose('timeout'); + tlsCleanupHandler('timeout'); }); // Handle TLS data @@ -172,30 +183,33 @@ export class HttpsTerminateToHttpHandler extends ForwardingHandler { dataBuffer = Buffer.alloc(0); } - // Set up bidirectional data flow - tlsSocket.pipe(backendSocket!); - backendSocket!.pipe(tlsSocket); + // Now set up bidirectional forwarding with proper cleanup + forwardingSetup = true; + setupBidirectionalForwarding(tlsSocket, backendSocket!, { + onCleanup: (reason) => { + this.emit(ForwardingHandlerEvents.DISCONNECTED, { + remoteAddress, + reason + }); + dataBuffer = Buffer.alloc(0); + connectionEstablished = false; + forwardingSetup = false; + }, + enableHalfOpen: false // Close both when one closes + }); } }); - // Update the cleanup handler with the backend socket - const newHandleClose = createSocketCleanupHandler(tlsSocket, backendSocket, (reason) => { - this.emit(ForwardingHandlerEvents.DISCONNECTED, { - remoteAddress, - reason - }); - dataBuffer = Buffer.alloc(0); - connectionEstablished = false; - }); - - // Set up handlers for backend socket - setupSocketHandlers(backendSocket, newHandleClose, undefined, 'backend'); - + // Additional error logging for backend socket backendSocket.on('error', (error) => { - this.emit(ForwardingHandlerEvents.ERROR, { - remoteAddress, - error: `Target connection error: ${error.message}` - }); + if (!connectionEstablished) { + // Connection failed during setup + this.emit(ForwardingHandlerEvents.ERROR, { + remoteAddress, + error: `Target connection error: ${error.message}` + }); + } + // If connected, setupBidirectionalForwarding handles cleanup }); } }); diff --git a/ts/forwarding/handlers/https-terminate-to-https-handler.ts b/ts/forwarding/handlers/https-terminate-to-https-handler.ts index 65624a2..1bdbe99 100644 --- a/ts/forwarding/handlers/https-terminate-to-https-handler.ts +++ b/ts/forwarding/handlers/https-terminate-to-https-handler.ts @@ -2,7 +2,7 @@ import * as plugins from '../../plugins.js'; import { ForwardingHandler } from './base-handler.js'; import type { IForwardConfig } from '../config/forwarding-types.js'; import { ForwardingHandlerEvents } from '../config/forwarding-types.js'; -import { createSocketCleanupHandler, setupSocketHandlers, createSocketWithErrorHandler } from '../../core/utils/socket-utils.js'; +import { setupSocketHandlers, createSocketWithErrorHandler, setupBidirectionalForwarding } from '../../core/utils/socket-utils.js'; /** * Handler for HTTPS termination with HTTPS backend @@ -96,17 +96,26 @@ export class HttpsTerminateToHttpsHandler extends ForwardingHandler { // Variable to track backend socket let backendSocket: plugins.tls.TLSSocket | null = null; + let isConnectedToBackend = false; - // Create cleanup handler for both sockets - const handleClose = createSocketCleanupHandler(tlsSocket, backendSocket, (reason) => { - this.emit(ForwardingHandlerEvents.DISCONNECTED, { - remoteAddress, - reason - }); - }); + // Set up initial error handling for TLS socket + const tlsCleanupHandler = (reason: string) => { + if (!isConnectedToBackend) { + // If backend not connected yet, just emit disconnected event + this.emit(ForwardingHandlerEvents.DISCONNECTED, { + remoteAddress, + reason + }); + + // Cleanup TLS socket if needed + if (!tlsSocket.destroyed) { + tlsSocket.destroy(); + } + } + // If connected to backend, setupBidirectionalForwarding will handle cleanup + }; - // Set up error handling with our cleanup utility - setupSocketHandlers(tlsSocket, handleClose, undefined, 'tls'); + setupSocketHandlers(tlsSocket, tlsCleanupHandler, undefined, 'tls'); // Set timeout const timeout = this.getTimeout(); @@ -117,7 +126,7 @@ export class HttpsTerminateToHttpsHandler extends ForwardingHandler { remoteAddress, error: 'TLS connection timeout' }); - handleClose('timeout'); + tlsCleanupHandler('timeout'); }); // Get the target from configuration @@ -131,44 +140,55 @@ export class HttpsTerminateToHttpsHandler extends ForwardingHandler { // In a real implementation, we would configure TLS options rejectUnauthorized: false // For testing only, never use in production }, () => { + isConnectedToBackend = true; + this.emit(ForwardingHandlerEvents.DATA_FORWARDED, { direction: 'outbound', target: `${target.host}:${target.port}`, tls: true }); - // Set up bidirectional data flow - tlsSocket.pipe(backendSocket!); - backendSocket!.pipe(tlsSocket); - }); - - // Update the cleanup handler with the backend socket - const newHandleClose = createSocketCleanupHandler(tlsSocket, backendSocket, (reason) => { - this.emit(ForwardingHandlerEvents.DISCONNECTED, { - remoteAddress, - reason + // Set up bidirectional forwarding with proper cleanup + setupBidirectionalForwarding(tlsSocket, backendSocket!, { + onCleanup: (reason) => { + this.emit(ForwardingHandlerEvents.DISCONNECTED, { + remoteAddress, + reason + }); + }, + enableHalfOpen: false // Close both when one closes + }); + + // Set timeout for backend socket + backendSocket!.setTimeout(timeout); + + backendSocket!.on('timeout', () => { + this.emit(ForwardingHandlerEvents.ERROR, { + remoteAddress, + error: 'Backend connection timeout' + }); + // Let setupBidirectionalForwarding handle the cleanup }); }); - // Set up handlers for backend socket - setupSocketHandlers(backendSocket, newHandleClose, undefined, 'backend'); - + // Handle backend connection errors backendSocket.on('error', (error) => { this.emit(ForwardingHandlerEvents.ERROR, { remoteAddress, error: `Backend connection error: ${error.message}` }); - }); - - // Set timeout for backend socket - backendSocket.setTimeout(timeout); - - backendSocket.on('timeout', () => { - this.emit(ForwardingHandlerEvents.ERROR, { - remoteAddress, - error: 'Backend connection timeout' - }); - newHandleClose('backend_timeout'); + + if (!isConnectedToBackend) { + // Connection failed, clean up TLS socket + if (!tlsSocket.destroyed) { + tlsSocket.destroy(); + } + this.emit(ForwardingHandlerEvents.DISCONNECTED, { + remoteAddress, + reason: `backend_connection_failed: ${error.message}` + }); + } + // If connected, let setupBidirectionalForwarding handle cleanup }); };