samuel-lucas6 / Kryptor

A simple, modern, and secure encryption and signing tool that aims to be a better version of age and Minisign.
https://www.kryptor.co.uk
GNU General Public License v3.0
414 stars 33 forks source link

FileDecryption.cs: Solves issue #28. #29

Closed sh-dv closed 2 years ago

sh-dv commented 2 years ago

Solves issue #28 .

Reformatting decrypted file name using regex. Now there is no need to worry about having bad extension names after decryption due to file duplication even if the filename is Document.txt (2)(1).kryptor

sh-dv commented 2 years ago

Thanks, That's totally fine, you can do that. I just want to clarify that this Regex pattern locates parenthesis in a given string and remove what's inside them too. However, you can do more regex patterns for brackets, slashes, dashes etc, or renaming in general. you can find full pattern list here. This pull request maybe only solves the issue 28 because it's system related (windows). Notice : i just figured that if a file is called (document).txt.kryptor the decrypted output will be a txt file with no name, but still can be opened. (Rare case because who names files like that..)

samuel-lucas6 commented 2 years ago

Regex pattern locates parenthesis in a given string and remove what's inside them too

Yes, thank you for reminding me. Thinking about it more thoroughly, this isn't going to work because if there are brackets elsewhere in the file path, then it will cause a DirectoryNotFoundException. Instead, this needs to only be applied to the content in-between the actual file extension and the .kryptor extension for the file name. That probably means modifying the GetUniqueFilePath() function to find both extensions (e.g. calling Path.GetExtension() on the fileNameWithoutExtension variable), checking whether there's a (number) in-between, and removing it if there is (e.g. using that Regex).

When I first saw this issue, I thought about modifying the GetUniqueFilePath() function to put the (number) before both file extensions (e.g. test (2).txt.kryptor). Unfortunately, that would mean that you can't revert to the original file name since you don't know whether the original file name included the number in brackets or not, so that's a bad idea.

Let me know if you're happy to give that a go. My bad for not noticing that; I'm multitasking when I shouldn't be.

sh-dv commented 2 years ago

I think this should solve both problems, it needs testing though.

Unfortunately, that would mean that you can't revert to the original file name since you don't know whether the original file name included the number in brackets or not, so that's a bad idea.

I totally agree, it's better to only work on the file extension for the moment. Until further development regarding file names and extensions.

sh-dv commented 2 years ago

I suggest separate functions for the extension (filename) , for encryption and decryption. Some tests failed.

sh-dv commented 2 years ago

The latest file updates should solve the issue. i did the tests again and all passed. Sorry for the inconvenience earlier. Basically made a new function called GetUniqueDecryptedFilePath() that handles the decrypted file naming. Instead of the extension name being replace on GetOutputFilePath() in FileDecryption.cs.

sh-dv commented 2 years ago

I'm afraid this still doesn't seem to work. document.txt (2).kryptor still decrypts to document.txt (2) for me because the file doesn't exist, meaning the Regex part is never reached.

it works for me for the extension, the new file name is document (2).txt, so extension is saved. Maybe it's a numbering issue.

sh-dv commented 2 years ago

I think the simplest solution is to just add an if statement at the top of the original function that says:

if (filePath.EndsWith(')')) { filePath = filePath.Remove(filePath.Length - 4); }
if (!File.Exists(filePath)) { return filePath; }

This approach should be fine in the vast majority of cases since an extension is normally at the end of file paths, and even if there's no extension, it seems unlikely that the file would end with ')'. To be safe, some additional checks could be added as well. Then the original code works.

I agree, should try it and test it against all cases. never thought about this approach.

samuel-lucas6 commented 2 years ago

I'm afraid this still doesn't seem to work. document.txt (2).kryptor still decrypts to document.txt (2) for me because the file doesn't exist, meaning the Regex part is never reached.

it works for me for the extension, the new file name is document (2).txt, so extension is saved. Maybe it's a numbering issue.

I must have missed implementing one of your commits. Sorry, this could have gone smoother on my part.

They should both fix the issue, but the non-Regex way helps avoid removing things accidentally. From some quick testing, it appears to work without causing any problems. Would you be up for using the additional checks version?

sh-dv commented 2 years ago

I think the simplest solution is to just add an if statement at the top of the original function that says:

if (filePath.EndsWith(')')) { filePath = filePath.Remove(filePath.Length - 4); }
if (!File.Exists(filePath)) { return filePath; }

Edit: Here are some additional checks:

string fileExtension = Path.GetExtension(filePath);
if (!string.IsNullOrEmpty(fileExtension) && filePath.EndsWith(')') && filePath[^4].Equals(' ') && filePath[^3].Equals('(') && char.IsDigit(filePath[^2]))
{
    filePath = filePath.Remove(startIndex: filePath.Length - 4);
}

This solves the issue perfectly and maintains correct original file name and extension.