-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BUG] Writing data to System32 #120
Comments
👀 100% should not try save StoredData there. It should be using directory where the DLSS Swapper exe is located. Thanks for reporting |
No worries, hopefully it's an easy fix. It's an amazing program and very well made. Looking forward to future updates! |
Tried to replicate and struggled to get it to behave the same. Can you give me any info on what version of Windows you are using? Also are you just running the exe from the extracted folder, or are you running it directly from the zip, did you create a shortcut to it perhaps, or are you running as admin? |
So today it appears to be working normally again, nothing has changed since I last tried it. I have tried retracing the things I did in the program and could not get the same behaviour again. I was running the exe from the extracted folder without a shortcut and only ran it as admin to see if it would write the data there, which it did. I did not run it as admin before this issue appeared. Running Windows 11, 22H2. If it happens again, does the program create a log anywhere I can send you? Go ahead and close the issue if you'd like since I can't get it to happen again and I'll report it again if it does. |
Happened to me too. I only got it to do it if I ran it from the start menu (like I searched for it using start and ran it like that). If I ran the exe from the download folder it did not cause an issue. I suggest just downloading to |
Can confirm, immediately happened again when running it from a search in the windows start menu. It's normal when ran through the exe in the folder or through a shortcut. |
@Oscillate18 , I too am on Win11 22H2. I don't want to close it as it was and still is a problem. I don't think I can do any harm by failing (or successfully if running as admin) to write files there. It's just very not ideal. I added an alert on startup to show where it was going to try write files. I made the folder I then tried:
I also tried running from a zip opened in explorer which is the default behavior and that failed to run at all. @rennr , I tried I am using the current working directory to store files for portable app so it would make sense why it would store in system32, just now how it got into that state. I can change the code to not use working directory and instead use the path the executable is in but I have seen people also have trouble with that. I'd prefer if I could replicate the problem before I fix it. But I could try the change, put it out as a pre-release portable package only and see if that fixes it. Do either of you still have 1.0 portable failing? If you have 1.0 portable failing and you swap to 1.1 pre-release in the same sort of config and it works then we can assume then the fix was correct. |
It only fails and writes to system32 when launched through search, same as rennr. It's completely normal again if ran through the exe in the folder or a shortcut. I'd be happy to test a pre-release for you and see if it's still an issue. |
I don't see an issue with using This is advisable instead of writing to the application executable folder because the application executable folder is not guaranteed to be writable (it might be on a network volume, in a folder owned by another user, or a user might have copied it to Program Files even though it's not "installed", etc.). Even though this app package is portable, I would argue that expecting the app folder to be writable actually makes it less portable. |
Oh sorry, I misunderstood your usage of From memory the installer version does everything you said to do. Portable version does everything it can to not leave a trace on the PC (including putting anything in system temp) and keep everything relative to itself. Moving downloads out of temp (folder relative to the exe in the StoredData folder) will only fix that one issue. Settings, the location of the fully downloaded DLSS dlss etc, will still try to go to system32. I have a writeup on what directories are used here (I think this is still up to date). While I agree with you that the executable location may not be writable that would be a different problem all together. I think by definition of portable it means that if you put this on a USB drive, download DLSS dlls, then move that USB to another computer then all your DLSS dlls are there and ready to go. |
OK that's fair and a reasonable use case. Then I would suggest two things: (1) Try and see if the working folder is system32 on startup and, if so, set the working folder to the executable file path; (2) test to see if the working folder (or the executable file path, as the case may be) is writable and give an error if it is not and quit. I assume that options 1 and 2 I set out above would only occur if this is the portable version, since the installed version would have different behaviour based on what you said (i.e., it uses local app data instead, and if that were not writable, that would certainly be an issue!) |
I didn't know I could change working directory while the application is running. That would fix things such as accidentally writing a random file out The fake temp directory I mentioned before is fetched from here. For portable app is is using In theory if I replace that @Oscillate18 , excellent news that you can still replicate. I need to duck out for a few hours but when I get back I'll work on a fix and get a 1.1 pre-release up for you to test when you are next available. |
So when working on the fix and this new test dialog on my other PC I was able to replicate the issue. For some reason the same steps I did on Win11 PC to launch from start menu search did not work, but doing the same on my Win10 PC did work 🤷♂️ The pre-release (v1.0.1.0) is available here. When launching this app build you should get the following popup. If |
Still the same issue unfortunately with the pre-release build. |
@beeradmoore Instead of doing this: #elif PORTABLE
static string storagePath => Path.Combine(Directory.GetCurrentDirectory(), "StoredData");
#else Maybe something like this? #elif PORTABLE
static string storagePath => GetPortableStoragePathDir();
static string cachedStoragePath = null;
#else
// ...
private static string GetPortableStoragePathDir() {
// Don't do these checks if we've already done them once - just return cached data
if (cachedStoragePath != null)
return cachedStoragePath;
var currentDirectory = Directory.GetCurrentDirectory();
var systemFolder = Environment.GetFolderPath(Environment.SpecialFolder.System);
if (currentDirectory.ToLower().Contains(systemFolder.ToLower())) {
// Fall back to application folder if the current directory is the system folder
currentDirectory = Assembly.GetExecutingAssembly().Location;
}
// Test to see if it's writable
try {
Directory.CreateDirectory(Path.Combine(currentDirectory, "StoredData"));
} catch {
// Probably failed to write - fall back to generic temp folder
currentDirectory = Path.GetTempPath();
}
cachedStoragePath = Path.Combine(currentDirectory, "StoredData");
return cachedStoragePath;
} This solves a few problems: (1) If the current folder is the system folder, use the executing assembly's folder Note that I didn't test this code and just wrote it from memory, so you may have to modify it for it to compile. But this is my general thoughts on a solution to this problem. |
@Oscillate18 if you see what you see in the screenshot it should behave normally. The first line saying system32 is just demonstrating it would have failed. The other two lines are saying where it will attempt to write files now. Are you still getting issues downloading? |
@rennr , in theory that isn't needed. The fix is in, I think my instructions on how to test were just not great. The solution I did was this. Your solution fixes problem 1, but with number 2 we don't want to fall back to writing all other things like settings to system temp, storing completed downloads in temp. At this point the application has failed and we shouldn't keep falling back and back and back. But I 110% agree with what you said about we should throw a useful error. I created issue #128 to investigate this further as currently game sub-folders could be read-only and I am not sure exactly what the system does when this happens 👀. I'll test these other scenarios and try throw better messages where possible as if an error is thrown now it will just say something generic which is not useful to the end user. |
@rennr , @Oscillate18 , are either of you two able to confirm on your end that portable version of 1.0.1 pre-release works for this issue? Keeping in mind it should say system32 on the top line of that first prompt. |
it seems I get this issue if I pin to start and launch from there, even with 1.0.1 it will launch in system if its pinned to start |
Hey @Nypheena , would you be able to upload a screenshot of the error you are getting when trying to download a DLSS dll with 1.0.1? |
sorry didn't see this, it wasn't happening when downloading a DLSS I was getting the "GetCurrentDirectory:" as system32 and the DLSS I did have already downloaded wasn't showing but I have not been able to reproduce this issue since stopping to launch from the windows start menu and even if I do now it sticks to where it actually is |
This should be fixed in v1.0.2 which was just released. I am closing this issue for now. If it is still happening in this updated release comment below and I'll re-open this card. |
Describe the bug
Program attempts to write the settings and DLSS files to the system32 directory, which fails without admin access. Launching with admin access writes these files to system32 and the DLSS downloads normally again.
Using the portable version, unsure if it is the same with the installer.
Expected behavior
Should not write these files to this directory.
Screenshots
The text was updated successfully, but these errors were encountered: