stefankueng / CryptSync

CryptSync is a small utility that synchronizes two folders while encrypting the contents in one folder. That means one of the two folders has all files unencrypted (the files you work with) and the other folder has all the files encrypted.
https://tools.stefankueng.com/CryptSync.html
GNU General Public License v3.0
387 stars 72 forks source link

Fixes for deleting file to trash and long pathname in pairs #126

Closed dansyl1 closed 1 month ago

dansyl1 commented 1 month ago

Fixed various issues in DeletePathToTrash():

Fixed long pathname support before calling EncyptFile(). Could also be done in the function itself.

Adjustments to logging (error text instead of error code).

stefankueng commented 1 month ago

do you think a new release is due or can/should I wait a few days first?

dansyl1 commented 1 month ago

Apologies for not responding and thank you for the new release.

I have been using your program successfully for years. As a retirement project I decided to try to understand how it work.

I identified a potential issue with regards to ignoring notification changes triggered by cryptsync.

MaxPath vs not-MaxPath

  1. PathWatcher uses paths as entered by user (no CPathUtils::AdjustForMaxPath() done on them); example: m_watchedPaths and GetChangedPaths()
  2. FolderSync uses a mixture of paths as entered by user and "adjusted paths" (ie, adjusted by CPathUtils::AdjustForMaxPath()). m_notifyIgnores.insert is one such case, called with both types of paths. GetNotifyIgnores() returns the set to TrayWindow
  3. TrayWindow removes the notifications that should be ignored but fails due to adjustments on some paths Options on how to resolve - I seek your feedback on this, there may be other options.

Option 1

Option 2 Seems complicated, and not sure it would help if a wstring is forgotten.... Create a new (CPathUtils?) CAdjustedPath class based on std::wstring which can only holds paths processed by CPathUtils::AdjustForMaxPath() and replace std::wstring with CAdjustedPath as appropriate where path variables are declared . g_pairs would continue to use wstring. std::wstring origPath = pair.m_origPath would become CAdjustedPath origPath = pair.m_origPath operator= will call CPathUtils::AdjustForMaxPath() transparently.

Please confirm how / if you'd like me to adjusts the code to resolve above issue.

Two more questions for you (in bold), code is in FolderSync.cpp

Around line 900 if (!CopyFile(cryptPath.c_str(), origPath.c_str(), FALSE)) { std::wstring targetFolder = pt.m_origPath + L"\" + it->first; targetFolder = targetFolder.substr(0, targetFolder.find_last_of('\')); CPathUtils::CreateRecursiveDirectory(targetFolder); { CAutoWriteLock nLocker(m_notingGuard); m_notifyIgnores.insert(CPathUtils::Append(pt.m_origPath, it->first)); } if (!CopyFile(cryptPath.c_str(), origPath.c_str(), FALSE)) retVal |= ErrorCopy; }

===> Why is the m_notifyIgnores.insert only done after the second CopyFile

Around line 1270: bool bRet = RunGPG(cmdlineBuf.get(), targetFolder); if (!bRet) { CPathUtils::CreateRecursiveDirectory(targetFolder); { CAutoWriteLock nLocker(m_notingGuard); m_notifyIgnores.insert(orig); } bRet = RunGPG(cmdlineBuf.get(), targetFolder);

===> Why is the m_notifyIgnores.insert only done after the second RunGPG

Thanks in advance

From: Stefan Küng @.> Sent: Tuesday, July 23, 2024 2:09 PM To: stefankueng/CryptSync @.> Cc: dansyl1 @.>; Author @.> Subject: Re: [stefankueng/CryptSync] Fixes for deleting file to trash and long pathname in pairs (PR #126)

do you think a new release is due or can/should I wait a few days first?

- Reply to this email directly, view it on GitHubhttps://github.com/stefankueng/CryptSync/pull/126#issuecomment-2245922789, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AQDFBNOSYIT5WK2GZNZVLRDZN2L3DAVCNFSM6AAAAABLJVUABCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENBVHEZDENZYHE. You are receiving this because you authored the thread.Message ID: @.**@.>>

stefankueng commented 1 month ago

I think option 1 is the way to go here.

...===> Why is the m_notifyIgnores.insert only done after the second CopyFile that's definitely a bug. Same with the second one.