Skip to content

Feat: Add port checking for VNC and sane defaults #2452

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 25, 2025
Merged

Feat: Add port checking for VNC and sane defaults #2452

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions emhttp/plugins/dynamix.vm.manager/templates/Custom.form.php
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@
'autoport' => 'yes',
'model' => 'qxl',
'keymap' => 'none',
'port' => -1 ,
'wsport' => -1,
'port' => 5900,
'wsport' => 5901,
'copypaste' => 'no',
'render' => 'auto',
'DisplayOptions' => ""
Expand Down Expand Up @@ -1325,9 +1325,9 @@
?>
</select></span>
<span id="Porttext" class="label <?=$hiddenport?>">_(VM Console Port)_:</span>
<input id="port" type="number" size="5" maxlength="5" class="trim second <?=$hiddenport?>" name="gpu[<?=$i?>][port]" value="<?=$arrGPU['port']?>">
<input id="port" onchange="checkVNCPorts()" min="5900" max="65535" type="number" size="5" maxlength="5" class="trim second <?=$hiddenport?>" name="gpu[<?=$i?>][port]" value="<?=$arrGPU['port']?>">
<span id="WSPorttext" class="label <?=$hiddenwsport?>">_(VM Console WS Port)_:</span>
<input id="wsport" type="number" size="5" maxlength="5" class="trim second <?=$hiddenwsport?>" name="gpu[<?=$i?>][wsport]" value="<?=$arrGPU['wsport']?>">
<input id="wsport" onchange="checkVNCPorts()" min="5900" max="65535" type="number" size="5" maxlength="5" class="trim second <?=$hiddenwsport?>" name="gpu[<?=$i?>][wsport]" value="<?=$arrGPU['wsport']?>">
</td>
<td></td>
</tr>
Expand Down Expand Up @@ -2088,6 +2088,18 @@
var storageType = "<?=get_storage_fstype($arrConfig['template']['storage']);?>";
var storageLoc = "<?=$arrConfig['template']['storage']?>";

function checkVNCPorts() {
const port = $("#port").val();
const wsport = $("#wsport").val();
if (port < 5900 || port > 65535 || wsport < 5900 || wsport > 65535 || port == wsport) {
swal({
title: "_(Invalid Port)_",
text: "_(VNC/SPICE ports must be between 5900 and 65535, and cannot be equal to each other)_",
type: "error",
confirmButtonText: "_(Ok)_"
});
}
}
Comment on lines +2091 to +2102
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix type coercion bug in numeric comparisons.

The function compares port values using < and > operators on strings returned by .val(), which can produce incorrect results. Additionally, the validation doesn't prevent form submission with invalid values.

Apply this diff to fix the type coercion and add form submission prevention:

 function checkVNCPorts() {
-	const port = $("#port").val();
-	const wsport = $("#wsport").val();
-	if (port < 5900 || port > 65535 || wsport < 5900 || wsport > 65535 || port == wsport) {
+	const port = parseInt($("#port").val(), 10);
+	const wsport = parseInt($("#wsport").val(), 10);
+	
+	// Only validate if fields are visible (autoport is "no")
+	if ($("#port").hasClass("hidden")) {
+		return true;
+	}
+	
+	if (isNaN(port) || isNaN(wsport) || port < 5900 || port > 65535 || wsport < 5900 || wsport > 65535 || port === wsport) {
 		swal({
 			title: "_(Invalid Port)_",
 			text: "_(VNC/SPICE ports must be between 5900 and 65535, and cannot be equal to each other)_",
 			type: "error",
 			confirmButtonText: "_(Ok)_"
 		});
+		return false;
 	}
+	return true;
 }

Additionally, consider validating before form submission by adding a check in the submit handlers (around lines 2662 and 2809):

// In the form submit handlers, before posting:
if (!checkVNCPorts()) {
	$panel.find('input').prop('disabled', false);
	$button.val($button.attr('readyvalue'));
	return;
}
🤖 Prompt for AI Agents 
In emhttp/plugins/dynamix.vm.manager/templates/Custom.form.php around lines 2091
to 2102, checkVNCPorts currently compares string values from .val(), causing
type coercion bugs and it doesn't prevent submission; update the function to
parse port and wsport to integers (use parseInt with base 10), validate for NaN
and the 5900–65535 range and inequality, return false when invalid (and show the
existing swal) and true when valid; then add the suggested pre-submit guard in
the form submit handlers near lines 2662 and 2809: call checkVNCPorts(), and if
it returns false re-enable inputs, reset the button value and return early to
stop the post.

function updateMAC(index, port) {
var wlan0 = '<?=$mac?>'; // mac address of wlan0
var mac = $('input[name="nic['+index+'][mac]"');
Expand Down
Loading