290735 - when opening a file from the Download Manager (nsILocalFile::launch on Windows), the working directory is set to the Firefox directory

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Bartosz Piec]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; pl-PL; rv:1.7.7) Gecko/20050414 Firefox/1.0.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; pl-PL; rv:1.7.7) Gecko/20050414 Firefox/1.0.3

After downloading a file and clicking 'Open' on it in Download Manager, it is
opened with a working directory set as a Firefox main directory. When it is i.e.
self-extracting archive, files will be extracted to this directory and can
overwrite some important Firefox files.

Reproducible: Always

Steps to Reproduce:
1. Download some self-extracting archive, i.e. this:
ftp://ftp.3com.com/pub/nic/3c59x/win2k59x.exe
2. Go to Download Manager
3. Find this file and click an 'Open' link
4. (if you downloaded the above file) Agree to the license

Actual Results:  
In Firefox directory there are extracted files (in the above example two files:
EL59X.SYS and NETEL59X.INF). I think that this is caused by setting working
directory for launched file to the Firefox directory.

Expected Results:  
Files should have been extracted to a directory where the file was downloaded
(so the working directory should be set to this directory).

Comment 1

[Szymon Konarski]

Confirmed on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2)
Gecko/20050418 Firefox/1.0+

Comment 2

[Gervase Markham [:gerv]]

This is an automated message, with ID "auto-resolve01".

This bug has had no comments for a long time. Statistically, we have found that
bug reports that have not been confirmed by a second user after three months are
highly unlikely to be the source of a fix to the code.

While your input is very important to us, our resources are limited and so we
are asking for your help in focussing our efforts. If you can still reproduce
this problem in the latest version of the product (see below for how to obtain a
copy) or, for feature requests, if it's not present in the latest version and
you still believe we should implement it, please visit the URL of this bug
(given at the top of this mail) and add a comment to that effect, giving more
reproduction information if you have it.

If it is not a problem any longer, you need take no action. If this bug is not
changed in any way in the next two weeks, it will be automatically resolved.
Thank you for your help in this matter.

The latest beta releases can be obtained from:
Firefox:     http://www.mozilla.org/projects/firefox/
Thunderbird: http://www.mozilla.org/products/thunderbird/releases/1.5beta1.html
Seamonkey:   http://www.mozilla.org/projects/seamonkey/

Comment 3

[Adam Guthrie]

*** Bug 296396 has been marked as a duplicate of this bug. ***

Comment 4

[Bartosz Piec]

This bug is still not fixed. It is present both in Fx 1.0.7 (Mozilla/5.0
(Windows; U; Windows NT 5.1; pl-PL; rv:1.7.12) Gecko/20050919 Firefox/1.0.7) and
in Fx 1.5 Beta 1 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4)
Gecko/20050908 Firefox/1.4)

Comment 5

[Clement Cherlin]

This bug is still present in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.4) Gecko/20070515 Firefox/2.0.0.4

I find this bug somewhat vexing when running executables such as Blizzard Entertainment's Bittorrent downloaders.  The downloaders save files in their current working directory, which ends up being the Firefox directory.  Once the download has completed, the downloader exits, but the downloaded file is nowhere to be found, until I remember this bug.

I have resorted to always using Open Containing Folder on executables and then double-clicking on the file in Windows Explorer so as to get the proper working directory.

Comment 7

[vaal12]

Although this is not a bug, but can be annoying for non experienced users.
I have stumbled on it developing a self extracting installer.
I have created a small program in C (working on workaround for my application):

********************PROGRAM START***********************************
#include <stdio.h>
#include <stdlib.h>

#include <Windows.h>
#include <Winbase.h>
#include<wchar.h>
#include<locale.h>

int main()
{
    //setlocale(LC_CTYPE, "utf-8");
    setlocale( LC_ALL, ".utf-8" );

    const int BUFFER_LEN = 1000;
    wchar_t * dirBuffer;
    dirBuffer = (wchar_t*) malloc (sizeof(wchar_t)*BUFFER_LEN);
    GetCurrentDirectoryW(BUFFER_LEN, dirBuffer);
    wprintf(L"\nCurrent directory:%s", dirBuffer);

    FILE * outFile;
    wchar_t * FILE_NAME = L"C:\\ffproblem.txt";
    outFile = _wfopen(FILE_NAME, L"w");
	char * convertedString;

	convertedString = (char*) malloc (sizeof(char)*BUFFER_LEN);


    //Doc: http://msdn.microsoft.com/en-us/library/dd374130%28VS.85%29.aspx
    WideCharToMultiByte(CP_UTF8, 0,
                        dirBuffer, wcslen(dirBuffer),
                        convertedString,
                        BUFFER_LEN, NULL, NULL);

    printf("\nConverted string:%s", convertedString);
	fprintf(outFile, "\nCurrent directory:%s", convertedString);

	fclose(outFile);

	wprintf(L"\nFile written");
	free (convertedString);
	free(dirBuffer);

    return 0;
}
********************PROGRAM END***********************************

