Closed Bug 951028 Opened 11 years ago Closed 10 years ago

Downloaded files don't inherit NTFS permissions from the parent folder/directory

Categories

(Toolkit Graveyard :: OS.File, defect)

Product:
Toolkit Graveyard
Component:
OS.File
Version:
26 Branch
Platform:
x86_64
Windows 7
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox27 wontfix, firefox28 verified, firefox29 verified, b2g-v1.3 fixed)

Status:
VERIFIED FIXED
Milestone:
mozilla29
Tracking Flags:
Tracking Status
firefox27 --- wontfix
firefox28 --- verified
firefox29 --- verified
b2g-v1.3 --- fixed

People

(Reporter: bugzilla, Assigned: emk)

References

Details

Attachments

(2 files, 6 obsolete files)

Fix NTFS permissions when a file is moved to a different directory
7.27 KB, patch
Yoric
: review+
bbondy
: review+
Sylvestre
: approval-mozilla-beta+
Details | Diff | Splinter Review
patch for beta
7.67 KB, patch
Yoric
: review+
Details | Diff | Splinter Review 
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0 (Beta/Release)
Build ID: 20131205075310

Steps to reproduce:

After updating to FF26 under Window 7 Pro 64bit with UAC enabled I noticed files be saved with a padlock icon on them. See this thread: http://forums.mozillazine.org/viewtopic.php?f=38&t=2783765




Actual results:

One thing I just noticed about files downloaded now, at least in Windows 7 Pro 64bit w/ UAC enabled, that the files are saved without inheriting permissions from the parent's object. I did have Download Statusbar running before and maybe it was adding that flag, not sure. Anyway, one of the visible side-effects of this is that the file's icon has a padlock on it because the security tab only lists "System, My username, and Administrators". A file saved with FF25 2 days ago in the same location was inheriting permissions from the parent object and had "Authenticated Users, SYSTEM, My username, Administrators, and Users" listed in the security tab with no padlock. 

If I save the file to C:\Users\(my username)\Downloads then it saves it without a padlock and with inherit permissions turned on. If I save the file somewhere else (C:\Download) , then it does not and it has a padlock. 

I tried it with browser.download.saveZoneInformation set to both true and false and browser.download.manager.scanWhenDone set to both true and false with no apparent effect in behavior.

Note: I tried downloading a file with IE and it does save with inherited permissions so it's not a OS issue. I tried a clean Firefox profile and it did not change the behavior so no extensions I have installed are causing this change in behavior.


Expected results:

That the file is set to inherit the parent/folder's permissions.
Component: Untriaged → File Handling
Works for me with both Firefox 26 and latest Aurora 28.0a2 (build ID: 20131222004002) on Win 7 64-bit.

