Bug 424939 - The browser integration extension should check the port status before using it.
Summary: The browser integration extension should check the port status before using it.
Status: REPORTED
Alias: None
Product: plasma-browser-integration
Classification: Plasma
Component: Firefox (other bugs)
Version First Reported In: unspecified
Platform: Other Other
: NOR minor
Target Milestone: ---
Assignee: Kai Uwe Broulik
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-08-02 19:02 UTC by Dietrich Daroch
Modified: 2020-08-02 19:26 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed/Implemented In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dietrich Daroch 2020-08-02 19:02:24 UTC
SUMMARY

The Firefox integration extension is not checking a port status before using it,
which makes journalctl logs very cluttered.


STEPS TO REPRODUCE
1. Run firefox with the plasma integration app
2. Check journalctl

OBSERVED RESULT

Logs are cluttered with,
```
│Aug 02 11:44:57 (hostname) xsession[3458]: JavaScript error: moz-extension://41f4e5f9-376d-4fcf-aa73-f662502f23bc/extension-utils.js, line 0: Error: Attempt to postMessage on disconnected port
```

ISSUE

I see that moz-extension://41f4e5f9-376d-4fcf-aa73-f662502f23bc/extension-utils.js
calls `port.postMessage` in 2 places, both unchecked,

```

function sendPortMessage(subsystem, event, payload)
{
    // why do we put stuff on root level here but otherwise have a "payload"? :(
    var message = payload || {}
    message.subsystem = subsystem;
    message.event = event;

    port.postMessage(message);
    ///  ^^^^^^^^^^^
    ///  line 0: Error: Attempt to postMessage on disconnected port
}

function sendPortMessageWithReply(subsystem, event, payload)
{
    return new Promise((resolve, reject) => {
        let message = payload || {};
        message.subsystem = subsystem;
        message.event = event;
        ++currentMessageSerial;
        if (currentMessageSerial >= Math.pow(2, 31) - 1) { // INT_MAX
            currentMessageSerial = 0;
        }
        message.serial = currentMessageSerial;

        port.postMessage(message);  ///   <-------
        ///  ^^^^^^^^^^^
        ///  line 0: Error: Attempt to postMessage on disconnected port

        pendingMessageReplyResolvers[message.serial] = resolve;
    });
}
```

Also, maybe the port disconnection is itself also a bug, but even if it were not
the defensive checks should still be in place.

EXPECTED RESULT

  * No verbose errors are being reported.
  * The postMessage calls check for the port status prior to using it.


SOFTWARE/OS VERSIONS

Operating System: NixOS 20.09pre236419.a45f68ccac4
KDE Plasma Version: 5.17.5
KDE Frameworks Version: 5.71.0
Qt Version: 5.12.7
Kernel Version: 5.4.54
Comment 1 Dietrich Daroch 2020-08-02 19:10:26 UTC
I'm not sure if suspending my laptop plays a role in breaking the connection or not.

It could be important to reproduce, but I guess irrelevant to the fix.
Comment 2 Kai Uwe Broulik 2020-08-02 19:26:00 UTC
Firejail prevents connecting to the host altogether and is not supported at this time.