-
Notifications
You must be signed in to change notification settings - Fork 6
Description
While working on PR #64, I was thinking about removing the sanity checks for paths and urls that we are using to filter backup files out of the chain.
I believe the syntax checks for urls/paths are already done implicitly by the underlying SMO functions when it tries to get the file.
Moreover, I feel it would be better to error out when there are bad paths/urls rather than to silently filter them out of the backup chain.
However, before we discuss removing these sanity checks on our side, we need to make sure that if the SMO functions error out because of bad paths we can recover gracefully - this will require some more investigation into how the paths/urls are used and the error recovery process in place. (which might prove that this is too much work for little benefit)
However, still filing this issue to discuss this further and as a reminder for the investigation (to be placed into the backlog for now).
The sanity checks:
AgDatabaseMove/src/BackupChain.cs
Lines 99 to 117 in babf28a
| private static bool IsValidFilePath(BackupMetadata meta) | |
| { | |
| if(BackupFileTools.IsUrl(meta.PhysicalDeviceName)) | |
| return true; | |
| // A quick check before leaning on exceptions | |
| if(Path.GetInvalidPathChars().Any(meta.PhysicalDeviceName.Contains)) | |
| return false; | |
| try { | |
| // This will throw an argument exception if the path is invalid | |
| Path.GetFullPath(meta.PhysicalDeviceName); | |
| // A relative path won't help us much if the destination is another server. It needs to be rooted. | |
| return Path.IsPathRooted(meta.PhysicalDeviceName); | |
| } | |
| catch(Exception) { | |
| return false; | |
| } | |
| } |