fix(smartproxy): Improve port binding intelligence and ACME challenge route management; update route configuration tests and dependency versions.

This commit is contained in:
2025-05-28 19:58:28 +00:00
parent 4ebaf6c061
commit 742adc2bd9
17 changed files with 948 additions and 1258 deletions

View File

@@ -10,7 +10,7 @@ tap.test('should correctly handle HTTP-01 challenge requests with initial data c
const challengePath = `/.well-known/acme-challenge/${challengeToken}`;
// Create a handler function that responds to ACME challenges
const acmeHandler = (context: any) => {
const acmeHandler = async (context: any) => {
// Log request details for debugging
console.log(`Received request: ${context.method} ${context.path}`);
@@ -46,7 +46,7 @@ tap.test('should correctly handle HTTP-01 challenge requests with initial data c
name: 'acme-challenge-route',
match: {
ports: 8080,
paths: ['/.well-known/acme-challenge/*']
path: '/.well-known/acme-challenge/*'
},
action: {
type: 'static',
@@ -99,7 +99,7 @@ tap.test('should correctly handle HTTP-01 challenge requests with initial data c
// Test that non-existent challenge tokens return 404
tap.test('should return 404 for non-existent challenge tokens', async (tapTest) => {
// Create a handler function that behaves like a real ACME handler
const acmeHandler = (context: any) => {
const acmeHandler = async (context: any) => {
if (context.path.startsWith('/.well-known/acme-challenge/')) {
const token = context.path.substring('/.well-known/acme-challenge/'.length);
// In this test, we only recognize one specific token
@@ -126,7 +126,7 @@ tap.test('should return 404 for non-existent challenge tokens', async (tapTest)
name: 'acme-challenge-route',
match: {
ports: 8081,
paths: ['/.well-known/acme-challenge/*']
path: '/.well-known/acme-challenge/*'
},
action: {
type: 'static',

View File

@@ -37,6 +37,18 @@ tap.test('should defer certificate provisioning until ports are ready', async (t
console.log('Creating mock cert manager');
operationOrder.push('create-cert-manager');
const mockCertManager = {
certStore: null,
smartAcme: null,
httpProxy: null,
renewalTimer: null,
pendingChallenges: new Map(),
challengeRoute: null,
certStatus: new Map(),
globalAcmeDefaults: null,
updateRoutesCallback: undefined,
challengeRouteActive: false,
isProvisioning: false,
acmeStateManager: null,
initialize: async () => {
operationOrder.push('cert-manager-init');
console.log('Mock cert manager initialized');
@@ -56,8 +68,15 @@ tap.test('should defer certificate provisioning until ports are ready', async (t
setAcmeStateManager: () => {},
setUpdateRoutesCallback: () => {},
getAcmeOptions: () => ({}),
getState: () => ({ challengeRouteActive: false })
};
getState: () => ({ challengeRouteActive: false }),
getCertStatus: () => new Map(),
checkAndRenewCertificates: async () => {},
addChallengeRoute: async () => {},
removeChallengeRoute: async () => {},
getCertificate: async () => null,
isValidCertificate: () => false,
waitForProvisioning: async () => {}
} as any;
// Call initialize immediately as the real createCertificateManager does
await mockCertManager.initialize();

View File

@@ -1,4 +1,4 @@
import { expect, tap } from '@git.zone/tapbundle';
import { expect, tap } from '@git.zone/tstest/tapbundle';
import * as net from 'net';
import * as tls from 'tls';
import * as fs from 'fs';
@@ -61,7 +61,7 @@ tap.test('should forward TCP connections correctly', async () => {
id: 'tcp-forward',
name: 'TCP Forward Route',
match: {
port: 8080,
ports: 8080,
},
action: {
type: 'forward',
@@ -110,8 +110,8 @@ tap.test('should handle TLS passthrough correctly', async () => {
id: 'tls-passthrough',
name: 'TLS Passthrough Route',
match: {
port: 8443,
domain: 'test.example.com',
ports: 8443,
domains: 'test.example.com',
},
action: {
type: 'forward',
@@ -171,8 +171,8 @@ tap.test('should handle SNI-based forwarding', async () => {
id: 'domain-a',
name: 'Domain A Route',
match: {
port: 8443,
domain: 'a.example.com',
ports: 8443,
domains: 'a.example.com',
},
action: {
type: 'forward',
@@ -189,8 +189,8 @@ tap.test('should handle SNI-based forwarding', async () => {
id: 'domain-b',
name: 'Domain B Route',
match: {
port: 8443,
domain: 'b.example.com',
ports: 8443,
domains: 'b.example.com',
},
action: {
type: 'forward',

View File

@@ -112,7 +112,7 @@ tap.test('NFTables forward route should not terminate connections', async () =>
// Wait a bit to ensure connection isn't immediately closed
await new Promise(resolve => setTimeout(resolve, 1000));
expect(connectionClosed).toBe(false);
expect(connectionClosed).toEqual(false);
console.log('NFTables connection stayed open as expected');
client.end();

View File

@@ -1,4 +1,4 @@
import { expect, tap } from '@git.zone/tapbundle';
import { expect, tap } from '@git.zone/tstest/tapbundle';
import * as net from 'net';
import { SmartProxy } from '../ts/proxies/smart-proxy/smart-proxy.js';
@@ -35,7 +35,7 @@ tap.test('forward connections should not be immediately closed', async (t) => {
id: 'forward-test',
name: 'Forward Test Route',
match: {
port: 8080,
ports: 8080,
},
action: {
type: 'forward',
@@ -80,9 +80,15 @@ tap.test('forward connections should not be immediately closed', async (t) => {
});
// Wait for the welcome message
await t.waitForExpect(() => {
return dataReceived;
}, 'Data should be received from the server', 2000);
let waitTime = 0;
while (!dataReceived && waitTime < 2000) {
await new Promise(resolve => setTimeout(resolve, 100));
waitTime += 100;
}
if (!dataReceived) {
throw new Error('Data should be received from the server');
}
// Verify we got the welcome message
expect(welcomeMessage).toContain('Welcome from test server');
@@ -94,7 +100,7 @@ tap.test('forward connections should not be immediately closed', async (t) => {
await new Promise(resolve => setTimeout(resolve, 100));
// Connection should still be open
expect(connectionClosed).toBe(false);
expect(connectionClosed).toEqual(false);
// Clean up
client.end();

View File

@@ -43,7 +43,7 @@ tap.test('should forward non-TLS connections on HttpProxy ports', async (tapTest
// Test the logic from handleForwardAction
const route = mockSettings.routes[0];
const action = route.action;
const action = route.action as any;
// Simulate the fixed logic
if (!action.tls) {
@@ -101,7 +101,7 @@ tap.test('should use direct connection for non-HttpProxy ports', async (tapTest)
};
const route = mockSettings.routes[0];
const action = route.action;
const action = route.action as any;
// Test the logic
if (!action.tls) {
@@ -162,7 +162,7 @@ tap.test('should handle ACME HTTP-01 challenges on port 80 with HttpProxy', asyn
};
const route = mockSettings.routes[0];
const action = route.action;
const action = route.action as any;
// Test the fix for ACME HTTP-01 challenges
if (!action.tls) {

View File

@@ -1,6 +1,6 @@
import { tap, expect } from '@git.zone/tstest/tapbundle';
import { RouteConnectionHandler } from '../ts/proxies/smart-proxy/route-connection-handler.js';
import { ISmartProxyOptions } from '../ts/proxies/smart-proxy/models/interfaces.js';
import type { ISmartProxyOptions } from '../ts/proxies/smart-proxy/models/interfaces.js';
import * as net from 'net';
// Direct test of the fix in RouteConnectionHandler
@@ -68,9 +68,9 @@ tap.test('should detect and forward non-TLS connections on useHttpProxy ports',
};
// Test: Create a mock socket representing non-TLS connection on port 8080
const mockSocket = new net.Socket();
mockSocket.localPort = 8080;
mockSocket.remoteAddress = '127.0.0.1';
const mockSocket = Object.create(net.Socket.prototype) as net.Socket;
Object.defineProperty(mockSocket, 'localPort', { value: 8080, writable: false });
Object.defineProperty(mockSocket, 'remoteAddress', { value: '127.0.0.1', writable: false });
// Simulate the handler processing the connection
handler.handleConnection(mockSocket);
@@ -147,9 +147,9 @@ tap.test('should handle TLS connections normally', async (tapTest) => {
mockRouteManager as any
);
const mockSocket = new net.Socket();
mockSocket.localPort = 443;
mockSocket.remoteAddress = '127.0.0.1';
const mockSocket = Object.create(net.Socket.prototype) as net.Socket;
Object.defineProperty(mockSocket, 'localPort', { value: 443, writable: false });
Object.defineProperty(mockSocket, 'remoteAddress', { value: '127.0.0.1', writable: false });
handler.handleConnection(mockSocket);

View File

@@ -8,9 +8,23 @@ tap.test('should detect and forward non-TLS connections on HttpProxy ports', asy
let forwardedToHttpProxy = false;
let connectionPath = '';
// Mock the HttpProxy forwarding
const originalForward = SmartProxy.prototype['httpProxyBridge'].prototype.forwardToHttpProxy;
SmartProxy.prototype['httpProxyBridge'].prototype.forwardToHttpProxy = function(...args: any[]) {
// Create a SmartProxy instance first
const proxy = new SmartProxy({
useHttpProxy: [8080],
httpProxyPort: 8844,
routes: [{
name: 'test-http-forward',
match: { ports: 8080 },
action: {
type: 'forward',
target: { host: 'localhost', port: 8181 }
}
}]
});
// Mock the HttpProxy forwarding on the instance
const originalForward = (proxy as any).httpProxyBridge.forwardToHttpProxy;
(proxy as any).httpProxyBridge.forwardToHttpProxy = async function(...args: any[]) {
forwardedToHttpProxy = true;
connectionPath = 'httpproxy';
console.log('Mock: Connection forwarded to HttpProxy');
@@ -18,22 +32,8 @@ tap.test('should detect and forward non-TLS connections on HttpProxy ports', asy
args[1].end(); // socket.end()
};
// Create a SmartProxy with useHttpProxy configured
const proxy = new SmartProxy({
useHttpProxy: [8080],
httpProxyPort: 8844,
enableDetailedLogging: true,
routes: [{
name: 'test-route',
match: {
ports: 8080
},
action: {
type: 'forward',
target: { host: 'localhost', port: 8181 }
}
}]
});
// Add detailed logging to the existing proxy instance
proxy.settings.enableDetailedLogging = true;
// Override the HttpProxy initialization to avoid actual HttpProxy setup
proxy['httpProxyBridge'].getHttpProxy = () => ({} as any);
@@ -65,7 +65,8 @@ tap.test('should detect and forward non-TLS connections on HttpProxy ports', asy
await proxy.stop();
// Restore original method
SmartProxy.prototype['httpProxyBridge'].prototype.forwardToHttpProxy = originalForward;
// Restore original method
(proxy as any).httpProxyBridge.forwardToHttpProxy = originalForward;
});
// Test that verifies the fix detects non-TLS connections

View File

@@ -51,7 +51,7 @@ tap.test('should handle ACME challenges on port 8080 with improved port binding
const tempCertDir = './temp-certs';
try {
await plugins.smartfile.SmartFile.createDirectory(tempCertDir);
await plugins.smartfile.fs.ensureDir(tempCertDir);
} catch (error) {
// Directory may already exist, that's ok
}
@@ -156,8 +156,10 @@ tap.test('should handle ACME challenges on port 8080 with improved port binding
console.log('Port binding attempts:', portBindAttempts);
// Check that we tried to bind to port 9009
expect(portBindAttempts.includes(9009)).toEqual(true, 'Should attempt to bind to port 9009');
expect(portBindAttempts.includes(9003)).toEqual(true, 'Should attempt to bind to port 9003');
// Should attempt to bind to port 9009
expect(portBindAttempts.includes(9009)).toEqual(true);
// Should attempt to bind to port 9003
expect(portBindAttempts.includes(9003)).toEqual(true);
// Get actual bound ports
const boundPorts = proxy.getListeningPorts();
@@ -165,10 +167,12 @@ tap.test('should handle ACME challenges on port 8080 with improved port binding
// If port 9009 was available, we should be bound to it
if (acmePortAvailable) {
expect(boundPorts.includes(9009)).toEqual(true, 'Should be bound to port 9009 if available');
// Should be bound to port 9009 if available
expect(boundPorts.includes(9009)).toEqual(true);
}
expect(boundPorts.includes(9003)).toEqual(true, 'Should be bound to port 9003');
// Should be bound to port 9003
expect(boundPorts.includes(9003)).toEqual(true);
// Test adding a new route on port 8080
console.log('Testing route update with port reuse...');
@@ -186,7 +190,7 @@ tap.test('should handle ACME challenges on port 8080 with improved port binding
path: '/additional'
},
action: {
type: 'forward',
type: 'forward' as const,
target: { host: 'localhost', port: targetPort }
}
}
@@ -198,16 +202,19 @@ tap.test('should handle ACME challenges on port 8080 with improved port binding
console.log('Port binding attempts after update:', portBindAttempts);
// We should not try to rebind port 9009 since it's already bound
expect(portBindAttempts.includes(9009)).toEqual(false, 'Should not attempt to rebind port 9009');
// Should not attempt to rebind port 9009
expect(portBindAttempts.includes(9009)).toEqual(false);
// We should still be listening on both ports
const portsAfterUpdate = proxy.getListeningPorts();
console.log('Bound ports after update:', portsAfterUpdate);
if (acmePortAvailable) {
expect(portsAfterUpdate.includes(9009)).toEqual(true, 'Should still be bound to port 9009');
// Should still be bound to port 9009
expect(portsAfterUpdate.includes(9009)).toEqual(true);
}
expect(portsAfterUpdate.includes(9003)).toEqual(true, 'Should still be bound to port 9003');
// Should still be bound to port 9003
expect(portsAfterUpdate.includes(9003)).toEqual(true);
// The test is successful at this point - we've verified the port binding intelligence
console.log('Port binding intelligence verified successfully!');
@@ -227,16 +234,8 @@ tap.test('should handle ACME challenges on port 8080 with improved port binding
// Clean up temp directory
try {
// Try different removal methods
if (typeof plugins.smartfile.fs.removeManySync === 'function') {
plugins.smartfile.fs.removeManySync([tempCertDir]);
} else if (typeof plugins.smartfile.fs.removeDirectory === 'function') {
await plugins.smartfile.fs.removeDirectory(tempCertDir);
} else if (typeof plugins.smartfile.removeDirectory === 'function') {
await plugins.smartfile.removeDirectory(tempCertDir);
} else {
console.log('Unable to find appropriate directory removal method');
}
// Remove temp directory
await plugins.smartfile.fs.remove(tempCertDir);
} catch (error) {
console.error('Failed to remove temp directory:', error);
}

View File

@@ -29,7 +29,7 @@ tap.test('NFTables forwarding should not terminate connections', async () => {
id: 'nftables-test',
name: 'NFTables Test Route',
match: {
port: 8080,
ports: 8080,
},
action: {
type: 'forward',
@@ -45,7 +45,7 @@ tap.test('NFTables forwarding should not terminate connections', async () => {
id: 'regular-test',
name: 'Regular Forward Route',
match: {
port: 8081,
ports: 8081,
},
action: {
type: 'forward',
@@ -83,7 +83,7 @@ tap.test('NFTables forwarding should not terminate connections', async () => {
// Check connection after 100ms
setTimeout(() => {
// Connection should still be alive even if app doesn't handle it
expect(nftablesConnection.destroyed).toBe(false);
expect(nftablesConnection.destroyed).toEqual(false);
nftablesConnection.end();
resolve();
}, 100);

View File

@@ -45,9 +45,9 @@ tap.test('should set update routes callback on certificate manager', async () =>
setUpdateRoutesCallback: function(callback: any) {
callbackSet = true;
},
setHttpProxy: function() {},
setGlobalAcmeDefaults: function() {},
setAcmeStateManager: function() {},
setHttpProxy: function(proxy: any) {},
setGlobalAcmeDefaults: function(defaults: any) {},
setAcmeStateManager: function(manager: any) {},
initialize: async function() {},
provisionAllCertificates: async function() {},
stop: async function() {},

View File

@@ -55,7 +55,8 @@ tap.test('should have error handling around logger calls in route update callbac
expect(true).toEqual(true);
} catch (error) {
// This shouldn't happen if our error handling is working
expect(false).toEqual(true, 'Error handling failed in addChallengeRoute');
// Error handling failed in addChallengeRoute
expect(false).toEqual(true);
}
// Verify that we handle errors in removeChallengeRoute
@@ -69,7 +70,8 @@ tap.test('should have error handling around logger calls in route update callbac
expect(true).toEqual(true);
} catch (error) {
// This shouldn't happen if our error handling is working
expect(false).toEqual(true, 'Error handling failed in removeChallengeRoute');
// Error handling failed in removeChallengeRoute
expect(false).toEqual(true);
}
});