I've tried saving a .jpg and a .pdf file in 2 different locations (C:\Users\(my username)\Downloads and C:\Downloads), and I didn't see any padlock icon on the saved items.
(In reply to Manuela Muntean [:Manuela] [QA] from comment #1)
> Works for me with both Firefox 26 and latest Aurora 28.0a2 (build ID:
> 20131222004002) on Win 7 64-bit.
> 
> I've tried saving a .jpg and a .pdf file in 2 different locations
> (C:\Users\(my username)\Downloads and C:\Downloads), and I didn't see any
> padlock icon on the saved items.

If you move a file from C:\Users\(my username)\Downloads to C:\Downloads using the MOVE command at a command prompt do you get a file with a padlock on it? When I do this on my system I get a file with a padlock and permissions the same as what FF26 is doing. Also are you running the Pro version of Windows 7 and is UAC enabled or not?
> If you move a file from C:\Users\(my username)\Downloads to C:\Downloads
> using the MOVE command at a command prompt do you get a file with a padlock
> on it? When I do this on my system I get a file with a padlock and
> permissions the same as what FF26 is doing. 

When performing this move, I don't see a padlock.

> Also are you running the Pro version of Windows 7 and is UAC enabled or not?

I'm using Win 7 Professional Service Pack 1 on 64bit and UAC is enabled.
Hi all,
I can confirm this issue with FF26 on two different computers:

1. Win 7 Professional, Service Pack 1, 64bit, UAC enabled
2. Win 7 Home, Service Pack 1, 64bit, UAC enabled

The target of a download doesn't matter... the permissions won't be inherited (worked sometimes before).

However I found out something else. It seems like
the issue depends on the downloaded filetype
and/or on the firefox download dialog (there are different types as you will see below).

1. 
Go to http://www.mozilla.org/de/
download the round firefox icon on the right lower corner by right click and "save image as" (Grafik speichern unter...)
-> the downloaded file inherits the permissions from the parent folder -> works fine

2.
Go to http://www.mozilla.org/de/
Download the whole site by click on firefox (upper left corner), select "save site as" (Seite speichern unter)
-> the downloaded file inherits the permissions from the parent folder -> works fine

3.
Go to http://amazon-presse.de/pressetexte/pressemeldung/year/2013/month/december/day/20/article/amazon-zieht-positive-zwischenbilanz-der-weihnachtssaison.html
Download the PDF "Pressemitteilung PDF" on the right side by right click, select "save as" (Speichern unter)
-> the downloaded file DOESN'T inherit the permissions from the parent folder -> ERROR

4.
Go to http://www.videolan.org/vlc/
Download the EXE-file (it's the well known media player) by clicking the orange button at the bottom of the page
-> the downloaded file inherits permissions from the parent folder, but in this case, there are three aditional permissions which are not inherited (same permissions as I find if inheritence doesn't work) -> works more or less fine - maybe because it's an executable file?

I hope that helps...

Regards
M
What's the best steps to reproduce that? Could you define some Windows settings.
You should be able to reproduce this by creating a folder under C:\ that is not in the C:\Users path.

On two Windows 7 Professional systems (one connected to a domain, one never connected to a domain), new folders created under the C:\ root folder inherit the following permissions (these may be Microsoft defaults):

Authenticated Users: Modify
SYSTEM: Full Control
Local Administrators: Full Control
Local Users: Read & Execute

Downloads saved into a folder with those permissions had the following differences in my tests:

(1) Inherited permissions (no bug)

menu > Save Page As (HTML only)

right-click > Save Image As

(2) SYSTEM/username/Local Administrators all set to Full Control (bug)

right-click > Save Link As (link to HTML content)

PDF (Application preference: "Always Ask")

PDF downloaded from PDF Viewer toolbar (Application preference: "Preview in Firefox")

JPG pushed through a script with content-disposition attachment (e.g., download link below images here: http://www.jeffersonscher.com/photos/Maui07/Kaanapali )

My username on both test systems has local administrator privileges; I'm not sure whether that is crucial.
Maybe it would be helpful if someone posts his or her windows settings/config whose firefox inherits the permissions correctly?

How can the affect users help above all?

I would appreciate further analysis (and hopefully a fix) very much as I think, this is an important issue from an "user experience" point of view. However it seems like nobody is working on it at the moment.

Please let me know if I can provide further information.

Best Regards
M.
I am having the same problem, as already described here (german):
http://forum.chip.de/browser-add-ons/downloads-haben-falsche-berechtigungen-eigenschaften-sicherheit-seit-v26-1774345.html

and there are a few other people if you search for this bug (e.g.: http://forums.mozillazine.org/viewtopic.php?f=38&t=2783765).


For this reason I suggest to change the bug's status to confirmed...
I'm seeing this with release channel FF26 on Windows 7 and Windows Vista. It also happens with FF26 in safe mode (add-ons disabled (but not plugins)) on Windows 7 and Windows Vista. Downloads were saved to C:\Users\Public where inherited permissions should have made them accessible.

I see downloads were created with an explicit ACE:

cacls "Thunderbird Setup 24.2.0.exe"
C:\Users\Public\Thunderbird Setup 24.2.0.exe Beta\Shamus:F 
                                             NT AUTHORITY\SYSTEM:F 
                                             BUILTIN\Administrators:F 

msls from utools.com (ls with Windows ACL's) reports explicit ACE's
msls -l --ac=v "Thunderbird Setup 24.2.0.exe"
-rwx-----a  1 Shamus           22148488 Jan 11 18:04 Thunderbird Setup 24.2.0.exe
 DACL is not inherited (but not protected).
    Shamus:           Full
                      Not Inherited
    SYSTEM:           Full
                      Not Inherited
    Administrators:   Full
                      Not Inherited
(In reply to Shamus from comment #10)
> Downloads were saved to C:\Users\Public

When I try to create a file under C:\Users\Public directly, the access has been denied. But I could move a file from somewhere else to C:\Users\Public. Download Manager must have moved the downloaded file (may be from user's temporary folder).
And moved file will inherit the NTFS permission from the source folder rather than the destination folder. It will explain the symptom of the bug.
And when I create a file (DIR >A) in my Windows temp directory (C:\Users\{user}\Appdata\Local\Temp) it has the permissions I see on my downloaded files (see above).
The TEMP folder does appear to be relevant.

I gave the domain administrator read/write permission to my TEMP folder, and when I download a PDF using right-click > Save Link As into my folder with more liberal permissions, the file shows that it has inherited permissions that match the modified permissions of the TEMP folder.

Of course, that's incorrect: the permissions are not inherited from the folder in which the file currently is located. What is going on?
This bug is basically return of bug 224692. We should port the acl manipulation code to jsdownloads component or OS.File.
Yeah, after reading more about NTFS and ACL, I did some tests running CACLS on files I created using Notepad and moved with a command prompt that mimics exactly what FF26 does now.  Here it is:

C:\Users\(user name)\AppData\Local>cacls Temp
C:\Users\(user name)\AppData\Local\Temp NT AUTHORITY\SYSTEM:(OI)(CI)F
                                     BUILTIN\Administrators:(OI)(CI)F
                                     MY-PC\(user name):(OI)(CI)F


C:\Users\(user name)\AppData\Local\Temp>cacls notepad.txt
C:\Users\(user name)\AppData\Local\Temp\notepad.txt NT AUTHORITY\SYSTEM:F
                                                 BUILTIN\Administrators:F
                                                 MY-PC\(user name):F

This is what Windows does automatically because I saved the file here. It creates an ACL in the file itself. 

C:\Users\(user name)\AppData\Local\Temp>move notepad.txt c:\download
        1 file(s) moved.

C:\Users\(user name)\AppData\Local\Temp>cd\download

C:\Download>cacls notepad.txt
C:\Download\notepad.txt NT AUTHORITY\SYSTEM:F
                        BUILTIN\Administrators:F
                        MY-PC\(user name):F

Notice notepad.txt retains the exact same ACL information after being moved even though it is now in a public directory.

C:\>cacls download
C:\Download BUILTIN\Administrators:(ID)F
            BUILTIN\Administrators:(OI)(CI)(IO)(ID)F
            NT AUTHORITY\SYSTEM:(ID)F
            NT AUTHORITY\SYSTEM:(OI)(CI)(IO)(ID)F
            BUILTIN\Users:(OI)(CI)(ID)R
            NT AUTHORITY\Authenticated Users:(ID)C
            NT AUTHORITY\Authenticated Users:(OI)(CI)(IO)(ID)C

If you notice, notepad.txt has inherited none of C:\Download's permissions. It's missing both Users and Authenticated Users. The reason is that the command prompt move command just updates the file's path in the NTFS table and does nothing with the ACL.

File just downloaded with FF26 using "Save Link As":

C:\Download>cacls axelf.zip
C:\Download\axelf.zip NT AUTHORITY\SYSTEM:F
                      BUILTIN\Administrators:F
                      MY-PC\(user name):F

Now, my expectation (and most users I believe) is that files that I save to whatever folder should inherit the permissions of that folder and not keep only that of the %temp% directory where FF26 creates a temp file while downloading. A file downloaded with earlier versions of Firefox have these permissions:

C:\Download\mp3>cacls mp3tagv257setup.exe
C:\Download\mp3\mp3tagv257setup.exe NT AUTHORITY\SYSTEM:F
                                    BUILTIN\Administrators:F
                                    MY-PC\(user-name):F
                                    BUILTIN\Administrators:(ID)F
                                    NT AUTHORITY\SYSTEM:(ID)F
                                    BUILTIN\Users:(ID)R
                                    NT AUTHORITY\Authenticated Users:(ID)C

It contains a mixture of the ACL of the file itself with the additional inherited permissions of the destination folder.

For the moment, I just have it save downloads to C:\Users\(user name)\Downloads without asking me and move it to where I want it via Windows Explorer which more or less takes the same amount of effort as navigating around with the Save As dialog. 

As a reminder, I'm using Windows 7 Pro 64bit with UAC enabled.
Taking.
Assignee: nobody → VYV03354
Status: UNCONFIRMED → ASSIGNED
Component: File Handling → OS.File
Ever confirmed: true
Product: Firefox → Toolkit
This is a port from the logic of nsLocalFileWin::CopySingleFile().
https://mxr.mozilla.org/mozilla-central/source/xpcom/io/nsLocalFileWin.cpp?rev=7aaa3c22fffc#1870
I added a method instead of adding the logic to OS.File.move because the whole purpose of OS.File is lightweight-ness IIUC.
Attachment #8359096 - Flags: review?(dteller)
Unlike CopySingleFile, we have to call OS.File.fixPermissions() explicitly.
Attachment #8359098 - Flags: review?(paolo.mozmail)
Try run: https://tbpl.mozilla.org/?tree=Try&rev=7da6d601c956

And I verified the behavior locally.
Try run for the optimized build to create a test build:
https://tbpl.mozilla.org/?tree=Try&rev=33b9fd9e659a
Comment on attachment 8359096 [details] [diff] [review]
Part 1: Implement OS.File.fixPermissions

Review of attachment 8359096 [details] [diff] [review]:
-----------------------------------------------------------------

I'm not a big fan of a "fixPermissions" option. I believe that this should rather be an option of copyFile/moveFile.

::: toolkit/components/osfile/modules/osfile_async_front.jsm
@@ +721,5 @@
> + */
> +File.fixPermissions = function fixPermissions(path) {
> +  return Scheduler.post("fixPermissions",
> +    [Type.path.toMsg(path)], path);
> +};

If you wish to set file permissions, please call this File.winSetFilePermissions. It should be defined only on Windows.

::: toolkit/components/osfile/modules/osfile_win_allthreads.jsm
@@ +48,5 @@
>    // tests accordingly
>    throw new Error("Could not open system library: " + ex.message);
>  }
>  exports.libc = libc;
> +exports.advapi32 = advapi32;

Could you add documentation for this field?
Attachment #8359096 - Flags: review?(dteller) → feedback+
Comment on attachment 8359098 [details] [diff] [review]
Part 2: Use OS.File.fixPermissions from Download Manager

It seems to me that the correct way to handle this is at the OS.File.move level, either by default or through an aOptions property on the "move" operation.

In fact, I don't think we have so many moves to different directories in the code base, and probably the permissions adjustment is the right thing to do in most if not all of them, thus I think this feature can be always on, or opt-out, as long as the permissions change is done off the main thread.

So, I'm waiting for David's review there.
Attachment #8359098 - Flags: review?(paolo.mozmail)
(In reply to David Rajchenbach Teller [:Yoric] (please use "needinfo?") from comment #22)
> I'm not a big fan of a "fixPermissions" option. I believe that this should
> rather be an option of copyFile/moveFile.

Mid-air collision with David. And we're in the same room now :-)
(In reply to David Rajchenbach Teller [:Yoric] (please use "needinfo?") from comment #22)
> I'm not a big fan of a "fixPermissions" option. I believe that this should
> rather be an option of copyFile/moveFile.

> If you wish to set file permissions, please call this
> File.winSetFilePermissions. It should be defined only on Windows.

Added winFixPermissions option to OS.File.move instead of a new method. Currently this is opt-in, but if you prefer to make this opt-out, I'm fine with that.

> Could you add documentation for this field?

Added a comment.

(In reply to :Paolo Amadini from comment #23)
> In fact, I don't think we have so many moves to different directories in the
> code base, and probably the permissions adjustment is the right thing to do
> in most if not all of them, thus I think this feature can be always on, or
> opt-out, as long as the permissions change is done off the main thread.

Opt-out is possible, but I would not like to make this always-on because setting NTFS permissions could be very slow (see bug 670911).
Attachment #8359096 - Attachment is obsolete: true
Attachment #8359176 - Flags: review?(dteller)
This part will be discarded if winFixPermissions is changed to opt-out.
Attachment #8359098 - Attachment is obsolete: true
(In reply to Masatoshi Kimura [:emk] from comment #25)
> Opt-out is possible, but I would not like to make this always-on because
> setting NTFS permissions could be very slow (see bug 670911).

The UI hang described there is not a problem here, as OS.File operates off the main thread.

However, the same logic introduced in bug 670911 makes sense in "OS.File.move": when the move is actually a rename (i.e. the parent folder of the target is the same as the source), we don't need to change the permissions. In the other cases, we do that by default.

This logic would also make a lot of sense for the case at hand, because, depending on how the download was started, the part file can be in the target directory already, and there is no need for the additional permission change in that case.

In general, it also seems that in all the moves to different directories that we do (like webapp installation) we'll need to actually reset permissions on Windows (see <http://mxr.mozilla.org/mozilla-central/search?string=OS.File.move> for the full list - some are renames and some are real moves). If this was opt-in for real moves, I think developers would often forget to add the flag, especially if Windows is not their primary development platform.
https://tbpl.mozilla.org/?tree=Try&rev=85c01c8ce71a
Attachment #8359176 - Attachment is obsolete: true
Attachment #8359177 - Attachment is obsolete: true
Attachment #8359176 - Flags: review?(dteller)
Attachment #8360093 - Flags: review?(dteller)
Comment on attachment 8360093 [details] [diff] [review]
Fix NTFS permissions when a file is moved to a different directory

Review of attachment 8360093 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/osfile/modules/osfile_win_allthreads.jsm
@@ +43,3 @@
>  try {
>    libc = ctypes.open("kernel32.dll");
> +  advapi32 = ctypes.open("advapi32.dll");

Given that advapi32 will only be used seldom and not during startup, I would prefer if you could load this with the new SharedAll.Library object.

::: toolkit/components/osfile/modules/osfile_win_back.jsm
@@ +363,5 @@
> +                     /*return*/       Type.DWORD,
> +                     /*objectName*/   Type.path,
> +                     /*objectType*/   Type.DWORD,
> +                     /*securityInfo*/ Type.DWORD,
> +                     /*sidOwner*/     Type.voidptr_t.out_ptr,

I'd prefer if you defined somewhere types for PSID, PACL, etc, rather than using a raw undecorated voidptr_t.

::: toolkit/components/osfile/modules/osfile_win_front.jsm
@@ +515,5 @@
>         }
>         throw_on_zero("move",
>           WinFile.MoveFileEx(sourcePath, destPath, flags)
>         );
> +

Please document this.

@@ +519,5 @@
> +
> +       if (Path.dirname(sourcePath) !== Path.dirname(destPath)) {
> +         let dacl = new ctypes.voidptr_t();
> +         let sd = new ctypes.voidptr_t();
> +         WinFile.GetNamedSecurityInfo(destPath, Const.SE_FILE_OBJECT,

Please handle errors.

@@ +521,5 @@
> +         let dacl = new ctypes.voidptr_t();
> +         let sd = new ctypes.voidptr_t();
> +         WinFile.GetNamedSecurityInfo(destPath, Const.SE_FILE_OBJECT,
> +                                      Const.DACL_SECURITY_INFORMATION,
> +                                      null, null, dacl.address(), null, sd.address());

Could you add comments after each ptr to make reading this function call easier?

@@ +523,5 @@
> +         WinFile.GetNamedSecurityInfo(destPath, Const.SE_FILE_OBJECT,
> +                                      Const.DACL_SECURITY_INFORMATION,
> +                                      null, null, dacl.address(), null, sd.address());
> +         if (!dacl.isNull()) {
> +             WinFile.SetNamedSecurityInfo(destPath, Const.SE_FILE_OBJECT,

Please handle errors.

@@ +526,5 @@
> +         if (!dacl.isNull()) {
> +             WinFile.SetNamedSecurityInfo(destPath, Const.SE_FILE_OBJECT,
> +                                          Const.DACL_SECURITY_INFORMATION |
> +                                          Const.UNPROTECTED_DACL_SECURITY_INFORMATION,
> +                                          null, null, dacl, null);

Nit: indentation is two whitespaces.
Attachment #8360093 - Flags: review?(dteller) → feedback+
Comment on attachment 8361605 [details] [diff] [review]
Fix NTFS permissions when a file is moved to a different directory

(In reply to David Rajchenbach Teller [:Yoric] (please use "needinfo?") from comment #31)
> ::: toolkit/components/osfile/modules/osfile_win_allthreads.jsm
> @@ +43,3 @@
> >  try {
> >    libc = ctypes.open("kernel32.dll");
> > +  advapi32 = ctypes.open("advapi32.dll");
> 
> Given that advapi32 will only be used seldom and not during startup, I would
> prefer if you could load this with the new SharedAll.Library object.

Done.

> ::: toolkit/components/osfile/modules/osfile_win_back.jsm
> @@ +363,5 @@
> > +                     /*return*/       Type.DWORD,
> > +                     /*objectName*/   Type.path,
> > +                     /*objectType*/   Type.DWORD,
> > +                     /*securityInfo*/ Type.DWORD,
> > +                     /*sidOwner*/     Type.voidptr_t.out_ptr,
> 
> I'd prefer if you defined somewhere types for PSID, PACL, etc, rather than
> using a raw undecorated voidptr_t.

Defined PSID, PACL, PSECURITY_DESCRIPTOR and HLOCAL.

> ::: toolkit/components/osfile/modules/osfile_win_front.jsm
> @@ +515,5 @@
> >         }
> >         throw_on_zero("move",
> >           WinFile.MoveFileEx(sourcePath, destPath, flags)
> >         );
> > +
> 
> Please document this.

Added a comment.

> @@ +519,5 @@
> > +
> > +       if (Path.dirname(sourcePath) !== Path.dirname(destPath)) {
> > +         let dacl = new ctypes.voidptr_t();
> > +         let sd = new ctypes.voidptr_t();
> > +         WinFile.GetNamedSecurityInfo(destPath, Const.SE_FILE_OBJECT,
> 
> Please handle errors.

Added comments explaining why I didn't inspect the return value of the function. This behavior is consistent with nsIFile.

> @@ +521,5 @@
> > +         let dacl = new ctypes.voidptr_t();
> > +         let sd = new ctypes.voidptr_t();
> > +         WinFile.GetNamedSecurityInfo(destPath, Const.SE_FILE_OBJECT,
> > +                                      Const.DACL_SECURITY_INFORMATION,
> > +                                      null, null, dacl.address(), null, sd.address());
> 
> Could you add comments after each ptr to make reading this function call
> easier?

Done.

> @@ +523,5 @@
> > +         WinFile.GetNamedSecurityInfo(destPath, Const.SE_FILE_OBJECT,
> > +                                      Const.DACL_SECURITY_INFORMATION,
> > +                                      null, null, dacl.address(), null, sd.address());
> > +         if (!dacl.isNull()) {
> > +             WinFile.SetNamedSecurityInfo(destPath, Const.SE_FILE_OBJECT,
> 
> Please handle errors.

See above.

> @@ +526,5 @@
> > +         if (!dacl.isNull()) {
> > +             WinFile.SetNamedSecurityInfo(destPath, Const.SE_FILE_OBJECT,
> > +                                          Const.DACL_SECURITY_INFORMATION |
> > +                                          Const.UNPROTECTED_DACL_SECURITY_INFORMATION,
> > +                                          null, null, dacl, null);
> 
> Nit: indentation is two whitespaces.

Fixed the indent.
Attachment #8361605 - Flags: review?(dteller)
Comment on attachment 8361605 [details] [diff] [review]
Fix NTFS permissions when a file is moved to a different directory

Review of attachment 8361605 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good to me, but I would like a second review from someone who understands Windows security descriptors better than me.

::: toolkit/components/osfile/modules/osfile_win_back.jsm
@@ +120,5 @@
>          * case of success.
>          */
>         Type.zero_or_nothing =
>           Type.int.withName("zero_or_nothing");
>  

Could you add a small comment mentioning that the following types are security-related?
Attachment #8361605 - Flags: review?(dteller) → review+
Attachment #8361605 - Flags: review?(netzen) → review+
https://hg.mozilla.org/mozilla-central/rev/76f5e6076997
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
I am new to bugzillas, so please don't flame me, if the answer is obvious to you.

The bug has been marked RESOLVED FIXED, but I just updated to Firefox 27.0 and I am still having this really, really anoying bug. So should it be reopened or do I have to do something else then just updating Firefox?
Hi https. If you are new to Bugzilla, you might have the newcomer page instead of the full page, so I'm not sure exactly how much information you have access to. On the full page, you can see "Target Milestone: Mozilla 29", which means that the bug has been fixed in Firefox 29.

More generally, RESOLVED FIXED means that the bug is considered fixed in the very latest version of Firefox, but that's not necessarily the latest version of Firefox available to the general population. If you wish to use the very latest version of Firefox, you can find it here: http://nightly.mozilla.org/ – by definition, of course, this version is generally less-tested than the released version, so expect a few surprises.
Okay, so what you are saying is that normal users like myself have to wait approximately another 4 months until this annoying bug gets fixed via update? After it somehow passed through nightly, alpha and beta stage? And it took 1,5 months for it to even get fixed in first place?
I mean this bug really bothers me, even if you're only downloading a file every couple days. Isn't that kind of conterproductive regarding the number of users...?
About 3 months (12 weeks), considering Firefox 27 just came out, and of course you could use the Beta to get it 6 weeks sooner.

Out of curiosity, what are the chances of getting this uplifted to beta? The fix isn't exactly trivial, but it's early in the cycle and maybe this bug is annoying enough to consider it. I could see this being pretty bad for non-tech savvy users or those using an unprivileged account.
Flags: needinfo?(dteller)
Comment on attachment 8361605 [details] [diff] [review]
Fix NTFS permissions when a file is moved to a different directory

Good point. I don't see any reason to not uplift this patch.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: See comment 1 – under Windows, some users can't access their downloaded files.
Testing completed (on m-c, etc.): Two weeks on m-c (currently on Aurora).
Risk to taking this patch (and alternatives if risky): Can't think of any.
String or IDL/UUID changes made by this patch: None.
Attachment #8361605 - Flags: approval-mozilla-beta?
Flags: needinfo?(dteller)
Attachment #8361605 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
https: This has now landed in mozilla-beta, so I would expect it to be in Firefox 28 Beta 2 (unless Beta 1 ends up needing another candidate build). That should be out in a week or so (going by past betas). If you're not willing or able to use a beta version, the final version of 28 should be out in a little under 6 weeks from now.
That's great news! Thank you very much for speeding things up!
Keywords: verifyme
Probably because SharedAll.Library (bug 854169) was not present on Firefox 28.
I feel a bit responsible now, so I feel I should ask: can we a get a branch patch for this? It seems like a fairly simple change, something like the following:

-let advapi32 = new SharedAll.Library("advapi32", "advapi32.dll");
+let advapi32 = ctypes.open("advapi32.dll");

and

-        advapi32.declareLazyFFI(SysFile, "GetNamedSecurityInfo",
+        declareLazyFFI(SysFile, "GetNamedSecurityInfo", advapi32

and

-        advapi32.declareLazyFFI(SysFile, "SetNamedSecurityInfo",
+        declareLazyFFI(SysFile, "SetNamedSecurityInfo", advapi32

... but that's just from code inspection.
Attached patch patch for beta — — Splinter Review 
Sure, I did it in the previous patch.
Attachment #8372972 - Flags: review?(dteller)
Comment on attachment 8372972 [details] [diff] [review]
patch for beta

Review of attachment 8372972 [details] [diff] [review]:
-----------------------------------------------------------------

This should work. Since this is not just unbitrotting the patch, but actually backporting it, I'm not exactly sure how we can test this prior to uplift. Maybe it's ok since we are early in the cycle.
Attachment #8372972 - Flags: review?(dteller) → review+
Comment on attachment 8372972 [details] [diff] [review]
patch for beta

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: See comment 1 – under Windows, some users can't access their downloaded files.
Testing completed (on m-c, etc.): Tested locally.
Risk to taking this patch (and alternatives if risky): Can't think of any.
String or IDL/UUID changes made by this patch: None.
Attachment #8372972 - Flags: approval-mozilla-beta?
(In reply to David Rajchenbach Teller [:Yoric] (please use "needinfo?") from comment #50)
> This should work. Since this is not just unbitrotting the patch, but
> actually backporting it, I'm not exactly sure how we can test this prior to
> uplift. Maybe it's ok since we are early in the cycle.

Looks like all the mochitests came back orange when this initially landed, so this could probably use a try run:

https://tbpl.mozilla.org/?tree=Try&rev=4e3f5c6dce17
Comment on attachment 8372972 [details] [diff] [review]
patch for beta

Unless the new patch is changing the original uplift criteria (making it more risky or something), you don't need to re-request approval.
Attachment #8372972 - Flags: approval-mozilla-beta?
https://hg.mozilla.org/releases/mozilla-beta/rev/c6ab73e8a021

Try is green enough that I'm going to cancel the rest and go ahead and push. Thanks for the quick fix!
status-firefox28: affected → fixed
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0

Reproduced the initial issue using steps from comment 4 and Firefox 26.0RC, verified as fixed in Firefox 28 beta 3 and latest Aurora 29.0a2 under Windows 7 Professional 64-bit.
Status: RESOLVED → VERIFIED
status-firefox28: fixed → verified
status-firefox29: fixed → verified
Keywords: verifyme
Since I started this bug, thought I should post my own results with the release of FF 28. It's looks to be fixed:

C:\Download\mp3>cacls A-T-S-W.MOD
C:\Download\mp3\A-T-S-W.MOD NT AUTHORITY\SYSTEM:F
                            BUILTIN\Administrators:F
                            HEAD-PC\Headrick:F
                            BUILTIN\Administrators:(ID)F
                            NT AUTHORITY\SYSTEM:(ID)F
                            BUILTIN\Users:(ID)R
                            NT AUTHORITY\Authenticated Users:(ID)C

Thanks to everyone looking at this and correcting (in my view) the behavior. :)
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: