Last Comment Bug 541420 - Files extracted from XPI files have their executableness stripped
: Files extracted from XPI files have their executableness stripped
: regression
Product: Toolkit
Classification: Components
Component: Add-ons Manager (show other bugs)
: Trunk
: All All
-- normal (vote)
: mozilla1.9.3a1
Assigned To: Dave Townsend [:mossop]
: Andy McKay [:andym]
Depends on: 542387
Blocks: 526598
  Show dependency treegraph
Reported: 2010-01-22 09:06 PST by Dave Townsend [:mossop]
Modified: 2010-07-16 14:05 PDT (History)
7 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch rev 1 (4.81 KB, patch)
2010-01-22 18:32 PST, Dave Townsend [:mossop]
robert.strong.bugs: review+
christian: approval1.9.2.9+
Details | Diff | Splinter Review
Test extension (603 bytes, application/x-xpinstall)
2010-07-01 14:30 PDT, Tony Chung [:tchung]
no flags Details

Description User image Dave Townsend [:mossop] 2010-01-22 09:06:46 PST
The tweaks we made to the extraction process in bug 526598 made us more forceful about ensuring the file permissions were correct but this has the side effect of removing any executable bit from the file permissions. We need to make sure at least the read/write permissions are there preserving the executable permissions.
Comment 1 User image Dave Townsend [:mossop] 2010-01-22 18:32:44 PST
Created attachment 423116 [details] [diff] [review]
patch rev 1

Took a while to get a unit test that did a decent check on all platforms but I think this is it. The test zip file includes a file called "binary" that was marked executable before compressing, it should extract as executable as well.

The fix just ensures we apply at least the file access bits we need at a minimum and allows any executable bits to pass through as well.
Comment 2 User image Robert Strong [:rstrong] (use needinfo to contact me) 2010-01-25 11:32:51 PST
Comment on attachment 423116 [details] [diff] [review]
patch rev 1

Might be good to expand the comment to state that our code doesn't understand NTFS file permissions. Hopefully one day we will get that implemented.
Comment 3 User image Dave Townsend [:mossop] 2010-01-25 14:17:32 PST
Landed on trunk:, will look for branch approval shortly.
Comment 4 User image Dave Townsend [:mossop] 2010-01-27 14:28:42 PST
Backed this out while I figure out what is up with the test failures.
Comment 5 User image Dave Townsend [:mossop] 2010-01-29 15:07:29 PST
Relanded with an additional line of logging to try to narrow down where the problem is. The logging shouldn't affect most users and I'll remove it after a short period of time.
Comment 6 User image Mike Beltzner [:beltzner, not reading bugmail] 2010-03-29 10:26:45 PDT
Where are we at with this?
Comment 7 User image Dave Townsend [:mossop] 2010-03-29 10:29:36 PDT
(In reply to comment #6)
> Where are we at with this?

We have an intermittent test failure I can neither reproduce or understand which leaves me unable to be sure of how safe this is to take on branch. It appears that the failure hasn't happened in over a month though.
Comment 8 User image Daniel Veditz [:dveditz] 2010-05-24 20:04:11 PDT
Ready for a branch landing yet?
Comment 9 User image Dave Townsend [:mossop] 2010-05-25 09:52:58 PDT
Comment on attachment 423116 [details] [diff] [review]
patch rev 1

Yeah lets take this on branch, the intermittent failures appear to have gone away, I can't see any way that they could have been a code problem.
Comment 10 User image Daniel Veditz [:dveditz] 2010-06-07 10:39:27 PDT
Comment on attachment 423116 [details] [diff] [review]
patch rev 1

Approved for, a=dveditz for release-drivers
Comment 11 User image Dave Townsend [:mossop] 2010-06-08 09:41:17 PDT
Landed on branch:
Comment 12 User image Tony Chung [:tchung] 2010-07-01 14:30:59 PDT
Created attachment 455550 [details]
Test extension

the test extension attached doesnt seem to verify the fix against   reopening.   It does however, work against trunk nightly.

Tested against on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv: Gecko/20100701 Namoroka/3.6.7pre
Comment 13 User image Tony Chung [:tchung] 2010-07-01 14:39:04 PDT
oops.  this should status = verified fixed on trunk, and then i reset flags of blocking1.9.2 =? and status1.9.2 = ?.   Please correct if i missed something.
Comment 14 User image Tony Chung [:tchung] 2010-07-01 14:39:40 PDT
and verified fix on Minefield trunk:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:2.0b2pre) Gecko/20100701 Minefield/4.0b2pre.
Comment 15 User image christian 2010-07-01 14:48:23 PDT
Comment on attachment 423116 [details] [diff] [review]
patch rev 1

a-LegNeato for
Comment 16 User image Dave Townsend [:mossop] 2010-07-16 14:05:48 PDT
Landed on the branch:
Looks like the right one this time too!

Note You need to log in before you can comment on or make changes to this bug.