AlphaFS
AlphaFS copied to clipboard
Allow configuration to exclude Path information from Exceptions
Hi, Thanks for v2.0, I will start to use AlphaFS soon, it looks excellent. I wonder if you include full path info in any exception thrown by AlphaFS like System.IO? I ask this as when in a web environment like ASP.NET, it's not wise to show physical path to users for security purposes. In System.IO, you can not avoid this so for now i need to catch exceptions and sanitize their message before throwing again. Maybe in AlphaFS, this may be at least made configurable with a parameter like displayablePath etc.
Path information is included in exceptions in AlphaFS, just like in System.IO. So you would need to sanitize these as well I'm afraid. No configuration option is available for this at the moment I'm afraid.
Although not frequently, I also have to deal with this. I like this idea though.
Perhaps we can add a member: SanitizeExceptionPath to enum PathFormat as to prevent any additional overloaded methods.
@catester Could you show me an example output, expectations?
@Yomodo Either that, or we could allow configuration through app.config or via a static member in some configuration class. I'm leaning towards the app.config approach, but I might be wrong there... But it seems kind of similar to the includeExceptionDetailsInFaults that is available for WCF configuration (and probably something similar is available for web applications as well).
After all, far from every method that may throw an exception accepts a PathFormat parameter. (Also, it has nothing to do with the PathFormat really.
I also think adding additional overloaded methods would be cumbersome so a static configuration class would be better. If you have static config class, you can always have the option to bind it to an app.config file if you wish. Below is my recommendation with sample usage:
public enum ExceptionPathFormat
{
FullPath, //default
RelativePath,
NoPath
}
public static class AlphaConfig
{
public static ExceptionPathFormat ExceptionPathFormat { get; set; }
}
//Example 1
try
{
AlphaConfig.ExceptionPathFormat = ExceptionPathFormat.RelativePath;
var directoryInfo = new DirectoryInfo(@"c:\folder1\folder2");
directoryInfo.EnumerateFileSystemInfos(WildcardStarMatchAll, SearchOption.AllDirectories);
}
catch (Exception exception)
{
/*
For root folder, message is
"Path is not accessible"
For resursive subfolder "c:\folder1\folder2\folder3\folder4", message is
"Path 'folder3\folder4' is not accessible"
*/
exception.Message;
}
//Example 2
try
{
AlphaConfig.ExceptionPathFormat = ExceptionPathFormat.NoPath;
var directoryInfo = new DirectoryInfo(@"c:\folder1\folder2");
directoryInfo.EnumerateFileSystemInfos(WildcardStarMatchAll, SearchOption.AllDirectories);
}
catch (Exception exception)
{
/*
For all folders, message is
"Path is not accessible"
*/
exception.Message;
}
This static AlphaConfig class may be useful when you need to add other additional global options in the future.
@catester I think it will prove difficult to implement relative vs. full path unfortunately. We usually just pass the path as specified to the method in to the exception. So I think the only real option would be whether to include it or not.
Yes I know, it can become complicated with RelativePath option so even NoPath option would be good. Then the developer can decide to build his own relative path when he needs to add to the exception.
Also using a static AlphaConfig class may not be thread safe. For example thread1 changes an option, then thread2 changes it again and by the time exception in thread1 occurs, the intended first option would not be used. Maybe the developer would accept the risk and use it.
Actually the best option would be to add an additional parameter of type non-static AlphaConfig to the methods. This way you can extend this class in the future without breaking method signatures in the future. Need a new option? Just add it to the AlphaConfig class and no more method overloads.
I see this as an application level setting however, not something that should be changed between method invocations. So I think that the static configuration (or app.config) is still the way to go. Adding overloads to every method accepting some configuration object just seems strange to me.
Ok but also consider ASP.NET applications support, they have web.config and not an app.config like desktop applications. I don't remember now maybe they have a common section though. Also config files are not good for component developers as they usually need to change options at runtime and without depending on an external file.
Regarding overloads, I mean you can group non-standard parameters (ie. parameters not found in System.IO) in a configuration object so you can actually reduce the number of non-standard overloads. I am not sure if this is feasible though, just an idea.
Actually if you decide not to implement this feature, it's fine because it's not a big deal. That's why I closed the issue initially. I can sanitize the messages like this easily:
safeMessage = Regex.Replace(exception.Message, @" ?['""\[][^'""\]]*['""\]]", "");
This covers both messages from Alpha and System.IO:
Path 'c:\folder1\folder2' is not accessible Path "c:\folder1\folder2" is not accessible Path [c:\folder1\folder2] is not accessible
As long as you are consistent with how you enclose the path in the message (square brackets in Alpha, single quotes in System.IO), it should be fine.
@catester Then actually your current solution is the cleanest. I'm sorry for keeping your hopes up.
There is only one method we use to throw exceptions in where the path is always constructed in the same manner.
App.config and Web.config can be accessed through the same classes, so that wouldn't be a problem. But we'll have it in mind. It is not a high-priority issue as of now though.