Skip to content

fix(core): handle JSON.parse errors in page callback and DevTools handlers #398

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

Open
lawrence3699 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(core): handle JSON.parse errors in page callback and DevTools handlers #398

Show file tree
Hide file tree
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
12 changes: 11 additions & 1 deletion agent/main/lib/Connection.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,17 @@ export class Connection extends TypedEventEmitter<{

private onMessage(message: string): void {
const timestamp = new Date();
const object = JSON.parse(message);
let object;
try {
object = JSON.parse(message);
} catch (error) {
log.warn('Connection.InvalidMessage', {
sessionId: null,
message: message.slice(0, 500),
error,
});
return;
}
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

JSON.parse can succeed but return a non-object (eg, null, a number, an array). In that case object.timestamp = ... will throw and still tear down the connection. After parsing, add a guard that object is a non-null plain object before mutating/reading fields; otherwise log+return.

Suggested change
}
}
if (
typeof object !== 'object' ||
object === null ||
Array.isArray(object)
) {
log.warn('Connection.InvalidMessageType', {
sessionId: null,
message: message.slice(0, 500),
parsedType: typeof object,
});
return;
}

Copilot uses AI. Check for mistakes.
object.timestamp = timestamp;
const devtoolsSessionId = object.params?.sessionId;

Expand Down
14 changes: 13 additions & 1 deletion core/lib/Tab.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -933,7 +933,19 @@ export default class Tab
return;
}

this.frameEnvironmentsById.get(frameId).onPageRecorderEvents(JSON.parse(payload));
let parsedPayload;
try {
parsedPayload = JSON.parse(payload);
} catch (error) {
log.warn('Tab.onPageCallback:InvalidPayload', {
sessionId: this.sessionId,
frameId,
payload,
error,
});
Comment on lines +940 to +945
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

payload for page recorder events can be very large (and may include DOM content). Logging the full raw payload on parse failure risks log bloat and potential data exposure. Consider truncating the payload (and optionally include payloadLength) similar to the 500-char cap used for DevTools messages.

Copilot uses AI. Check for mistakes.
return;
}
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

Catching JSON.parse errors prevents SyntaxErrors, but parseable non-conforming JSON (e.g., null, a number, or an unexpected object) will still be passed into onPageRecorderEvents and can throw (it assumes an array-of-arrays). Add a lightweight shape/type check (eg, Array.isArray(parsedPayload) and expected length) and warn+return when invalid.

Suggested change
}
}
// Validate payload shape: expect an array-of-arrays.
const isValidArrayOfArrays =
Array.isArray(parsedPayload) && parsedPayload.every(item => Array.isArray(item));
if (!isValidArrayOfArrays) {
log.warn('Tab.onPageCallback:InvalidPayloadShape', {
sessionId: this.sessionId,
frameId,
payload,
});
return;
}

Copilot uses AI. Check for mistakes.
this.frameEnvironmentsById.get(frameId).onPageRecorderEvents(parsedPayload);
}
}

Expand Down
Loading