The default bug view has changed. See this FAQ.

isExecutable method of nsILocalFile reports false for executable files on OSX

NEW
Unassigned

Status

()

Core
XPCOM
11 years ago
2 years ago

People

(Reporter: davel, Unassigned)

Tracking

Trunk
PowerPC
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

11 years ago
fl_class=Components.classes["@mozilla.org/file/local;1"];
ln=fl_class.createInstance(Components.interfaces.nsILocalFile);
ln.initWithPath("/bin/ls");
js> ln.isExecutable(); // this reports false ?!

isExecutable reports true for directories with the application flag set (an application bundle), but not for simple executable programs.

This does not match the unix behavior, where the above would report true.
(Reporter)

Comment 1

11 years ago
additionally, isSymlink() reports true for apple aliases, but false for unix symlinks.

Comment 2

11 years ago
I'll look into this. Dave - dougt was a good guess at an owner for this bug, but anything in xpcom that is specific to Mac OS X should probably go through me or Mark Mentovai first.
Assignee: dougt → joshmoz

Comment 3

11 years ago
I asked him to assign this to me first. :-)

Comment 4

11 years ago
My bad then - I just try to save large component owners some trouble when the problem is specific to Mac OS X.

Comment 5

11 years ago
Doug - can we remove Mac OS 8/9 support from nsLocalFileMac? It makes the code messy and causes us to do extra work. I'll probably clean up the file first to make further fixes more clear. Also, the mixed indentation in that file makes it hard to read.

Comment 6

11 years ago
Actually, we should probably just remove nsLocalFileMac. We don't ever use it.

Comment 7

11 years ago
Created attachment 208095 [details] [diff] [review]
possible fix

Something like this will probably work. This patch compiles, and Firefox runs with it, but I haven't tested the actual functionality yet. Because of that, I'm not asking for reviews. I'll test it soon if nobody else does.

Comment 8

11 years ago
Just a reminder - fixing this could have Ts consequences. Another thing to test in addition to the functionality.
(Reporter)

Comment 9

11 years ago
Problem: isExecutable on unix reports true for directories where the execute permission bits are set.

Now isExecutable on OSX reports the same.  But all app bundles are directories (and most dirs have +x)

If the intent of isExecutable is to return true if the file is something that can be run, then it is broken on unix and OSX (with this patch).

Sigh.  I'm not sure what should be done for this case . . .

I did do some basic tests

isSymlink:
regular file: false
mac alias: true
symlink: true

isExecutable:
regular file not executable: false
regular file, various x bits set: true
dir, -x: false
dir, +x: true
file, app bundle bit set, -x: true

Comment 10

11 years ago
So what you're saying is broken is broken with or without this patch right? If I understand what you're saying, it is an easy fix.
(Reporter)

Comment 11

11 years ago
First, your fix for nsLocalFile::isSymlink() on OSX looks great.
 
I'm trying to understand the bigger picture.  I don't understand windows (yet), so the following is limited to unix and OSX.  And I'm ignoring FollowLinks.

On Unix, there is one type of thing that can be run - an executable file.  On OSX, there are two types of things that can be run - an executable file, and something (directory or file) with the application flag set.

What is the purpose of nsLocalFile::isExecutable()?

1. identify executable files (files that can be passed to execve())?

2. identify stuff than can be run (executable files + applications)?

3. identify entities in the filesystem that have one or more execute permission bits set (files or directories)?

I'm thinking it is #2.

Currently (before applying your patch), unix implements 3, and OSX implements 4 (identify applications only).

Your patch changes OSX to implement 2|3.

Now is where I start to get lost in the weeds.  nsProcess::Init(nsIFile) fails if the nsIFile->isFile() is false, so you can't seem to get to the point of calling nsProcess::Run() on a directory.  That seems to make the fact that unix implements 3 (and not 1 or 2) less of an issue.

But that seems to imply that you can't launch an app bundle that is a directory on OSX using nsProcess::Run().  There's special OSX code in Run() to call LaunchApplication if the arg count is 0 . . . does LaunchApplication fall back to execve() on OSX?

