search

winfsp / winfsp

Windows File System Proxy - FUSE for Windows
https://winfsp.dev
Other
6.9k stars 494 forks source link

Add FirefoxPortable testing (was: installing FirefoxPortable fails with MEMFS) #168

Open JohnOberschelp opened 6 years ago

JohnOberschelp commented 6 years ago

Bug Report

An install of Firefox Portable fails with memfs, but succeeds with my filesystem.

How to Reproduce

1) Install winfsp-1.3.18121.msi, choosing to include developers tools. 2) Change the maximum file size to 500111000 by changing... HKLM\Software\WOW6432Node\WinFsp\Services\memfs64 CommandLine ... to... -i -F NTFS -n 65536 -s 500111000 -u %1 -m %2 3) Launch memfs with... launchctl -x64 start memfs64 testdsk "" Z: 4) Open Z:, and drop in FirefoxPortable_55.0.3_English.paf.exe 5) Launch it, taking defaults.

Behaviors

Once it has "installed", you will find the FirefoxPortable folder, which has been created next to the exe is empty. I suspect it is not a FirefoxPortable problem, as this install and subsequent execute works with my WinFsp-based filesystem.

Environment

billziss-gh commented 6 years ago

Is this a regression? I seem to recall we resolved a problem with FirefoxPortable in the past.

billziss-gh commented 6 years ago

Here is the earlier issue: #103.

JohnOberschelp commented 6 years ago

I am thinking it is related. In Issue #103, I could see the FirefoxPortable.exe file in the install-created FirefoxPortable folder. Now, the folder is "empty".

But, I just now installed FirefoxPortable as described before, opened a command prompt and did...

C:\Users\John>z:

Z:\>cd FirefoxPortable

Z:\FirefoxPortable>dir
 Volume in drive Z is MEMFS
 Volume Serial Number is 1105-09E9

 Directory of Z:\FirefoxPortable

05/10/2018  09:54 AM    <DIR>          .
05/10/2018  09:53 AM    <DIR>          ..
               0 File(s)              0 bytes
               2 Dir(s)  32,622,764,124,160 bytes free

Z:\FirefoxPortable>FirefoxPortable.exe

... and it launches.

billziss-gh commented 6 years ago

I can confirm the problem. This was definitely working before, so it must be a recent regression(?). I will investigate.

billziss-gh commented 6 years ago

This is now doubly embarrassing. The problem still appears to be related to the ordering of directory entries in the internal MEMFS map.

capture

Notice the sorting order:

\
\FirefoxPortable        <- directory
\FirefoxPortable_55...  <- installer
\FirefoxPortable_55...  <- stream on the installer
\FirefoxPortable\App    <- directory contents
...

It should be:

\
\FirefoxPortable        <- directory
\FirefoxPortable\App    <- directory contents
...
\FirefoxPortable_55...  <- installer
\FirefoxPortable_55...  <- stream on the installer
billziss-gh commented 6 years ago

The problem appears to be this use of CompareStringW.

If I change the code like this:

#if 0
        res = CompareStringW(LOCALE_INVARIANT, NORM_IGNORECASE, a, alen, b, blen);
        if (0 != res)
            res -= 2;
        else
#endif
            res = _wcsnicmp(a, b, len);

Everything works just fine:

capture

CompareString may be failing because I purposefully translate ':' to '\x1' and '\\' to '\x2' to create the proper map order. I believe characters '\x1' and '\x2' are not valid Unicode.

The CompareString addition was introduced in commit f8a05eae9541e5f21c083103a93a2937b32e9390 (a few days after the original FirefoxPortable commits). Probably a boneheaded decision on my part, because _wcsnicmp does the right thing here.

The fix is to revert that commit or see if we can instruct CompareStringW to include illegal characters in its comparison.

Finally I believe that FirefoxPortable has earned a place in the WinFsp test suite! 😄

JohnOberschelp commented 6 years ago

😄

billziss-gh commented 6 years ago

Commit 8727497662224040b5469adb18ec4667477b9642 should fix this. It will be included in the upcoming 2018.1 release.

I am leaving this issue here, because I want it to serve as a reminder to add FirefoxPortable testing in WinFsp.

billziss-gh commented 6 years ago

Argh! This change fails one of the Unicode tests on Microsoft's IfsTest:

OpenCreateGeneral.UnicodeOnDiskTest............................ KO
    Test         :UnicodeOnDiskTest
    Group        :OpenCreateGeneral
    Status       :C000002E (IFSTEST_TEST_UNICODE_NAME_PRESERVED)
    LastNtStatus     :C000000F STATUS_NO_SUCH_FILE
    ExpectedNtStatus :00000000 STATUS_SUCCESS
    Description  :{Msg# OpCreatG12} Failure quering directory
                  for a file/directory with unicode name.

We will have to come up with a better fix.

JohnOberschelp commented 6 years ago

Would you like me to modify memfs so that instead of using 1 map, it would use a map per directory and a map per file that had named streams?

billziss-gh commented 6 years ago

Actually that would be a fantastic contribution.

Perhaps it would be best to fork MEMFS and create a new file system (e.g. "Memfs2" or "NewMemfs" or "JohnsMemfs" or "ProperMemfs") that eventually takes over as the reference WinFsp file system once we stabilize it!

JohnOberschelp commented 6 years ago

Sounds good. I think I'll give it a go.

billziss-gh commented 6 years ago

Excellent! Thank you!

billziss-gh commented 6 years ago

I completely rewrote MemfsFileNameCompare in commit 5f325304d369dba904fe6f2c72e1ec3ef05b26f5. This passes all tests on appveyor and fixes the issues with FirefoxPortable.

JohnOberschelp commented 6 years ago

From Memfs source, I have (finally) created Airfs.

I can install, launch, and run FirefoxPortable under it, as well as run a small custom path torture test. I have not done any other testing (yet).

It is currently in its own Visual Studio solution and links against fsctl.h, winfsp.h, and winfsp-*.lib from winfsp-1.3.

So what do you suggest I/we do with it?

billziss-gh commented 6 years ago

Airfs is an interesting name. I hesitate to speculate on its origin :)

So what do you suggest I/we do with it?

Please tell me a little bit more about it.

JohnOberschelp commented 6 years ago
billziss-gh commented 6 years ago

It uses a Set per directory, and a Set per file or directory that has alternate data streams.

I would expect some kind of mapping name -> file or name -> stream, or do I misunderstand?

I think of it as a drop-in replacement; it is very similar: it has the same parameters passed, and the same feature controlling defines.

The winfsp-tests test-suite has two main testing modes: "internal" and "external". In the "internal" mode it runs a number of esoteric tests that exercise various corners of WinFsp functionality. In the "external" mode it can be used to test any file system.

In order to run the "internal" tests winfsp-tests needs to implement its own file system and for this reason it links with Memfs.

Likewise fscrash (which is used for crash tolerance testing) also links with Memfs.

I think it could fill both those roles, as Memfs does, after a good shakedown, but I am comfortable with any role it might fill.

Agree.

I think as a first step we can include it as an additional sample file system and start stabilizing it. Once we achieve stability with winfsp-tests "external" mode and Microsoft's IfsTest, we can consider taking this further.

Depending on how close to Memfs it behaves, we may be able to link it with winfsp-tests and fscrash eventually and fully replace Memfs.

One drawback that I see is that as new features are added to WinFsp, we would have to add them to both file systems (at least during the transitional period).

This code is of course to be released under the Contributor Agreement.

Fantastic. If you have this somewhere available I am happy to take a look.

BTW, I am also leaving for vacation this weekend, so I may not always be fast to respond.

JohnOberschelp commented 6 years ago

Using the Standard Template Library ordered containers (not a big fan; I think STL is cumbersome), you have the choice of an external key (Map), or an internal key (Set); I switched the code from Map to Set since the file name is the key and already in each node.

I think as a first step we can include it as an additional sample file system and start stabilizing it. Once we achieve stability with winfsp-tests "external" mode and Microsoft's IfsTest, we can consider taking this further.

So as a next step, do you think I should get winfsp-tests to use Airfs, and start comparing results to Memfs, or am I lost?

I just emailed you an Airfs Visual Studio Solution. My ugly but easy test is to build it, then on my test machine, with a full install of Winfsp-1.3.18121.msi, and the registry maximum file size for Memfs set to 500111000:

Speaking of ugly, the code I added to SetAllocSize occasionally over-allocates a file by 0 to 50%, but for example it triples the install speed of FirefoxPortable.

