Fragile Assumptions

[UltraHex]

As mentioned in my pull request ( #72 ) Microsoft can and will change the implementation details of its APIs. This is completely fair as the implementation is not part of the specification. As such, users of the APIs should rely solely on the specification and not the implementation. When users begin to rely on the implementation and it inevitably changes, bugs can occur ( again, #72 ). Indeed we've seen a number of examples of this with the introduction of Windows 11 24H2: Sid Meier's Alpha Centauri, GTA San Andreas

So I wonder, why are we relying on the fact that certain API functions call other API functions in order to avoid implementing more hooks? Why not simply hook all of them? If I'm not mistaken the redirection is idempotent so there should be no issues if a particular call gets hooked more than once.

Let's discuss.

[Holt59]

As mentioned in my pull request ( #72 ) Microsoft can and will change the implementation details of its APIs. This is completely fair as the implementation is not part of the specification. As such, users of the APIs should rely solely on the specification and not the implementation. When users begin to rely on the implementation and it inevitably changes, bugs can occur ( again, #72 ). Indeed we've seen a number of examples of this with the introduction of Windows 11 24H2: Sid Meier's Alpha Centauri, GTA San Andreas

So I wonder, why are we relying on the fact that certain API functions call other API functions in order to avoid implementing more hooks? Why not simply hook all of them? If I'm not mistaken the redirection is idempotent so there should be no issues if a particular call gets hooked more than once.

Let's discuss.

1. For the same reason you avoid duplicating the same code multiple times in your project. Your PR is a good example of that, you just copy/pasted the code from one function to the other, using GetModuleFileNameA instead of GetModuleFileNameW so from now on any change to the function needs to be done in both places, with a good chance that people do not recall that both are hooked so only make change in a single location (hence, adding another weird bug). This "problem" can be reduced by having common code between the two hooks.

2. Because unlike the A/W issue, there are some functions that rely on very low-level API, and hooking the high-level function is much easier than the low-level one, or vice-versa.

3. Because hooking all the functions in the Windows API would take ages, especially with the current "team" of developers, so we only hook what's needed when a problem occurs.

Probably many other reasons, but simply put, unless you re-implement the whole of Windows API, there will always be bug appearing - While you could in theory prevent breaking changes from the Windows API implementation (with a huge amount of effort), preventing issues from changes in games or other tools code is impossible, so I don't think the whole "re-hooking" everything effort is worth it.

[UltraHex]

1. Fair.
2. When I said "hook all of them" I meant only in kernel32.dll/kernelbase.dll. While technically making the same "fragile assumption", user-space libraries eventually have to talk to the kernel and that's almost entirely done through that point.
3. There can't be that many... *looks it up* Over 2000!? OK, but we only need to worry about ones that take a path as a parameter. That ought to narrow it down, right? "It doesn't", you say? Oh. Shame. 😞

[Holt59]

2. When I said "hook all of them" I meant only in kernel32.dll/kernelbase.dll. While technically making the same "fragile assumption", user-space libraries eventually have to talk to the kernel and that's almost entirely done through that point.

You also need to consider ntdll.dll, which contains ~500 functions. Those are usually especially annoying to hook...

3. There can't be that many... looks it up Over 2000!? OK, but we only need to worry about ones that take a path as a parameter. That ought to narrow it down, right? "It doesn't", you say? Oh. Shame. 😞

You would also need to consider many that takes a HANDLE or something similar, so not sure that would narrow it down that much.

We are currently hooking less than 50 functions, even if you consider that from the 2500 above, only 10% needs to be hooked, that would means 5 as much as today.

If you look at the open PR, there is an open PR for two NTDLL functions, that's still not merged. While this PR contains other changes that makes it larger than it should, the main reason this is not merged is because I am still not 100% sure that the implementation is correct and making a mistake in such places could render USVFS completely bogus for many users, so that cannot be taken lightly.

So doing that for 200 functions is probably a decade of work with the current manpower, unfortunately...

[UltraHex]

You would also need to consider many that takes a HANDLE or something similar, so not sure that would narrow it down that much.

A handle identifies a file that is already open. The redirection has already been done, no? I suppose if a function returns the path of a handle we'd need to hook that as well.

[Holt59]

You would also need to consider many that takes a HANDLE or something similar, so not sure that would narrow it down that much.

A handle identifies a file that is already open. The redirection has already been done, no? I suppose if a function returns the path of a handle we'd need to hook that as well.

That's basically (part of) the issue the current PR is trying to fix. But that's not limited to that, listing directories is done through an HANDLE to the directory, so you need hooking in that case too. There are probably various other cases too.

[Silarn]

You would also need to consider many that takes a HANDLE or something similar, so not sure that would narrow it down that much.

A handle identifies a file that is already open. The redirection has already been done, no? I suppose if a function returns the path of a handle we'd need to hook that as well.

Plenty of functions ask an open file handle where it is or more generally for info about itself, which includes the file location. These have to be hooked to update the location data to the virtual location.

I haven't looked at the PR yet but I do have a handful of hooks already implemented on a branch, making use of appropriate redirects to the W hook so as to reduce duplicate code.

[Silarn]

You know what, I may not have pushed those changes because they're somewhat tied up with Holt's current PR. My bad.