Which when compiled to exe will create a file C:\ffproblem.txt with working directory of the exe file, while started.
The issue is appearing even when file is being 'downloaded' from local disk. Exe file should be started from download manager of firefox and then working directory for exe will be installation directory of the firefox, which is an essence of the issue at hand.

Compiled exe file of the programm can be found at: http://vassiliev.com.ua/FireFoxProblemProof.exe
Though I can imagine you will be afraid to run exe files from other sources, hence - source of the program above.
Also for testing purposes there is a rar sfx archive (http://vassiliev.com.ua/config.exe) which will show the run directory once executed. You also can create any sfx RAR archive and use it for testing.

To be fair this issue is also present in Opera (tested on 11th version), though IE and Chrome (current versions) are working as expected, providing working directory for the executed programm as one where it was downloaded.

Comment 8

[vaal12]

Attachment: RAR SFX archive, which will show startup directory once executed.

Comment 9

[:Gavin Sharp [email: [e-mail]]]

The download manager just calls nsLocalFileWin::Launch - this ends up calling AsyncLocalFileWinOperation::Launch, which calls ShellExecuteExW with lpDirectory==NULL, which means "current working directory".

We could pass in the path to the parent directory in the lpDirectory parameter, but that would require some munging, since AsyncLocalFileWinOperation only holds on to the path to the file to be launched.

Comment 10

[Bartosz Piec]

IMO the best solution would be to pass the directory in which the launched file is located.

Comment 11

[Brian R. Bondy [:bbondy]]

Attachment: Patch v1.

Comment 12

[Jim Mathies [:jimm]]

Comment on attachment 623536 [details] [diff] [review]
Patch v1.

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

::: xpcom/io/nsLocalFileWin.cpp
@@ +256,5 @@
> +
> +        // Use the directory of the file we're launching as the working
> +        // directory.  That way if we have a self extracting EXE it won't
> +        // suggest to extract to the install directory.
> +        WCHAR workingDirectory[MAX_PATH + 1] = { 'L\0' };

oops - 'L\0'

@@ +258,5 @@
> +        // directory.  That way if we have a self extracting EXE it won't
> +        // suggest to extract to the install directory.
> +        WCHAR workingDirectory[MAX_PATH + 1] = { 'L\0' };
> +        wcsncpy(workingDirectory,  mResolvedPath.get(), MAX_PATH);
> +        if (PathRemoveFileSpecW(workingDirectory)) {

Looks like this should never fail, so lets add an NS_WARNING if it does.

Comment 13

[Jim Mathies [:jimm]]

Does this change the working directory for the application? If so, we'll also need to reset it back. I don't think it does, but we should confirm.

Comment 14

[Brian R. Bondy [:bbondy]]

Attachment: Patch v2.

> Does this change the working directory for the application? If so, we'll also 
> need to reset it back. I don't think it does, but we should confirm.

I'm pretty certain it does not but I'll verify before landing.

Comment 15

[vaal12]

At last I have made Firefox compile and I have checked the change of the local directory by the patch and confirm that current directory stays the same as expected.
Not really sure how to submit testing patches, so please find below a code I have used. It's a bit rough, but does it's thing.

*******************************START***************************************
// Use the directory of the file we're launching as the working
        // directory.  That way if we have a self extracting EXE it won't
        // suggest to extract to the install directory.
        WCHAR workingDirectory[MAX_PATH + 1] = { L'\0' };
        wcsncpy(workingDirectory,  mResolvedPath.get(), MAX_PATH);
        if (PathRemoveFileSpecW(workingDirectory)) {
          seinfo.lpDirectory = workingDirectory;
        } else {
          NS_WARNING("Could not set working directory for launched file.");
        }

        if (ShellExecuteExW(&seinfo)) {
            GetCurrentDirectory(MAX_PATH + 1, currentDirectory);
            MessageBoxW(NULL, currentDirectory, L"After open file1.", MB_OK);
            return NS_OK;
            }

        GetCurrentDirectory(MAX_PATH + 1, currentDirectory);
        MessageBoxW(NULL, currentDirectory, L"After open file2.", MB_OK);

*******************************END***************************************
Hope it will save a couple of minutes.
Thank you Gavin, Brian and Jim for your help. I did not expect this to be so fast.

Comment 16

[Brian R. Bondy [:bbondy]]

> [e-mail] wrote:
> I have checked the change of the local directory by the patch and confirm 
> that current directory stays the same as expected.

Thanks for double verifying this, try results looks good so pushed to inbound.  
http://hg.mozilla.org/integration/mozilla-inbound/rev/fd33193a3063

Comment 17

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/fd33193a3063

Comment 18

[vaal12]

This change made to the central repository and works as expected.
Thank you everyone.