Also, if you think it is a good idea, I would like to add code to exercise the new DeviceIoControl API, as I needed that for another project. If so, any ideas as to what small useful or interesting Control to implement?

I have just started digging in to your test code. There is a lot there!

billziss-gh commented 6 years ago

Using the Standard Template Library ordered containers (not a big fan; I think STL is cumbersome), you have the choice of an external key (Map), or an internal key (Set); I switched the code from Map to Set since the file name is the key and already in each node.

Ok, makes sense.

Old C++ guy here, but never a fan of STL myself :) I just use map when I need order or unordered_map when I do not and I am not in the mood of implementing a hash table myself.

So as a next step, do you think I should get winfsp-tests to use Airfs, and start comparing results to Memfs, or am I lost?

I think this would give us the option of switching Memfs to Airfs in the future.

It would also allow you to more easily test Airfs by fully integrating it into the WinFsp test suite.

I just emailed you an Airfs Visual Studio Solution.

I will have a look.

Also, if you think it is a good idea, I would like to add code to exercise the new DeviceIoControl API, as I needed that for another project. If so, any ideas as to what small useful or interesting Control to implement?

In MEMFS I just implemented a ROT13 function. Completely useless, except for testing purposes.

Otherwise the only thing I have ever used such functionality for is to make dumps of my internal file system structures. For example, in one file system I was maintaining file metadata in a B-tree of sorts, so I had a method for dumping the tree in debug builds.

However ioctl functionality has often been requested by people. So there must be other useful scenarios for it, but I think they are file system specific.

I have just started digging in to your test code. There is a lot there!

Unfortunately winfsp-tests is quite badly written code. It grew over time as I was developing WinFsp and has been changed, updated, mutilated and transmogrified multiple times to support ever evolving functionality and scenarios.

The good news is that if Airfs has the same interface as winfsp-tests you should not have to stare at that code for too long.

billziss-gh commented 6 years ago

@JohnOberschelp I am doing some issue cleanup and I am revisiting this issue.

It would be nice to add FirefoxPortable testing as it has found one issue and one regression in Memfs! I do not use Firefox much these days, so I do know if it is possible to script it (for testing purposes).

The test I am thinking of would probably involve the following steps:

  1. Install FirefoxPortable. Maybe test that certain files are in place, etc.
  2. Open an HTML file using the newly installed executable and (somehow?) check that it worked.
  3. Uninstall FirefoxPortable.

Just wondering if you know if Firefox can be scripted/automated.

JohnOberschelp commented 6 years ago

I know of nothing built into FF or bundled with Windows 10 that will do this.

The way I do it ain't pretty, but I sometimes use AutoHotKey.

This crude .ahk script will install FP, launch it, do a little control, exit, clean up, and return an error code if it doesn't finish in a minute. (At least it worked twice for me, so it is good enough, right??)

RunAs
SetControlDelay 0
SetWorkingDir %A_ScriptDir%

Result=1
settimer, timeOut, 60000 ; Exit with error if not done in 1 minute.

;;;;;;;;;;;;;;;;;;;;  Install FirefoxPortable
run, FirefoxPortable_55.0.3_English.paf.exe
WinWaitActive, ahk_class #32770, This wizard will guide you
ControlClick, Button2, ahk_class #32770
WinWaitActive, ahk_class #32770, Choose Install Location
ControlClick, Button2, ahk_class #32770
WinWaitActive, ahk_class #32770, Click Finish
ControlClick, Button2, ahk_class #32770

;;;;;;;;;;;;;;;;;;;;  Use FirefoxPortable
run, C:\Users\John\Desktop\ahktest\FirefoxPortable\FirefoxPortable.exe https://github.com/billziss-gh/winfsp
Sleep, 5000
Send, ^t
Sleep, 5000
Send, google.com{enter}
Sleep, 5000

;;;;;;;;;;;;;;;;;;;;  Leave FirefoxPortable
Send, !{F4}
Sleep, 1000
Send, {enter}
Result=0

timeOut:
;;;;;;;;;;;;;;;;;;;;  Remove FirefoxPortable
Sleep, 3000
FileRemoveDir, FirefoxPortable, 1
Sleep, 1000
Exit, %Result%
billziss-gh commented 6 years ago

John, this will work just fine for now. Thanks!