Either I'm missing something, or apps and extensions built for OSX don't use nsProcess::Run() and don't use nsLocalFile::isExecutable() the way they are used on unix.

It's late and my brain is fading - I'll talk with dougt tomorrow in the office if this is still confusing.

Comment 12

11 years ago
Created attachment 208202 [details] [diff] [review]
more scratchings

This patch is the same as the last but will not mark directories with the x bit set as executable on UNIX or Mac OS X unless they are application bundles on Mac OS X.

I'll have to put some more thought into the nsProcess thing. Somebody was asking me about that a few months ago, there might be a bug filed somewhere.
Attachment #208095 - Attachment is obsolete: true

Comment 13

11 years ago
I think we should be careful about changing the meaning of nsIFile::isExecutable.  That is a frozen API, and if we've returned true for directories in the past, then we may need to continue to do so.  At the least, we should document the API better.

Any changes to the API have to be valuable enough to overcome the fact that we said the API wouldn't change.
(Reporter)

Comment 14

11 years ago
after thinking about this for the past week, I believe the first version of the patch is the one that should go in for now.  It does not allow nsProcess to work with application bundles on OSX, but that is the same behaviour as current.

and it leaves unix and win and mac (os9) alone.

Updated

11 years ago
Attachment #208095 - Attachment is obsolete: false
Blocks: 172817
Some sort of fix that can land on the branch pre-Firefox 2.0 would be nice: between this and bug 307463, the external view-source editor from bug 172817 fails on OS X either way: Foo.app/Contents/MacOS/foo on isExecutable(), Foo.app on nsProcess::init.
*** Bug 301539 has been marked as a duplicate of this bug. ***

Comment 17

10 years ago
FYI: This is currently very broken.

Without the Mac OS X version working, then you might as well not use isExecutable() at all!

If you really want to use isExecutable() a work around for the moment, as crapy as this is....

var is_executable = /Mac/.test(navigator.platform) || nsifile.isExecutable();

Ciao!

Comment 18

10 years ago
A better OS detect is at:
http://developer.mozilla.org/en/docs/Code_snippets:Miscellaneous#Operating_system_detection

Ciao!

Comment 19

10 years ago
Indeed, this is very broken, not having this function return true on executable binary files makes it unusable. The problem is exacerbated by the fact that init() doesn't seem to error (it just does nothing and return 0) so there's no way to reliably check whether you have got an executable file present, without above os-checking hack...

There was a message above saying the API documentation should explain the function better. I'd say add that if you want a portable app, you shouldn't use this at all.

Comment 20

10 years ago
Who abuses this to check whether a directory has the x flag set? Where does a Windows directory have an x flag? This is an interface to abstract directories and files from the platform specific representations. In this platform independent abstraction there is no x flag for directories.

On the Mac a file is executable if the files x flag is set. An application, that is not a flat file is executable, when it is a directory with name ending with .app and a Info.plist and PkgInfo inside and some other stuff like MacOS and Resources folder inside. If all that stuff is present and fits together it is executable.

If a directory does not match this criterias it is not an executable application but simply a directory. The x flag of a directory that does not represent the root of an excutable application is out of interest, because it is only a directory and therefore never executable. The meaning of the x flag of directories is not executable but an indicator whether it is allowed to browse into or not. So a directory, that is no root of an application should allways return false as value.

If we need platform dependent tools to get the flags or to get file attributes like known in NTFS then we need an additional interface or set of interfaces for that other purpose. But this interface should not be abused for that.

Comment 21

10 years ago
If somebody is interested in a reasonable (IMHO) workaround take a look at my ViewSourceWith code.

http://dafizilla.svn.sourceforge.net/viewvc/dafizilla/viewsourcewith/src/main/content/viewsourcewith/common.js?revision=1&view=markup#l_85

The code checks if passed filename ends with .app and then opens Info.plist xml file and extracts the CFBundleExecutable tag value.
The CFBundleExecutable contains the "real" exe file name

You should take a look also at http://dafizilla.sourceforge.net/viewsourcewith/faq-macosx.php for

Updated

8 years ago
Assignee: joshmoz → nobody
You need to log in before you can comment on or make changes to this bug.