Skip to content
This repository has been archived by the owner on Aug 12, 2018. It is now read-only.

Restart Notepad2 as Administrator, keeping the changes, when unable to save file #154

Closed
wants to merge 3 commits into from
Closed

Conversation

hmemcpy
Copy link

@hmemcpy hmemcpy commented Apr 30, 2016

Fixes #8

This PR adds a much-needed feature to restart Notepad2-mod as an Administrator, when saving a file fails, similarly to how other editors, such as Notepad++ behave.

animation

The main issue was - how to retain the changes between restarts. I solved it by saving a temporary file on disk, and passing it as a new parameter to Notepad2-mod, allowing to load the original file, but replace its text with the contents of the temp file.
The new parameter is called buffer, and it will set the text of Notepad2 to the contents of a file passed as parameter, e.g.:
notepad2.exe /buffer "C:\temp\N2ffff.tmp" "C:\Program Files (x86)\original.txt"

This allows restarting Notepad2-mod (by leveraging the RelaunchElevated function, passing it the additional buffer parameter)

…e editor from the specified file.

This is needed to replace the text buffer from a temp. save file between re-launching as elevated.
@hmemcpy hmemcpy changed the title Restart Notepad2 as Administrator, keeping the changes, when unable to save file [WIP] Restart Notepad2 as Administrator, keeping the changes, when unable to save file Apr 30, 2016
@hmemcpy hmemcpy changed the title [WIP] Restart Notepad2 as Administrator, keeping the changes, when unable to save file Restart Notepad2 as Administrator, keeping the changes, when unable to save file Apr 30, 2016
@XhmikosR
Copy link
Owner

XhmikosR commented May 1, 2016

There's also #78 which seems less intrusive to be honest.

@hmemcpy
Copy link
Author

hmemcpy commented May 1, 2016

I've seen it, but it looks like it's just trying to save a file, and prompts to restart if fails. It doesn't keep the changes as my solution does (and it also seems to be taking a dependency on a guid lib).

Is there anything in particular about my solution you find problematic? I'd be happy to rework it!

@mauroservienti
Copy link

This is indeed a much better approach compared to #78 however my question is: dies this introduce a security issue?
I mean blindly overriding the content of the file with what is found in the buffer might compromise the file that is protected by security and UAC for a good reason.
Is there a way to introduce some sort of CRC or encryption/signature to guarantee that the buffer content has not been tampered in the meanwhile?

@hmemcpy
Copy link
Author

hmemcpy commented May 1, 2016

Hmm, I'm honestly not sure, but I don't think this is a security issue. The content from the temp file is merely loaded in the text editor, saving the original file after a reload is still an explicit action by the user. FWIW, this is a similar approach that Notepad++ uses, they keep a copy of the buffer in temp files always, this is how it survives a reload. I'm not 100% sure encoding this information is worth the effort, since if an attacker can modify files (even in the %temp% dir), it's already too late...

In any case, I think this is a good suggestion, since I'd like to be 100% transparent about where the actual text is coming from, maybe display it in the title?

@hmemcpy
Copy link
Author

hmemcpy commented May 3, 2016

Is there anything else about this PR that doesn't "sit right"? I'd really love for this to be accepted into Notepad2-mod. I use this feature almost every day, having to edit files in Program Files often...

@bluenlive
Copy link
Contributor

I love this feature. It's very good.
BTW, sometime it fails with x64 version. (So I use notepad2-mod-x86)

@hmemcpy
Copy link
Author

hmemcpy commented May 3, 2016

@bluenlive I'm using the x64, and so far it worked fine on 2 machines I have, and another user which compiled the PR.
What kind of failure are we talking? A new instance does not start, or..? Have you compiled from the latest changes?

@bluenlive
Copy link
Contributor

