Skip to content

Commit

Permalink
chore: correct port validation (#7077)
Browse files Browse the repository at this point in the history 
* chore: correct port validation

Signed-off-by: Vladyslav Zhukovskyi <[email protected]>

* chore: correct port validation

Signed-off-by: Vladyslav Zhukovskyi <[email protected]>

---------

Signed-off-by: Vladyslav Zhukovskyi <[email protected]>
  • Loading branch information
@vzhukovs
vzhukovs committed May 14, 2024
1 parent ba70ba9 commit d1b138b
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 150 deletions.
14 changes: 8 additions & 6 deletions packages/main/src/plugin/util/port.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,14 +133,16 @@ test.each(hosts)(
},
);

test('fails with range error if trying to get a port which is over upper range', async () => {
await expect(port.getFreePort(200000)).rejects.toThrowError(/options.port should be >= 0 and < 65536/);
});

test('fails with range error if value is over upper range', async () => {
await expect(port.isFreePort(200000)).rejects.toThrowError(/options.port should be >= 0 and < 65536/);
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 () => {
await expect(port.isFreePort(-1)).rejects.toThrowError(/options.port should be >= 0 and < 65536/);
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 () => {
await expect(port.isFreePort(1)).rejects.toThrowError(/The port must be greater than 1024./);
});
38 changes: 27 additions & 11 deletions packages/main/src/plugin/util/port.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,10 @@ export async function getFreePort(port = 0): Promise<number> {
}
let isFree = false;
while (!isFree) {
isFree = await isFreePort(port);
if (!isFree) {
try {
await isFreePort(port);
isFree = true;
} catch (e) {
port++;
}
}
Expand All @@ -45,9 +47,10 @@ export async function getFreePortRange(rangeSize: number): Promise<string> {
let startPort = port;

do {
if (await isFreePort(port)) {
try {
await isFreePort(port);
++port;
} else {
} catch (e) {
++port;
startPort = port;
}
Expand All @@ -60,20 +63,33 @@ function isFreeAddressPort(address: string, port: number): Promise<boolean> {
const server = net.createServer();
return new Promise((resolve, reject) =>
server
.on('error', (error: NodeJS.ErrnoException) => (error.code === 'EADDRINUSE' ? resolve(false) : reject(error)))
.on('error', (error: NodeJS.ErrnoException) => {
if (error.code === 'EADDRINUSE') {
reject(new Error(`Port ${port} is already in use.`));
} else if (error.code === 'EACCES') {
reject(new Error('Operation require administrative privileges.'));
} else {
reject(new Error(`Failed to check port status: ${error}`));
}
})
.on('listening', () => server.close(() => resolve(true)))
.listen(port, address),
);
}

export async function isFreePort(port: number): Promise<boolean> {
const intfs = getIPv4Interfaces();
// Test this special interface separately, or it will interfere with other tests done in parallel
if (!(await isFreeAddressPort('0.0.0.0', port))) {
return false;
if (isNaN(port) || port > 65535) {
throw new Error('The port must have an integer value within the range from 1025 to 65535.');
} else if (port < 1024) {
throw new Error('The port must be greater than 1024.');
}
const checkInterfaces = await Promise.all(intfs.map(intf => isFreeAddressPort(intf, port)));
return checkInterfaces.every(element => element === true);

const intfs = getIPv4Interfaces();

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

return true;
}

function getIPv4Interfaces(): string[] {
Expand Down
116 changes: 6 additions & 110 deletions packages/renderer/src/lib/image/RunImage.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,7 @@ describe('RunImage', () => {
});

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

await createRunImage(undefined, ['command1', 'command2']);
Expand All @@ -352,6 +353,9 @@ describe('RunImage', () => {
await userEvent.clear(containerInput);
await userEvent.keyboard('9000-9003');

// wait onPortInputTimeout (500ms) triggers
await new Promise(resolve => setTimeout(resolve, 600));

const button = screen.getByRole('button', { name: 'Start Container' });

await fireEvent.click(button);
Expand Down Expand Up @@ -464,86 +468,8 @@ describe('RunImage', () => {
);
});

test('Expect "start container" button to be disabled if user adds a port which is invalid (lower than 0)', async () => {
router.goto('/basic');

await createRunImage(undefined, ['command1', 'command2']);

const link1 = screen.getByRole('link', { name: 'Basic' });
await fireEvent.click(link1);

const customMappingButton = screen.getByRole('button', { name: 'Add custom port mapping' });
await fireEvent.click(customMappingButton);

const hostInput = screen.getByLabelText('host port');
await userEvent.click(hostInput);
await userEvent.clear(hostInput);
// adds a negative port
await userEvent.keyboard('-1');

const containerInput = screen.getByLabelText('container port');
await userEvent.click(containerInput);
await userEvent.clear(containerInput);
await userEvent.keyboard('80');

const button = screen.getByRole('button', { name: 'Start Container' });
expect((button as HTMLButtonElement).disabled).toBeTruthy();
});

test('Expect "start container" button to be disabled if user adds a port which is invalid (over upper limit > 65535)', async () => {
router.goto('/basic');

await createRunImage(undefined, ['command1', 'command2']);

const link1 = screen.getByRole('link', { name: 'Basic' });
await fireEvent.click(link1);

const customMappingButton = screen.getByRole('button', { name: 'Add custom port mapping' });
await fireEvent.click(customMappingButton);

const hostInput = screen.getByLabelText('host port');
await userEvent.click(hostInput);
await userEvent.clear(hostInput);
// adds a negative port
await userEvent.keyboard('71000');

const containerInput = screen.getByLabelText('container port');
await userEvent.click(containerInput);
await userEvent.clear(containerInput);
await userEvent.keyboard('80');

const button = screen.getByRole('button', { name: 'Start Container' });
expect((button as HTMLButtonElement).disabled).toBeTruthy();
});

test('Expect "start container" button to be disabled if user adds a port which is invalid (isNaN)', async () => {
router.goto('/basic');

await createRunImage(undefined, ['command1', 'command2']);

const link1 = screen.getByRole('link', { name: 'Basic' });
await fireEvent.click(link1);

const customMappingButton = screen.getByRole('button', { name: 'Add custom port mapping' });
await fireEvent.click(customMappingButton);

const hostInput = screen.getByLabelText('host port');
await userEvent.click(hostInput);
await userEvent.clear(hostInput);
// adds a negative port
await userEvent.keyboard('test');

const containerInput = screen.getByLabelText('container port');
await userEvent.click(containerInput);
await userEvent.clear(containerInput);
await userEvent.keyboard('80');

const button = screen.getByRole('button', { name: 'Start Container' });
expect((button as HTMLButtonElement).disabled).toBeTruthy();
});

test('Expect "start container" button to be disabled if user adds a port which is NOT free', async () => {
(window.isFreePort as Mock).mockResolvedValue(false);
test('Expect "start container" button to be disabled when port is not free', async () => {
(window.isFreePort as Mock).mockRejectedValue(new Error('Error Message'));
router.goto('/basic');

await createRunImage(undefined, ['command1', 'command2']);
Expand Down Expand Up @@ -571,34 +497,4 @@ describe('RunImage', () => {
const button = screen.getByRole('button', { name: 'Start Container' });
expect((button as HTMLButtonElement).disabled).toBeTruthy();
});

test('Expect "start container" button to be enabled if user adds a port which is valid and free', async () => {
(window.isFreePort as Mock).mockResolvedValue(true);
router.goto('/basic');

await createRunImage(undefined, ['command1', 'command2']);

const link1 = screen.getByRole('link', { name: 'Basic' });
await fireEvent.click(link1);

const customMappingButton = screen.getByRole('button', { name: 'Add custom port mapping' });
await fireEvent.click(customMappingButton);

const hostInput = screen.getByLabelText('host port');
await userEvent.click(hostInput);
await userEvent.clear(hostInput);
// adds a negative port
await userEvent.keyboard('8080');

const containerInput = screen.getByLabelText('container port');
await userEvent.click(containerInput);
await userEvent.clear(containerInput);
await userEvent.keyboard('80');

// wait onPortInputTimeout (500ms) triggers
await new Promise(resolve => setTimeout(resolve, 600));

const button = screen.getByRole('button', { name: 'Start Container' });
expect((button as HTMLButtonElement).disabled).toBeFalsy();
});
});
35 changes: 12 additions & 23 deletions packages/renderer/src/lib/image/RunImage.svelte
View file
Original file line number Diff line number Diff line change
Expand Up @@ -598,33 +598,22 @@ function onPortInput(event: Event, portInfo: PortInfo, updateUI: () => void) {
const target = event.currentTarget as HTMLInputElement;
// convert string to number
const _value: number = Number(target.value);
// if number is not valid (NaN or outside the value range), set the error
if (isNaN(_value) || _value < 0 || _value > 65535) {
portInfo.error = 'port should be >= 0 and < 65536';
updateUI();
return;
}
onPortInputTimeout = setTimeout(() => {
isPortFree(_value).then(isFree => {
portInfo.error = isFree;
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);
}
function isPortFree(port: number): Promise<string> {
return window
.isFreePort(port)
.then(isFree => {
if (!isFree) {
return `Port ${port} is already in use`;
} else {
return '';
}
})
.catch((_: unknown) => `Port ${port} is already in use`);
}
async function assertAllPortAreValid(): Promise<void> {
const invalidHostPorts = hostContainerPortMappings.filter(pair => pair.hostPort.error);
const invalidContainerPortMapping = containerPortMapping?.filter(port => port.error) || [];
Expand Down

0 comments on commit d1b138b

Please sign in to comment.