Skip to content

Unguarded JSON.parse in mds/mds.js — MDS.cmd callbacks can be lost forever #10

Description

@Drug0j

Unguarded JSON.parse in mds/mds.js — MDS.cmd callbacks can be lost forever
File: mds/mds.js (master)
Lines: 712 (httpPostAsync) and 751 (httpPostAsyncPoll)
Impact: MiniDAPPs can hang permanently on a single malformed HTTP response.
Summary
Both XHR handlers parse the response with a bare JSON.parse:

//Send it to the callback function..
if(callback){
callback(JSON.parse(xmlHttp.responseText));
}

If responseText is empty or truncated, JSON.parse throws inside
onreadystatechange. The exception escapes as an uncaught error and
callback is never invoked.
Why line 712 is the serious one
MDS.cmd (and MDS.sql, MDS.file., MDS.keypair., MDS.net.* — every
command API) routes through httpPostAsync:

cmd : function(command, callback){
httpPostAsync("cmd", command, callback);

The standard MiniDAPP pattern wraps this in a Promise:

function cmd(command){
return new Promise((resolve, reject) => {
MDS.cmd(command, (res) => resolve(res)); // never called -> never settles
});
}

When the parse throws, resolve is never reached, so the promise never
settles. Any await on it hangs forever. If the dapp serialises work behind
that await (a mutex, a sequential task queue), the whole dapp deadlocks and the
only recovery is a page reload — the caller gets no error, no rejection, no
timeout, nothing.
We hit exactly this in a Maxima-based MiniDAPP: one lost sign callback
permanently blocked the action queue. We worked around it with an 8s timeout on
every MDS.cmd call, which turns the deadlock into a recoverable error — but
that workaround has to be duplicated by every dapp, for every command.
Line 751 (poll) is less severe but still wrong
PollListener runs under setInterval(...,2500), so the poll loop itself keeps
firing after a throw. The cost is a dropped message batch plus a stream of
uncaught Unexpected end of JSON input errors in the dapp console. Since
PollCounter is only advanced inside the callback, not advancing it on failure
is actually the correct recovery — the next poll re-requests the same counter.
That just needs to happen deliberately rather than via an exception.
Reproduction
Any condition producing an empty/truncated response body reproduces it. In our
case it appeared under sustained Maxima traffic, tens of times per session:

Uncaught SyntaxError: Unexpected end of JSON input
at JSON.parse ()
at xmlHttp.onreadystatechange (https://127.0.0.1:9103/0x/mds.js:712:26)

Same trace at :751:26. Observed on node v1.0.46.x / v1.0.47.x.
Proposed fix
See mds_js_jsonparse.patch. Two guards, no API change:
httpPostAsync — on parse failure, still invoke callback with a
standard failure object ({command, status:false, pending:false, error}),
matching the shape callers already handle. Callers get an error instead of
silence, so nothing can hang.
httpPostAsyncPoll — on parse failure, log and skip the batch without
invoking the callback, leaving PollCounter unadvanced so the next poll
re-requests the same counter.
Patched file passes node --check.
Why it's worth fixing upstream
The dapp-side workaround (timeout every command) is possible but has to be
repeated in every MiniDAPP by every developer, and it can only convert the hang
into an error after an arbitrary delay — it can't recover the lost response.
Fixing it in mds.js removes an entire class of silent, hard-to-diagnose
deadlocks for every dapp on the platform.

mds_patched.js

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions