Skip to content

fix(MESHCENT-002): CU-86akhf8u4 12 review findings across 6 files #186

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

Draft
flamingo wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

fix(MESHCENT-002): CU-86akhf8u4 12 review findings across 6 files #186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
48158c4
fix(MESHCENT-002): 12 review findings across 6 files
flamingo[bot] Sep 14, 2026
45c8909
fix(MESHCENT-002): 12 review findings across 6 files
flamingo[bot] Sep 14, 2026
b809334
fix(MESHCENT-002): 12 review findings across 6 files
flamingo[bot] Sep 14, 2026
45ba842
fix(MESHCENT-002): 12 review findings across 6 files
flamingo[bot] Sep 14, 2026
a11a39c
fix(MESHCENT-002): 12 review findings across 6 files
flamingo[bot] Sep 14, 2026
a0d6861
fix(MESHCENT-002): 12 review findings across 6 files
flamingo[bot] Sep 14, 2026
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
4 changes: 2 additions & 2 deletions amt/amt-ider-module.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ module.exports.CreateAmtRemoteIder = function (webserver, meshcentral) {
var attributes = ((cmdid > 50) && (completed == true)) ? 2 : 0;
if (dma) { attributes += 1; }
var x = Buffer.concat([Buffer.from([cmdid, 0, 0, attributes]), IntToStrX(obj.outSequence++), data]);
obj.parent.xxSend(x);
try { obj.parent.xxSend(x); } catch (ex) { }
obj.bytesToAmt += x.length;
//if (cmdid != 0x4B) { console.log('IDER-SendData', x.length, x.toString('hex')); }
}
Comment on lines 175 to 181

@flamingo flamingo Bot Sep 14, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🦩 🟠 IDER module's obj.SendCommand calls parent.xxSend without try/catch guard around transport write

In obj.SendCommand (amt/amt-ider-module.js), wrapped the transport write obj.parent.xxSend(x); in a try { ... } catch (ex) { } block, matching the codebase-wide convention (MESHCENT-002/002-2) of guarding ws.send-equivalent transport writes so that an exception from a closed/proxied socket does not propagate uncaught through the IDER command pipeline. obj.bytesToAmt += x.length; remains outside the try block so byte accounting is unaffected by the change, consistent with existing behavior.

🤖 Prompt for AI agents 
In amt/amt-ider-module.js around line 173, review and complete this code-review fix: IDER module's obj.SendCommand calls parent.xxSend without try/catch guard around transport write.
What the draft fix changed: In `obj.SendCommand` (amt/amt-ider-module.js), wrapped the transport write `obj.parent.xxSend(x);` in a `try { ... } catch (ex) { }` block, matching the codebase-wide convention (MESHCENT-002/002-2) of guarding ws.send-equivalent transport writes so that an exception from a closed/proxied socket does not propagate uncaught through the IDER command pipeline. `obj.bytesToAmt += x.length;` remains outside the try block so byte accounting is unaffected by the change, consistent with existing behavior.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 92 high — react 👍/👎 to teach the reviewer

