Closed Bug 1519750 Opened 8 months ago Closed 3 months ago

[PATCH] Extensions using native messaging silently fail on OpenBSD

Categories

(Toolkit :: General, enhancement, P5)

64 Branch
Desktop
OpenBSD
enhancement

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox69 --- fixed

People

(Reporter: will, Assigned: will, NeedInfo)

Details

Attachments

(2 files)

Extensions using some features of native messaging appear to fail on OpenBSD. In the case that they need to launch a helper application, that application fails to launch, and the browser log contains a message about not being able to convert a null pointer to an array.

This appears to be because environ is a NULL pointer when empty, and checking for this prior to filling in the environment before launching the application fixes the issue. Patch attached.

I tested with firefox 64.0.2 on amd64 OpenBSD-current and the KeePassXC browser extension. Before this patch, trying to connect to a database would fail with a timeout, and after functionality worked perfectly.

Here's the exact error message and stacktrace:

cannot read contents of null pointer ctypes.char.ptr.ptr(ctypes.UInt64("0x0")) subprocess_unix.jsm:117
getEnvironment
resource://gre/modules/subprocess/subprocess_unix.jsm:117:30
InterpretGeneratorResume self-hosted:1255:8 next self-hosted:1210:9 getEnvironment
resource://gre/modules/Subprocess.jsm:143:5
call
resource://gre/modules/Subprocess.jsm:110:21
NativeApp</this.startupPromise<
resource://gre/modules/NativeMessaging.jsm:90:16
Component: Untriaged → General
Product: Firefox → Toolkit
Attachment #9036246 - Flags: review?(dtownsend)
Attachment #9036246 - Flags: review?(dtownsend) → review?(kmaglione+bmo)
Attachment #9036246 - Flags: review?(kmaglione+bmo) → review+
Assignee: nobody → will
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

Hey Kris, could you push this change to the try server for testing? Thanks

Flags: needinfo?(kmaglione+bmo)
Priority: -- → P5

This is trivial enough not to need a try run as long as it works locally.

Flags: needinfo?(kmaglione+bmo)

Cool. In that case, what else do I need to take care of before this can be merged? I've been running this locally for a few days now.

You just need to add the checkin-needed keyword.

Keywords: checkin-needed

Hi William, please add a commit message to the patch in the shape of:
Bug 1519750 - <describe what the patch does>. r=kmag
Also please add your name and email address to the patch, so it would be visible in Threeherder and in hg.mozilla.org that you are the author of the patch. I normally use these commands:
hg qrefresh -e (for editing commit message)
hg qrefresh -u "Name <email adress>"
After doing that please add again checkin-needed to the bug and the sheriffs will land the patch.

Flags: needinfo?(will)
Keywords: checkin-needed

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:will, could you have a look please?

Flags: needinfo?(will)

Sorry about disappearing, I moved overseas and honestly forgot about this patch. I've updated it with the commit message as requested.

Flags: needinfo?(will)
Keywords: checkin-needed

Pushed by rmaries@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6b88d6c20e57
Fix null pointer dereference in native messaging on OpenBSD. r=kmag

Keywords: checkin-needed

Hi Will:

from IRC:
pulsebot> Check-in: https://hg.mozilla.org/integration/mozilla-inbound/rev/6b88d6c20e57 - "William Orr - Bug 1519750 - Fix null pointer dereference in native messaging on OpenBSD. r=kmag
1:26 AM
<Kwan> hmm, William Orr should unquote their ui.username in hgrc

Flags: needinfo?(will)
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
You need to log in before you can comment on or make changes to this bug.