Skip to content

Commit

Permalink
chore: correct port validation
Browse files Browse the repository at this point in the history
Signed-off-by: Vladyslav Zhukovskyi <[email protected]>
  • Loading branch information
vzhukovs committed May 13, 2024
1 parent f71e1ba commit 7c5590e
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 78 deletions.
4 changes: 2 additions & 2 deletions packages/main/src/plugin/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ import { Troubleshooting } from './troubleshooting.js';
import type { IDisposable } from './types/disposable.js';
import type { Deferred } from './util/deferred.js';
import { Exec } from './util/exec.js';
import { getFreePort, getFreePortRange, isFreePort, type PortStatus } from './util/port.js';
import { getFreePort, getFreePortRange, isFreePort } from './util/port.js';
import { ViewRegistry } from './view-registry.js';
import { WebviewRegistry } from './webview/webview-registry.js';
import { WelcomeInit } from './welcome/welcome-init.js';
Expand Down Expand Up @@ -1325,7 +1325,7 @@ export class PluginSystem {
return getFreePortRange(rangeSize);
});

this.ipcHandle('system:is-port-free', async (_, port: number): Promise<PortStatus> => {
this.ipcHandle('system:is-port-free', async (_, port: number): Promise<boolean> => {
return isFreePort(port);
});

Expand Down
31 changes: 12 additions & 19 deletions packages/main/src/plugin/util/port.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ test('return valid port range', async () => {
expect(isNaN(endRange)).toBe(false);

expect(endRange + 1 - startRange).toBe(3);
expect(await port.isFreePort(startRange)).toStrictEqual({ available: true });
expect(await port.isFreePort(endRange)).toStrictEqual({ available: true });
expect(await port.isFreePort(startRange)).toBe(true);
expect(await port.isFreePort(endRange)).toBe(true);
});

test.each(hosts)(
Expand Down Expand Up @@ -75,8 +75,8 @@ test.each(hosts)(

expect(startNewRange > endRange).toBe(true);
expect(endNewRange + 1 - startNewRange).toBe(3);
expect(await port.isFreePort(startNewRange)).toStrictEqual({ available: true });
expect(await port.isFreePort(endNewRange)).toStrictEqual({ available: true });
expect(await port.isFreePort(startNewRange)).toBe(true);
expect(await port.isFreePort(endNewRange)).toBe(true);
},
);

Expand All @@ -85,7 +85,7 @@ test('return first empty port, no port is used', async () => {
const freePort = await port.getFreePort(start);

expect(freePort).toBe(start);
expect(await port.isFreePort(freePort)).toStrictEqual({ available: true });
expect(await port.isFreePort(freePort)).toBe(true);
});

test.each(hosts)(
Expand All @@ -104,7 +104,7 @@ test.each(hosts)(
server.close();

expect(freePort).toBe(port20001);
expect(await port.isFreePort(freePort)).toStrictEqual({ available: true });
expect(await port.isFreePort(freePort)).toBe(true);
},
);

Expand All @@ -129,27 +129,20 @@ test.each(hosts)(
server2.close();

expect(freePort).toBe(port20002);
expect(await port.isFreePort(freePort)).toStrictEqual({ available: true });
expect(await port.isFreePort(freePort)).toBe(true);
},
);

test('fails with range error if value is over upper range', async () => {
expect(await port.isFreePort(200000)).toStrictEqual({
available: false,
message: 'The port must have an integer value within the range from 1025 to 65535.',
});
await expect(port.isFreePort(200000)).rejects.toThrowError(
/The port must have an integer value within the range from 1025 to 65535./,
);
});

test('fails with range error if value is less lower range', async () => {
expect(await port.isFreePort(-1)).toStrictEqual({
available: false,
message: 'The port must be greater than 1024.',
});
await expect(port.isFreePort(-1)).rejects.toThrowError(/The port must be greater than 1024./);
});

test('should return message that user is trying to check unprivileged port', async () => {
expect(await port.isFreePort(1)).toStrictEqual({
available: false,
message: 'The port must be greater than 1024.',
});
await expect(port.isFreePort(1)).rejects.toThrowError(/The port must be greater than 1024./);
});
70 changes: 21 additions & 49 deletions packages/main/src/plugin/util/port.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,6 @@
import * as net from 'node:net';
import { type NetworkInterfaceInfoIPv4, networkInterfaces } from 'node:os';

export interface PortStatus {
available: boolean;
message?: string;
}

/**
* Find a free port starting from the given port
*/
Expand All @@ -33,8 +28,10 @@ export async function getFreePort(port = 0): Promise<number> {
}
let isFree = false;
while (!isFree) {
isFree = (await isFreePort(port)).available;
if (!isFree) {
try {
await isFreePort(port);
isFree = true;
} catch (e) {
port++;
}
}
Expand All @@ -50,10 +47,10 @@ export async function getFreePortRange(rangeSize: number): Promise<string> {
let startPort = port;

do {
const portStatus = await isFreePort(port);
if (portStatus.available) {
try {
await isFreePort(port);
++port;
} else {
} catch (e) {
++port;
startPort = port;
}
Expand All @@ -62,62 +59,37 @@ export async function getFreePortRange(rangeSize: number): Promise<string> {
return `${startPort}-${port - 1}`;
}

function isFreeAddressPort(address: string, port: number): Promise<PortStatus> {
function isFreeAddressPort(address: string, port: number): Promise<boolean> {
const server = net.createServer();
return new Promise(resolve =>
return new Promise((resolve, reject) =>
server
.on('error', (error: NodeJS.ErrnoException) => {
if (error.code === 'EADDRINUSE') {
resolve({
available: false,
message: `Port ${port} is already in use.`,
});
reject(new Error(`Port ${port} is already in use.`));
} else if (error.code === 'EACCES') {
resolve({
available: false,
message: 'Operation require administrative privileges.',
});
reject(new Error('Operation require administrative privileges.'));
} else {
resolve({
available: false,
message: `Failed to check port status: ${error}`,
});
reject(new Error(`Failed to check port status: ${error}`));
}
})
.on('listening', () => server.close(() => resolve({ available: true })))
.on('listening', () => server.close(() => resolve(true)))
.listen(port, address),
);
}

export async function isFreePort(port: number): Promise<PortStatus> {
export async function isFreePort(port: number): Promise<boolean> {
if (isNaN(port) || port > 65535) {
return {
available: false,
message: 'The port must have an integer value within the range from 1025 to 65535.',
};
throw new Error('The port must have an integer value within the range from 1025 to 65535.');
} else if (port < 1024) {
return {
available: false,
message: 'The port must be greater than 1024.',
};
throw new Error('The port must be greater than 1024.');
}

const intfs = getIPv4Interfaces();
// Test this special interface separately, or it will interfere with other tests done in parallel
const portCheckStatus = await isFreeAddressPort('0.0.0.0', port);
if (!portCheckStatus.available) {
return portCheckStatus;
}
const checkInterfaces = await Promise.all(intfs.map(intf => isFreeAddressPort(intf, port)));
const combinedPortChecks = checkInterfaces.filter(check => !check.available);
if (combinedPortChecks && combinedPortChecks.length > 0) {
// Means, that some check was not successful and port is not available
return {
available: false,
message: combinedPortChecks.map(check => check.message).join('\n'),
};
}
return { available: true };

await isFreeAddressPort('0.0.0.0', port);
await Promise.all(intfs.map(intf => isFreeAddressPort(intf, port)));

return true;
}

function getIPv4Interfaces(): string[] {
Expand Down
3 changes: 1 addition & 2 deletions packages/preload/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ import type { MessageBoxOptions, MessageBoxReturnValue } from '../../main/src/pl
import type { ExtensionBanner, RecommendedRegistry } from '../../main/src/plugin/recommendations/recommendations-api';
import type { StatusBarEntryDescriptor } from '../../main/src/plugin/statusbar/statusbar-registry';
import type { IDisposable } from '../../main/src/plugin/types/disposable';
import type { PortStatus } from '../../main/src/plugin/util/port';
import { Deferred } from './util/deferred';

export type DialogResultCallback = (openDialogReturnValue: Electron.OpenDialogReturnValue) => void;
Expand Down Expand Up @@ -1417,7 +1416,7 @@ export function initExposure(): void {
return ipcInvoke('system:get-free-port-range', rangeSize);
});

contextBridge.exposeInMainWorld('isFreePort', async (port: number): Promise<PortStatus> => {
contextBridge.exposeInMainWorld('isFreePort', async (port: number): Promise<boolean> => {
return ipcInvoke('system:is-port-free', port);
});

Expand Down
4 changes: 2 additions & 2 deletions packages/renderer/src/lib/image/RunImage.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ describe('RunImage', () => {
});

test('Expect to see an error if the container/host ranges have different size', async () => {
(window.isFreePort as Mock).mockResolvedValue({ available: true });
(window.isFreePort as Mock).mockResolvedValue(true);
router.goto('/basic');

await createRunImage(undefined, ['command1', 'command2']);
Expand Down Expand Up @@ -469,7 +469,7 @@ describe('RunImage', () => {
});

test('Expect "start container" button to be disabled when port is not free', async () => {
(window.isFreePort as Mock).mockResolvedValue({ available: false, message: 'Error message' });
(window.isFreePort as Mock).mockRejectedValue(new Error('Error Message'));
router.goto('/basic');

await createRunImage(undefined, ['command1', 'command2']);
Expand Down
16 changes: 12 additions & 4 deletions packages/renderer/src/lib/image/RunImage.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -599,10 +599,18 @@ function onPortInput(event: Event, portInfo: PortInfo, updateUI: () => void) {
// convert string to number
const _value: number = Number(target.value);
onPortInputTimeout = setTimeout(() => {
window.isFreePort(_value).then(portStatus => {
portInfo.error = !portStatus.available ? portStatus.message! : '';
updateUI();
});
window
.isFreePort(_value)
.then(_ => {
portInfo.error = '';
updateUI();
})
.catch((error: unknown) => {
if (error && typeof error === 'object' && 'message' in error) {
portInfo.error = (error as { message: string }).message;
}
updateUI();
});
}, 500);
}
Expand Down

0 comments on commit 7c5590e

Please sign in to comment.