Support runtime.connectNative manifest containing relative path

RESOLVED FIXED in Firefox 50

Status

()

Toolkit
WebExtensions: Untriaged
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: gorf4673, Assigned: aswan)

Tracking

unspecified
mozilla50
Points:
---

Firefox Tracking Flags

(firefox50 fixed)

Details

(Whiteboard: [native messaging] triaged)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36

Steps to reproduce:

Have manifest and executable in the same folder
{
  "name": "com.myhost",
  "description": "messaging host",
  "path": "mynative.exe",
  "type": "stdio",
  "allowed_extensions": [ "myext" ]
}

Javascript in webextension 

var port = chrome.runtime.connectNative('com.myhost');
port.onMessage.addListener( function(message){
  console.log('Got ' + JSON.stringify(message));
}
port.onDisconnect.addListener( function(){
  if (chrome.runtime.lastError) {
      console.log('last error: ' + chrome.runtime.lastError);
  }
  console.log('disconnected from native app.');
}
port.postMessage("echo this");



Actual results:

In Browser console get

disconnected from native app. (unknown)
File at path "mynative.exe" does not exist, or is not executable
this.NativeApp</this.startupPromise<()
NativeMessaging.jsm:208



Expected results:

Should see in console "Got " with some additional text.
(Reporter)

Comment 1

2 years ago
Referring to Chrome doc - https://developer.chrome.com/extensions/nativeMessaging#native-messaging-host
Path description
On Windows it can be relative to the directory in which the manifest file is located. The host process is started with the current directory set to the directory that contains the host binary.

As experiment NativeMesaging.jsm At line 192-193  added following to force relative path
hostInfo.manifest.path = OS.Path.dirname(hostInfo.path) + "\\" + hostInfo.manifest.path

Actual result was then browser console showing
"Got "
Blocks: 1190682
(Assignee)

Updated

2 years ago
Assignee: nobody → aswan
Status: UNCONFIRMED → ASSIGNED
Iteration: --- → 50.2
Ever confirmed: true
Whiteboard: [native messaging] triaged
(Assignee)

Comment 2

2 years ago
Created attachment 8765624 [details]
Bug 1281995 Support relative paths in host manifest on windows

Review commit: https://reviewboard.mozilla.org/r/60914/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/60914/
(Reporter)

Comment 3

2 years ago
https://reviewboard.mozilla.org/r/60914/#review57782

::: toolkit/components/extensions/NativeMessaging.jsm:202
(Diff revision 1)
> -          command: hostInfo.manifest.path,
> +          command: command,
>            arguments: [hostInfo.path],
>            workdir: OS.Path.dirname(hostInfo.manifest.path),

Should workdir also refer to command?

 The host process is started with the current directory set to the directory that contains the host binary. 
 For example if this parameter is set to C:\Application\nm_host.exe then it will be started with current directory C:\Application\.
(Assignee)

Comment 4

2 years ago
Comment on attachment 8765624 [details]
Bug 1281995 Support relative paths in host manifest on windows

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60914/diff/1-2/
(Assignee)

Comment 5

2 years ago
Comment on attachment 8765624 [details]
Bug 1281995 Support relative paths in host manifest on windows

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60914/diff/2-3/
Attachment #8765624 - Flags: review?(kmaglione+bmo)
Comment on attachment 8765624 [details]
Bug 1281995 Support relative paths in host manifest on windows

https://reviewboard.mozilla.org/r/60914/#review58450

::: toolkit/components/extensions/NativeMessaging.jsm:195
(Diff revision 3)
>            throw new Error(`This extension does not have permission to use native application ${application}`);
>          }
>  
> +        let command = hostInfo.manifest.path;
> +        if (AppConstants.platform == "win") {
> +          if (!OS.Path.split(command).absolute) {

This check isn't necessary. If the path is absolute, `OS.Path.join` will return it unmodified.
Attachment #8765624 - Flags: review?(kmaglione+bmo) → review+
(Assignee)

Comment 7

2 years ago
Comment on attachment 8765624 [details]
Bug 1281995 Support relative paths in host manifest on windows

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60914/diff/3-4/
(Assignee)

Comment 8

2 years ago
Comment on attachment 8765624 [details]
Bug 1281995 Support relative paths in host manifest on windows

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60914/diff/4-5/

Comment 9

2 years ago
Pushed by aswan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b02724be5907
Support relative paths in host manifest on windows r=kmag

Comment 10

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b02724be5907
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.