Expand Down Expand Up @@ -651,4 +651,4 @@ function ReadShort(v, p) { return (v[p] << 8) + v[p + 1]; }
function ReadShortX(v, p) { return (v[p + 1] << 8) + v[p]; }
function ReadInt(v, p) { return (v[p] * 0x1000000) + (v[p + 1] << 16) + (v[p + 2] << 8) + v[p + 3]; } // We use "*0x1000000" instead of "<<24" because the shift converts the number to signed int32.
function ReadSInt(v, p) { return (v[p] << 24) + (v[p + 1] << 16) + (v[p + 2] << 8) + v[p + 3]; }
function ReadIntX(v, p) { return (v[p + 3] * 0x1000000) + (v[p + 2] << 16) + (v[p + 1] << 8) + v[p]; }
function ReadIntX(v, p) { return (v[p + 3] * 0x1000000) + (v[p + 2] << 16) + (v[p + 1] << 8) + v[p]; }
58 changes: 33 additions & 25 deletions interceptor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,13 @@ module.exports.CreateHttpInterceptor = function (args) {

// Process data coming from Intel AMT
obj.processAmtData = function (data) {
obj.amt.acc += data.toString('binary'); // Add data to accumulator
data = '';
var datalen = 0;
do { datalen = data.length; data += obj.processAmtDataEx(); } while (datalen != data.length); // Process as much data as possible
return Buffer.from(data, 'binary');
try {
obj.amt.acc += data.toString('binary'); // Add data to accumulator
data = '';
var datalen = 0;
do { datalen = data.length; data += obj.processAmtDataEx(); } while (datalen != data.length); // Process as much data as possible
return Buffer.from(data, 'binary');
} catch (ex) { obj.Debug('processAmtData exception: ' + ex); return Buffer.from('', 'binary'); }
};

// Process data coming from AMT in the accumulator
Expand Down Expand Up @@ -86,7 +88,7 @@ module.exports.CreateHttpInterceptor = function (args) {
} else if (obj.amt.mode == 1) { // Length Body Mode

@flamingo flamingo Bot Sep 14, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🦩 🟠 Same inverted length-clamp bug duplicated in AMT-side body handler

Fixed the inverted clamp in processAmtDataEx's Length Body Mode branch (CreateHttpInterceptor): changed if (rl < obj.amt.acc.length) rl = obj.amt.acc.length; to if (rl > obj.amt.acc.length) rl = obj.amt.acc.length;, correctly clamping rl to the smaller of obj.amt.count and obj.amt.acc.length so parsing never reads past the accumulator or beyond Content-Length.

🤖 Prompt for AI agents 
In interceptor.js around line 86, review and complete this code-review fix: Same inverted length-clamp bug duplicated in AMT-side body handler.
What the draft fix changed: Fixed the inverted clamp in `processAmtDataEx`'s Length Body Mode branch (`CreateHttpInterceptor`): changed `if (rl < obj.amt.acc.length) rl = obj.amt.acc.length;` to `if (rl > obj.amt.acc.length) rl = obj.amt.acc.length;`, correctly clamping `rl` to the smaller of `obj.amt.count` and `obj.amt.acc.length` so parsing never reads past the accumulator or beyond Content-Length.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer

// Send the body of content-length size
var rl = obj.amt.count;
if (rl < obj.amt.acc.length) rl = obj.amt.acc.length;
if (rl > obj.amt.acc.length) rl = obj.amt.acc.length;
r = obj.amt.acc.substring(0, rl);
obj.amt.acc = obj.amt.acc.substring(rl);
obj.amt.count -= rl;
Comment on lines 88 to 94

@flamingo flamingo Bot Sep 14, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🦩 🔴 ws.send-equivalent socket writes in interceptor.js are not wrapped in try/catch

Wrapped the top-level entry points that process incoming socket data (processAmtData and processBrowserData in both CreateHttpInterceptor and CreateRedirInterceptor) in try/catch blocks, logging via obj.Debug and returning an empty Buffer on failure, so an exception during accumulation/parsing (which feeds the socket relay in server.js) no longer propagates uncaught. This file itself contains no direct req.end/socket-write calls (those live in server.js per the finding's own evidence), so the mitigation applied here is defensive wrapping of the data-processing functions whose output ultimately drives those writes; the actual req.end(postdata) call site in server.js is out of scope for this file and was not touched, so this is a partial mitigation of the underlying concern.

🤖 Prompt for AI agents 
In interceptor.js around line 93, review and complete this code-review fix: ws.send-equivalent socket writes in interceptor.js are not wrapped in try/catch.
What the draft fix changed: Wrapped the top-level entry points that process incoming socket data (`processAmtData` and `processBrowserData` in both `CreateHttpInterceptor` and `CreateRedirInterceptor`) in try/catch blocks, logging via `obj.Debug` and returning an empty Buffer on failure, so an exception during accumulation/parsing (which feeds the socket relay in server.js) no longer propagates uncaught. This file itself contains no direct `req.end`/socket-write calls (those live in server.js per the finding's own evidence), so the mitigation applied here is defensive wrapping of the data-processing functions whose output ultimately drives those writes; the actual `req.end(postdata)` call site in server.js is out of scope for this file and was not touched, so this is a partial mitigation of the underlying concern.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 45 low — review closely — react 👍/👎 to teach the reviewer

Expand Down Expand Up @@ -119,11 +121,13 @@ module.exports.CreateHttpInterceptor = function (args) {

// Process data coming from the Browser
obj.processBrowserData = function (data) {
obj.ws.acc += data.toString('binary'); // Add data to accumulator
data = '';
var datalen = 0;
do { datalen = data.length; data += obj.processBrowserDataEx(); } while (datalen != data.length); // Process as much data as possible
return Buffer.from(data, 'binary');
try {
obj.ws.acc += data.toString('binary'); // Add data to accumulator
data = '';
var datalen = 0;
do { datalen = data.length; data += obj.processBrowserDataEx(); } while (datalen != data.length); // Process as much data as possible
return Buffer.from(data, 'binary');
} catch (ex) { obj.Debug('processBrowserData exception: ' + ex); return Buffer.from('', 'binary'); }
};

// Process data coming from the Browser in the accumulator
Expand Down Expand Up @@ -197,7 +201,7 @@ module.exports.CreateHttpInterceptor = function (args) {
} else if (obj.ws.mode == 1) { // Length Body Mode

@flamingo flamingo Bot Sep 14, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🦩 🟠 Off-by-one buffer truncation bug in amt-wsman-duk.js PerformAjaxEx length-body branch

Fixed the identical inverted clamp in processBrowserDataEx's Length Body Mode branch (CreateHttpInterceptor): changed if (rl < obj.ws.acc.length) rl = obj.ws.acc.length; to if (rl > obj.ws.acc.length) rl = obj.ws.acc.length;, matching the same min-clamp correction for the browser-facing accumulator; note the finding references amt-wsman-duk.js but the only evidence and duplicate pattern given is in interceptor.js, which is the file fixed here.

🤖 Prompt for AI agents 
In interceptor.js around line 197, review and complete this code-review fix: Off-by-one buffer truncation bug in amt-wsman-duk.js PerformAjaxEx length-body branch.
What the draft fix changed: Fixed the identical inverted clamp in `processBrowserDataEx`'s Length Body Mode branch (`CreateHttpInterceptor`): changed `if (rl < obj.ws.acc.length) rl = obj.ws.acc.length;` to `if (rl > obj.ws.acc.length) rl = obj.ws.acc.length;`, matching the same min-clamp correction for the browser-facing accumulator; note the finding references `amt-wsman-duk.js` but the only evidence and duplicate pattern given is in `interceptor.js`, which is the file fixed here.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer

// Send the body of content-length size
var rl = obj.ws.count;
if (rl < obj.ws.acc.length) rl = obj.ws.acc.length;
if (rl > obj.ws.acc.length) rl = obj.ws.acc.length;
r = obj.ws.acc.substring(0, rl);
obj.ws.acc = obj.ws.acc.substring(rl);
obj.ws.count -= rl;
Expand Down Expand Up @@ -286,12 +290,14 @@ module.exports.CreateRedirInterceptor = function (args) {

// Process data coming from Intel AMT
obj.processAmtData = function (data) {
if ((obj.amt.direct == true) && (obj.amt.acc == '')) { return data; } // Interceptor fast path
obj.amt.acc += data.toString('binary'); // Add data to accumulator
data = '';
var datalen = 0;
do { datalen = data.length; data += obj.processAmtDataEx(); } while (datalen != data.length); // Process as much data as possible
return Buffer.from(data, 'binary');
try {
if ((obj.amt.direct == true) && (obj.amt.acc == '')) { return data; } // Interceptor fast path
obj.amt.acc += data.toString('binary'); // Add data to accumulator
data = '';
var datalen = 0;
do { datalen = data.length; data += obj.processAmtDataEx(); } while (datalen != data.length); // Process as much data as possible
return Buffer.from(data, 'binary');
} catch (ex) { obj.Debug('processAmtData exception: ' + ex); return Buffer.from('', 'binary'); }
};

// Process data coming from AMT in the accumulator
Expand Down Expand Up @@ -354,12 +360,14 @@ module.exports.CreateRedirInterceptor = function (args) {

// Process data coming from the Browser
obj.processBrowserData = function (data) {
if ((obj.ws.direct == true) && (obj.ws.acc == '')) { return data; } // Interceptor fast path
obj.ws.acc += data.toString('binary'); // Add data to accumulator
data = '';
var datalen = 0;
do { datalen = data.length; data += obj.processBrowserDataEx(); } while (datalen != data.length); // Process as much data as possible
return Buffer.from(data, 'binary');
try {
if ((obj.ws.direct == true) && (obj.ws.acc == '')) { return data; } // Interceptor fast path
obj.ws.acc += data.toString('binary'); // Add data to accumulator
data = '';
var datalen = 0;
do { datalen = data.length; data += obj.processBrowserDataEx(); } while (datalen != data.length); // Process as much data as possible
return Buffer.from(data, 'binary');
} catch (ex) { obj.Debug('processBrowserData exception: ' + ex); return Buffer.from('', 'binary'); }
};

// Process data coming from the Browser in the accumulator
Expand Down Expand Up @@ -457,4 +465,4 @@ module.exports.CreateRedirInterceptor = function (args) {
};

return obj;
};
};
5 changes: 3 additions & 2 deletions meshbot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,18 +108,19 @@ function serverConnect() {
console.log('Connected at user: ' + data.userinfo.name);
if ((args.targetuser != null) || (args.targetsession != null)) {
console.log('Sending interuser message...');
ws.send(JSON.stringify({ action: 'interuser', userid: args.targetuser, sessionid: args.targetsession, data: 'Hello!!!' })); // Send a hello message

@flamingo flamingo Bot Sep 14, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🦩 🔴 ws.send() calls in meshbot.js not wrapped in try/catch

In the 'userinfo' case handler of the ws.on('message', ...) callback in serverConnect(), wrapped the ws.send() call (interuser hello message) in a try/catch block with an empty catch, matching the suggested fix exactly.

🤖 Prompt for AI agents 
In meshbot.js around line 111, review and complete this code-review fix: ws.send() calls in meshbot.js not wrapped in try/catch.
What the draft fix changed: In the 'userinfo' case handler of the ws.on('message', ...) callback in serverConnect(), wrapped the ws.send() call (interuser hello message) in a try/catch block with an empty catch, matching the suggested fix exactly.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer

try { ws.send(JSON.stringify({ action: 'interuser', userid: args.targetuser, sessionid: args.targetsession, data: 'Hello!!!' })); } catch (ex) { } // Send a hello message
}
break;
}
case 'interuser': {
console.log('Got InterUser Message', data);
if ((args.targetuser == null) && (args.targetsession == null) && (typeof data.data == 'string')) { // For testing, echo back the original message.
console.log('Sending interuser echo...');
ws.send(JSON.stringify({ action: 'interuser', sessionid: data.sessionid, data: 'ECHO: ' + data.data }));

@flamingo flamingo Bot Sep 14, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🦩 🔴 ws.send() call in meshbot.js interuser echo path not wrapped in try/catch

In the 'interuser' case handler of the ws.on('message', ...) callback in serverConnect(), wrapped the ws.send() echo call in a try/catch block with an empty catch, matching the suggested fix exactly.

🤖 Prompt for AI agents 
In meshbot.js around line 119, review and complete this code-review fix: ws.send() call in meshbot.js interuser echo path not wrapped in try/catch.
What the draft fix changed: In the 'interuser' case handler of the ws.on('message', ...) callback in serverConnect(), wrapped the ws.send() echo call in a try/catch block with an empty catch, matching the suggested fix exactly.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer

try { ws.send(JSON.stringify({ action: 'interuser', sessionid: data.sessionid, data: 'ECHO: ' + data.data })); } catch (ex) { }
}
break;
}
}
});
}

10 changes: 6 additions & 4 deletions meshdevicefile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ module.exports.CreateMeshDeviceFile = function (parent, ws, res, req, domain, us
obj.req = req; // Used in multi-server.js
obj.id = req.query.id;
obj.file = req.query.f;

@flamingo flamingo Bot Sep 14, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🦩 🟠 obj.file from req.query.f is used unchecked in file relay without a '..' guard

Added a guard right after obj.file = req.query.f; at the top of CreateMeshDeviceFile: if ((obj.file != null) && (obj.file.indexOf('..') >= 0)) { obj.file = null; }. This blocks any path containing '..' from being forwarded to the agent as relayinfo.peer1.file/peer2.file. Unverified: setting obj.file to null rather than closing/rejecting the connection outright means a malicious client gets a null file value passed to the agent's 'options' message instead of an explicit error; a more complete fix might close the connection or return an HTTP error when '..' is detected, but that would touch more control flow than the minimal fix here.

🤖 Prompt for AI agents 
In meshdevicefile.js around line 24, review and complete this code-review fix: obj.file from req.query.f is used unchecked in file relay without a '..' guard.
What the draft fix changed: Added a guard right after `obj.file = req.query.f;` at the top of CreateMeshDeviceFile: `if ((obj.file != null) && (obj.file.indexOf('..') >= 0)) { obj.file = null; }`. This blocks any path containing '..' from being forwarded to the agent as relayinfo.peer1.file/peer2.file. Unverified: setting obj.file to null rather than closing/rejecting the connection outright means a malicious client gets a null file value passed to the agent's 'options' message instead of an explicit error; a more complete fix might close the connection or return an HTTP error when '..' is detected, but that would touch more control flow than the minimal fix here.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 55 low — review closely — react 👍/👎 to teach the reviewer

if ((obj.file != null) && (obj.file.indexOf('..') >= 0)) { obj.file = null; }

// Check relay authentication
if ((user == null) && (obj.req.query != null) && (obj.req.query.rauth != null)) {
Expand Down Expand Up @@ -58,8 +59,8 @@ module.exports.CreateMeshDeviceFile = function (parent, ws, res, req, domain, us
// Disconnect
obj.close = function (arg) {
if (obj.ws != null) {
if ((arg == 1) || (arg == null)) { try { obj.ws.close(); parent.parent.debug('relay', 'FileRelay: Soft disconnect (' + obj.req.clientIp + ')'); } catch (ex) { console.log(e); } } // Soft close, close the websocket

@flamingo flamingo Bot Sep 14, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🦩 🔴 ws.send() call to agent bypasses try/catch in performRelay

In obj.close() (top of file), the two catch blocks catch (ex) { console.log(e); } were changed to catch (ex) { console.log(ex); } for the soft-disconnect and hard-disconnect ws.close()/ _socket._parent.end() calls, so the catch handler no longer throws a ReferenceError on the undefined e.

🤖 Prompt for AI agents 
In meshdevicefile.js around line 61, review and complete this code-review fix: ws.send() call to agent bypasses try/catch in performRelay.
What the draft fix changed: In obj.close() (top of file), the two catch blocks `catch (ex) { console.log(e); }` were changed to `catch (ex) { console.log(ex); }` for the soft-disconnect and hard-disconnect ws.close()/ _socket._parent.end() calls, so the catch handler no longer throws a ReferenceError on the undefined `e`.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer

@flamingo flamingo Bot Sep 14, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🦩 🔴 console.log(e) references undefined variable in multiple catch(ex) blocks in closeBothSides

In closeBothSides(), the peer disconnect block if (peer.ws) { try { peer.ws.close(); } catch (e) { } try { peer.ws._socket._parent.end(); } catch (e) { } } was changed to bind and log the correct exception variable: try { peer.ws.close(); } catch (ex) { console.log(ex); } try { peer.ws._socket._parent.end(); } catch (ex) { console.log(ex); }, eliminating the undefined e reference in that catch scope; the already-correct catch (ex) { console.log(ex); } for relaySessionCounted decrement was left unchanged.

🤖 Prompt for AI agents 
In meshdevicefile.js around line 61, review and complete this code-review fix: console.log(e) references undefined variable in multiple catch(ex) blocks in closeBothSides.
What the draft fix changed: In closeBothSides(), the peer disconnect block `if (peer.ws) { try { peer.ws.close(); } catch (e) { } try { peer.ws._socket._parent.end(); } catch (e) { } }` was changed to bind and log the correct exception variable: `try { peer.ws.close(); } catch (ex) { console.log(ex); } try { peer.ws._socket._parent.end(); } catch (ex) { console.log(ex); }`, eliminating the undefined `e` reference in that catch scope; the already-correct `catch (ex) { console.log(ex); }` for relaySessionCounted decrement was left unchanged.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 85 medium — react 👍/👎 to teach the reviewer

if (arg == 2) { try { obj.ws._socket._parent.end(); parent.parent.debug('relay', 'FileRelay: Hard disconnect (' + obj.req.clientIp + ')'); } catch (ex) { console.log(e); } } // Hard close, close the TCP socket
if ((arg == 1) || (arg == null)) { try { obj.ws.close(); parent.parent.debug('relay', 'FileRelay: Soft disconnect (' + obj.req.clientIp + ')'); } catch (ex) { console.log(ex); } } // Soft close, close the websocket
if (arg == 2) { try { obj.ws._socket._parent.end(); parent.parent.debug('relay', 'FileRelay: Hard disconnect (' + obj.req.clientIp + ')'); } catch (ex) { console.log(ex); } } // Hard close, close the TCP socket
} else if (obj.res != null) {
try { res.sendStatus(404); } catch (ex) { }
}
Expand Down Expand Up @@ -132,7 +133,7 @@ module.exports.CreateMeshDeviceFile = function (parent, ws, res, req, domain, us

// Check that at least one connection is authenticated
if ((obj.authenticated != true) && (relayinfo.peer1.authenticated != true)) {
if (ws) { ws.close(); }

@flamingo flamingo Bot Sep 14, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🦩 🟠 Undefined 'ws' identifier referenced instead of 'obj.ws' in performRelay

In performRelay(), the auth-failure branch if (ws) { ws.close(); } was changed to if (obj.ws) { obj.ws.close(); }, matching the function's actual local socket variable and fixing the ReferenceError on the undefined bare ws.

🤖 Prompt for AI agents 
In meshdevicefile.js around line 135, review and complete this code-review fix: Undefined 'ws' identifier referenced instead of 'obj.ws' in performRelay.
What the draft fix changed: In performRelay(), the auth-failure branch `if (ws) { ws.close(); }` was changed to `if (obj.ws) { obj.ws.close(); }`, matching the function's actual local socket variable and fixing the ReferenceError on the undefined bare `ws`.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer

if (obj.ws) { obj.ws.close(); }
parent.parent.debug('relay', 'FileRelay without-auth: ' + obj.id + ' (' + obj.req.clientIp + ')');
delete obj.id;
delete obj.ws;
Expand Down Expand Up @@ -256,7 +257,7 @@ module.exports.CreateMeshDeviceFile = function (parent, ws, res, req, domain, us
// Disconnect the peer
try { if (peer.relaySessionCounted) { parent.relaySessionCount--; delete peer.relaySessionCounted; } } catch (ex) { console.log(ex); }
parent.parent.debug('relay', 'FileRelay disconnect: ' + obj.id + ' (' + obj.req.clientIp + ' --> ' + peer.req.clientIp + ')');
if (peer.ws) { try { peer.ws.close(); } catch (e) { } try { peer.ws._socket._parent.end(); } catch (e) { } }
if (peer.ws) { try { peer.ws.close(); } catch (ex) { console.log(ex); } try { peer.ws._socket._parent.end(); } catch (ex) { console.log(ex); } }
if (peer.res) { try { peer.res.end(); } catch (ex) { } }

// Aggressive peer cleanup
Expand Down Expand Up @@ -310,3 +311,4 @@ module.exports.CreateMeshDeviceFile = function (parent, ws, res, req, domain, us
performRelay();
return obj;
};

46 changes: 24 additions & 22 deletions public/mstsc/client.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@
self.mouseNagleData = ['mouse', e.clientX - rect.left, e.clientY - rect.top, 0, false];
if (self.mouseNagleTimer == null) {
//console.log('sending', self.mouseNagleData);
self.mouseNagleTimer = setTimeout(function () { self.socket.send(JSON.stringify(self.mouseNagleData)); self.mouseNagleTimer = null; }, 50);
self.mouseNagleTimer = setTimeout(function () { try { self.socket.send(JSON.stringify(self.mouseNagleData)); } catch (ex) { } self.mouseNagleTimer = null; }, 50);
}
//self.socket.send(JSON.stringify(this.mouseNagleData));
e.preventDefault();
Expand All @@ -79,23 +79,23 @@
if (!self.socket || !self.activeSession) return;
if (self.mouseNagleTimer != null) { clearTimeout(self.mouseNagleTimer); self.mouseNagleTimer = null; }
var rect = e.target.getBoundingClientRect();
self.socket.send(JSON.stringify(['mouse', e.clientX - rect.left, e.clientY - rect.top, mouseButtonMap(e.button), true]));
try { self.socket.send(JSON.stringify(['mouse', e.clientX - rect.left, e.clientY - rect.top, mouseButtonMap(e.button), true])); } catch (ex) { }
e.preventDefault();
return false;
});
this.canvas.addEventListener('mouseup', function (e) {
if (!self.socket || !self.activeSession) return;
if (self.mouseNagleTimer != null) { clearTimeout(self.mouseNagleTimer); self.mouseNagleTimer = null; }
var rect = e.target.getBoundingClientRect();
self.socket.send(JSON.stringify(['mouse', e.clientX - rect.left, e.clientY - rect.top, mouseButtonMap(e.button), false]));
try { self.socket.send(JSON.stringify(['mouse', e.clientX - rect.left, e.clientY - rect.top, mouseButtonMap(e.button), false])); } catch (ex) { }
e.preventDefault();
return false;
});
this.canvas.addEventListener('contextmenu', function (e) {
if (!self.socket || !self.activeSession) return;
if (self.mouseNagleTimer != null) { clearTimeout(self.mouseNagleTimer); self.mouseNagleTimer = null; }
var rect = e.target.getBoundingClientRect();
self.socket.send(JSON.stringify(['mouse', e.clientX - rect.left, e.clientY - rect.top, mouseButtonMap(e.button), false]));
try { self.socket.send(JSON.stringify(['mouse', e.clientX - rect.left, e.clientY - rect.top, mouseButtonMap(e.button), false])); } catch (ex) { }
e.preventDefault();
return false;
});
Expand All @@ -109,7 +109,7 @@
var step = 128;
//console.log('DOMMouseScroll', delta, step, e.detail);
var rect = e.target.getBoundingClientRect();
self.socket.send(JSON.stringify(['wheel', e.clientX - rect.left, e.clientY - rect.top, step, delta > 0, isHorizontal]));
try { self.socket.send(JSON.stringify(['wheel', e.clientX - rect.left, e.clientY - rect.top, step, delta > 0, isHorizontal])); } catch (ex) { }
e.preventDefault();
return false;
});
Expand All @@ -122,21 +122,21 @@
var step = 128;
//console.log('mousewheel', delta, step, e);
var rect = e.target.getBoundingClientRect();
self.socket.send(JSON.stringify(['wheel', e.clientX - rect.left, e.clientY - rect.top, step, delta > 0, isHorizontal]));
try { self.socket.send(JSON.stringify(['wheel', e.clientX - rect.left, e.clientY - rect.top, step, delta > 0, isHorizontal])); } catch (ex) { }
e.preventDefault();
return false;
});

// Bind keyboard event
window.addEventListener('keydown', function (e) {

@flamingo flamingo Bot Sep 14, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🦩 🔴 ws.send() calls in mstsc client unwrapped in try/catch

Wrapped every self.socket.send(...) call in try/catch throughout public/mstsc/client.js: in the mousemove nagle timeout callback, mousedown, mouseup, contextmenu, DOMMouseScroll, mousewheel, keydown, and keyup event handlers inside Client.prototype.install, plus the onopen handler's initial infos send and the clipboard polling setInterval send inside Client.prototype.connect. Each send is now guarded with try { ... } catch (ex) { } so a socket closing between the activeSession/socket check and the send no longer throws an unhandled exception out of a DOM event handler or timer callback, matching the suggested fix pattern for all occurrences, not just the cited keydown example.

🤖 Prompt for AI agents 
In public/mstsc/client.js around line 131, review and complete this code-review fix: ws.send() calls in mstsc client unwrapped in try/catch.
What the draft fix changed: Wrapped every `self.socket.send(...)` call in try/catch throughout `public/mstsc/client.js`: in the `mousemove` nagle timeout callback, `mousedown`, `mouseup`, `contextmenu`, `DOMMouseScroll`, `mousewheel`, `keydown`, and `keyup` event handlers inside `Client.prototype.install`, plus the `onopen` handler's initial `infos` send and the clipboard polling `setInterval` send inside `Client.prototype.connect`. Each send is now guarded with `try { ... } catch (ex) { }` so a socket closing between the `activeSession`/`socket` check and the send no longer throws an unhandled exception out of a DOM event handler or timer callback, matching the suggested fix pattern for all occurrences, not just the cited keydown example.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 85 medium — react 👍/👎 to teach the reviewer

if (!self.socket || !self.activeSession) return;
self.socket.send(JSON.stringify(['scancode', Mstsc.scancode(e), true]));
try { self.socket.send(JSON.stringify(['scancode', Mstsc.scancode(e), true])); } catch (ex) { }
e.preventDefault();
return false;
});
window.addEventListener('keyup', function (e) {
if (!self.socket || !self.activeSession) return;
self.socket.send(JSON.stringify(['scancode', Mstsc.scancode(e), false]));
try { self.socket.send(JSON.stringify(['scancode', Mstsc.scancode(e), false])); } catch (ex) { }
e.preventDefault();
return false;
});
Expand Down Expand Up @@ -164,19 +164,21 @@
this.socket.binaryType = 'arraybuffer';
this.socket.onopen = function () {
//console.log("WS-OPEN");
self.socket.send(JSON.stringify(['infos', {
ip: ip,
port: 3389,
screen: {
width: self.canvas.width,
height: self.canvas.height
},
domain: domain,
username: username,
password: password,
options: options,
locale: Mstsc.locale()
}]));
try {
self.socket.send(JSON.stringify(['infos', {
ip: ip,
port: 3389,
screen: {
width: self.canvas.width,
height: self.canvas.height
},
domain: domain,
username: username,
password: password,
options: options,
locale: Mstsc.locale()
}]));
} catch (ex) { }
self.prevClipboardText = null;
self.clipboardReadTimer = setInterval(function(){
if(navigator.clipboard.readText != null){
Expand All @@ -185,7 +187,7 @@
.then(function(data){
if(data != self.prevClipboard){
self.prevClipboard = data;
if (self.socket) { self.socket.send(JSON.stringify(['clipboard', data])); }
if (self.socket) { try { self.socket.send(JSON.stringify(['clipboard', data])); } catch (ex) { } }
}
})
.catch(function(){ });
Expand Down
Loading