@hmemcpy
My case is little complex.

  1. After running Notepad2-mod and open file(for example, hosts) it works well.
  2. When open hosts with Notepad2-mod, it fails. (I've done this with total commander x64)
  • Notepad2-mod is relaunched with elevated privilege
  • But, newly launched Notepad2-mod fails to save file and show error message.

I'm using Windows 10 x64 and it never happens with Notepad2-mod x86.
Thanks for reading.

@hmemcpy
Copy link
Author

hmemcpy commented May 3, 2016

@bluenlive can you post a screenshot of the error message? What happens if you initially launch Notepad2-mod elevated, and try to save the hosts file?

Also, I want to be absolutely sure, are you using the latest build off my PR sources?

@bluenlive
Copy link
Contributor

bluenlive commented May 3, 2016

@hmemcpy
Thank you for replay.

(1) Messages are as following...

error1_b_q
error2_b_q

And, it doens't happen in Notepad2-mod x86.
Error message in Korean is "The filename, directory name, or volume label syntax is incorrect".
It's Windows' error message not translated by me.

(2) When I wrote last comment, I used my own build applying your PR.
So this time, I tested with your last build (hmemcpy/notepad2-mod --> restart-as-admin).
Same result.

@hmemcpy
Copy link
Author

hmemcpy commented May 4, 2016

@bluenlive thank you so much! This seems very helpful, I'll try installing a Korean version of Windows in a VM and try it there. Will update! Thanks again!

@hmemcpy
Copy link
Author

hmemcpy commented May 4, 2016

@bluenlive ok the good news is that I successfully installed Windows 10 in Korean, and learned that Korean versions of Windows use the character to display the path separator instead of a backslash!

The bad news (well, not really bad) is that I am unable to repro the bug so far: I installed Notepad2-mod on the VM, and copied over my build (Release, x64). I was able to relaunch and than save the hosts file just fine.

How are you launching Notepad2? You mentioned Total Commander? Do you press F4 on the file?
Also, I'm not sure if this is somehow relevant, but your hosts file seem to be in UPPERCASE, on all my machines it's in lowercase. Did you make any other changes to it?

@hmemcpy
Copy link
Author

hmemcpy commented May 4, 2016

Success! I was able to reproduce this when launching from Total Commander! (pressing F4 to edit a file)
Oddly this doesn't happen on my local machine, but it happens in the Korean VM! So I have a repro.
Thank you for the bug report @bluenlive!

@hmemcpy
Copy link
Author

hmemcpy commented May 4, 2016

More interesting reading on the Yen and Won characters for path separators, if you're interested! https://web.archive.org/web/20090305082113/http://blogs.msdn.com/michkap/archive/2005/09/17/469941.aspx

@hmemcpy
Copy link
Author

hmemcpy commented May 4, 2016

Finally (sorry for spamming!), the reason for the bug is that the filename was specified twice, for some reason. Here is the command line used to launch Notepad2-mod elevated:

"C:\Program Files\Notepad2\Notepad2.exe" /buffer C:\Users\hmemcpy\AppData\Local\Temp\N259A2.tmp /z C:\Windows\notepad.exe c:\Windows\System32\drivers\etc\hosts C:\Windows\System32\drivers\etc\hosts

Note that c:\Windows\System32\drivers\etc\hosts appears twice. This means that Notepad2-mod considers it to be one single filename, which is actually a bug on its own. Launching Notepad2 with:

notepad2.exe c:\Windows\System32\drivers\etc\hosts c:\Windows\System32\drivers\etc\hosts

will fail:
image

Anyway, very interesting!

@bluenlive
Copy link
Contributor

bluenlive commented May 4, 2016

@hmemcpy Thank you for your effort. Anyway, It's a very good feature!

@bluenlive
Copy link
Contributor

bluenlive commented May 4, 2016

@hmemcpy
I've noticed that szCurFile's drive name was changed to UPPER CASE.
c:\Windows\... --> C:\Windows\...

And, in your following code in Notepad2.c,

if (szCurFile)
{
    if (!StrStr(szArguments, szCurFile)) {
        PathQuoteSpaces(szCurFile);
        wsprintf(szArguments, L"%s %s", szArguments, szCurFile);
    }
}

I've changed StrStr() to StrStrI(), and it made your code work very very well!

p.s. WDK failes to compile at following lines in Notepad2.c.
ExtractFirstArgument(lpCmdLine, lpExe, lpArgs);
LocalFree(lpArgs);

they should be fixed like these...
ExtractFirstArgument(lpCmdLine, lpExe, (LPWSTR)lpArgs);
LocalFree((LPWSTR)lpArgs);

@hmemcpy
Copy link
Author

hmemcpy commented May 4, 2016

@bluenlive that's awesome, thanks so much! I'll fix and push the changes soon.
BTW, I've suddenly realized I should not be using (and changing) szCurFile directly, there's a copy into a local tchFile variable. I'll use this instead.
Also, I've removed the PathQuoteSpaces the original readme says the filename should be unquoted.

@hmemcpy
Copy link
Author

hmemcpy commented May 4, 2016

Ok, I pushed the changes, it seems to work great now!
Also, I changed the parameter type to LPWSTR, so there's no need to cast now (it's not constant anyway)

So everything is good to go! 👍

@bluenlive
Copy link
Contributor

It works well! Thank you for your great stuff!

RaiKoHoff added a commit to RaiKoHoff/notepad2-mod-crypto that referenced this pull request Nov 22, 2016
+ massive code cleanup according to VS2015 Analyze-Mode and Warning Level 4 (partially - not finished yet)
@tofof
Copy link

tofof commented Jan 3, 2017

Looking forward to a release with this included. Any reason for the 7 month delay in merging?

@hmemcpy
Copy link
Author

hmemcpy commented Nov 2, 2017

Wow, I just saw that Notepad3 (a more active fork) had cherry picked this PR and merged it! So better try that!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants