-
Notifications
You must be signed in to change notification settings - Fork 18
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
Reduce probability of temp file permission errors #44
Conversation
It less likely we will encounter system permission changes that affect the app's read/write access by using the system-determined temp path instead of always using Window's temp directory.
DwmLutGUI/DwmLutGUI/Injector.cs
Outdated
@@ -20,7 +20,7 @@ internal static class Injector | |||
|
|||
static Injector() | |||
{ | |||
var basePath = Environment.ExpandEnvironmentVariables("%SYSTEMROOT%\\Temp\\"); | |||
var basePath = Path.GetTempPath(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Crevax Add + "\\"
at the end, otherwise it won't work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, good catch! I was part-way through using Path
methods instead of string concatenation, but after seeing a bunch more people in a Discord post issues I wanted to get this out quick.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Crevax Going to test it today and I'll approve the PR
What's your reasoning for preferring the system temp path? It is because debug mode is being leveraged to hook into DWM, so process escalation is essentially keeping the app out of a user context? Typically I want to leverage core language methods and systems APIs instead of doing anything directly, but I understand that this app needs to circumvent the standard process with the lack of APIs for DWM so things have to get weird. |
Because otherwise I had to change the dll project (which expects the LUTs in the SYSTEM TEMP folder). Also, privilege escalation to SYSTEM integrity level prevents possible permission-related issues. |
Haha, oh right. I don't do any C++ development so I totally forgot about that component as well. |
Even though the app runs as Administrator, I believe Windows can still restrict access to the system's temp directory. I've seen multiple reports around temp file access being denied, so this should help avoid those cases.