From 64f8e1b13f253cddc7b84b1186f04a0ab0dbb7eb Mon Sep 17 00:00:00 2001 From: AraHaan Date: Tue, 28 Jun 2022 23:16:51 -0400 Subject: [PATCH 1/4] Use Path.GetFullPath() to resolve the full path of the input path. Note: I also changed a few string == null checks to call string.IsNullOrEmpty() so that empty strings also throw (as I think those are error cases as well). Fixes https://github.com/serilog/serilog-sinks-file/issues/257. --- .../FileLoggerConfigurationExtensions.cs | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/Serilog.Sinks.File/FileLoggerConfigurationExtensions.cs b/src/Serilog.Sinks.File/FileLoggerConfigurationExtensions.cs index 3518322..59ff13a 100644 --- a/src/Serilog.Sinks.File/FileLoggerConfigurationExtensions.cs +++ b/src/Serilog.Sinks.File/FileLoggerConfigurationExtensions.cs @@ -267,8 +267,10 @@ public static LoggerConfiguration File( TimeSpan? retainedFileTimeLimit = null) { if (sinkConfiguration == null) throw new ArgumentNullException(nameof(sinkConfiguration)); - if (path == null) throw new ArgumentNullException(nameof(path)); - if (outputTemplate == null) throw new ArgumentNullException(nameof(outputTemplate)); + + // check if null or empty on these. + if (string.IsNullOrEmpty(path)) throw new ArgumentNullException(nameof(path)); + if (string.IsNullOrEmpty(outputTemplate)) throw new ArgumentNullException(nameof(outputTemplate)); var formatter = new MessageTemplateTextFormatter(outputTemplate, formatProvider); return File(sinkConfiguration, formatter, path, restrictedToMinimumLevel, fileSizeLimitBytes, @@ -337,7 +339,7 @@ public static LoggerConfiguration File( { if (sinkConfiguration == null) throw new ArgumentNullException(nameof(sinkConfiguration)); if (formatter == null) throw new ArgumentNullException(nameof(formatter)); - if (path == null) throw new ArgumentNullException(nameof(path)); + if (string.IsNullOrEmpty(path)) throw new ArgumentNullException(nameof(path)); return ConfigureFile(sinkConfiguration.Sink, formatter, path, restrictedToMinimumLevel, fileSizeLimitBytes, levelSwitch, buffered, false, shared, flushToDiskInterval, encoding, rollingInterval, rollOnFileSizeLimit, @@ -449,8 +451,8 @@ public static LoggerConfiguration File( FileLifecycleHooks? hooks = null) { if (sinkConfiguration == null) throw new ArgumentNullException(nameof(sinkConfiguration)); - if (path == null) throw new ArgumentNullException(nameof(path)); - if (outputTemplate == null) throw new ArgumentNullException(nameof(outputTemplate)); + if (string.IsNullOrEmpty(path)) throw new ArgumentNullException(nameof(path)); + if (string.IsNullOrEmpty(outputTemplate)) throw new ArgumentNullException(nameof(outputTemplate)); var formatter = new MessageTemplateTextFormatter(outputTemplate, formatProvider); return File(sinkConfiguration, formatter, path, restrictedToMinimumLevel, levelSwitch, encoding, hooks); @@ -493,7 +495,7 @@ public static LoggerConfiguration File( { if (sinkConfiguration == null) throw new ArgumentNullException(nameof(sinkConfiguration)); if (formatter == null) throw new ArgumentNullException(nameof(formatter)); - if (path == null) throw new ArgumentNullException(nameof(path)); + if (string.IsNullOrEmpty(path)) throw new ArgumentNullException(nameof(path)); return ConfigureFile(sinkConfiguration.Sink, formatter, path, restrictedToMinimumLevel, null, levelSwitch, false, true, false, null, encoding, RollingInterval.Infinite, false, null, hooks, null); @@ -519,39 +521,41 @@ static LoggerConfiguration ConfigureFile( { if (addSink == null) throw new ArgumentNullException(nameof(addSink)); if (formatter == null) throw new ArgumentNullException(nameof(formatter)); - if (path == null) throw new ArgumentNullException(nameof(path)); + if (string.IsNullOrEmpty(path)) throw new ArgumentNullException(nameof(path)); if (fileSizeLimitBytes.HasValue && fileSizeLimitBytes < 1) throw new ArgumentException("Invalid value provided; file size limit must be at least 1 byte, or null.", nameof(fileSizeLimitBytes)); if (retainedFileCountLimit.HasValue && retainedFileCountLimit < 1) throw new ArgumentException("At least one file must be retained.", nameof(retainedFileCountLimit)); if (retainedFileTimeLimit.HasValue && retainedFileTimeLimit < TimeSpan.Zero) throw new ArgumentException("Negative value provided; retained file time limit must be non-negative.", nameof(retainedFileTimeLimit)); if (shared && buffered) throw new ArgumentException("Buffered writes are not available when file sharing is enabled.", nameof(buffered)); if (shared && hooks != null) throw new ArgumentException("File lifecycle hooks are not currently supported for shared log files.", nameof(hooks)); + // time to resolve path to it's full path if full path not provided. + var fullPath = Path.GetFullPath(path); ILogEventSink sink; try { if (rollOnFileSizeLimit || rollingInterval != RollingInterval.Infinite) { - sink = new RollingFileSink(path, formatter, fileSizeLimitBytes, retainedFileCountLimit, encoding, buffered, shared, rollingInterval, rollOnFileSizeLimit, hooks, retainedFileTimeLimit); + sink = new RollingFileSink(fullPath, formatter, fileSizeLimitBytes, retainedFileCountLimit, encoding, buffered, shared, rollingInterval, rollOnFileSizeLimit, hooks, retainedFileTimeLimit); } else { if (shared) { #pragma warning disable 618 - sink = new SharedFileSink(path, formatter, fileSizeLimitBytes, encoding); + sink = new SharedFileSink(fullPath, formatter, fileSizeLimitBytes, encoding); #pragma warning restore 618 } else { - sink = new FileSink(path, formatter, fileSizeLimitBytes, encoding, buffered, hooks); + sink = new FileSink(fullPath, formatter, fileSizeLimitBytes, encoding, buffered, hooks); } } } catch (Exception ex) { - SelfLog.WriteLine("Unable to open file sink for {0}: {1}", path, ex); + SelfLog.WriteLine("Unable to open file sink for {0}: {1}", fullPath, ex); if (propagateExceptions) throw; From fc99f80db6233c410f7d423889b9ef225c7475bd Mon Sep 17 00:00:00 2001 From: AraHaan Date: Tue, 28 Jun 2022 23:36:42 -0400 Subject: [PATCH 2/4] Apply suggestions from code review Looks like the only places where the issue is is FileSink and SharedFileSink. --- .../FileLoggerConfigurationExtensions.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/Serilog.Sinks.File/FileLoggerConfigurationExtensions.cs b/src/Serilog.Sinks.File/FileLoggerConfigurationExtensions.cs index 59ff13a..bcd76a1 100644 --- a/src/Serilog.Sinks.File/FileLoggerConfigurationExtensions.cs +++ b/src/Serilog.Sinks.File/FileLoggerConfigurationExtensions.cs @@ -528,34 +528,32 @@ static LoggerConfiguration ConfigureFile( if (shared && buffered) throw new ArgumentException("Buffered writes are not available when file sharing is enabled.", nameof(buffered)); if (shared && hooks != null) throw new ArgumentException("File lifecycle hooks are not currently supported for shared log files.", nameof(hooks)); - // time to resolve path to it's full path if full path not provided. - var fullPath = Path.GetFullPath(path); ILogEventSink sink; try { if (rollOnFileSizeLimit || rollingInterval != RollingInterval.Infinite) { - sink = new RollingFileSink(fullPath, formatter, fileSizeLimitBytes, retainedFileCountLimit, encoding, buffered, shared, rollingInterval, rollOnFileSizeLimit, hooks, retainedFileTimeLimit); + sink = new RollingFileSink(path, formatter, fileSizeLimitBytes, retainedFileCountLimit, encoding, buffered, shared, rollingInterval, rollOnFileSizeLimit, hooks, retainedFileTimeLimit); } else { if (shared) { #pragma warning disable 618 - sink = new SharedFileSink(fullPath, formatter, fileSizeLimitBytes, encoding); + sink = new SharedFileSink(path, formatter, fileSizeLimitBytes, encoding); #pragma warning restore 618 } else { - sink = new FileSink(fullPath, formatter, fileSizeLimitBytes, encoding, buffered, hooks); + sink = new FileSink(path, formatter, fileSizeLimitBytes, encoding, buffered, hooks); } } } catch (Exception ex) { - SelfLog.WriteLine("Unable to open file sink for {0}: {1}", fullPath, ex); + SelfLog.WriteLine("Unable to open file sink for {0}: {1}", path, ex); if (propagateExceptions) throw; From 83f49706e86ba1fd3a58e7bcf92fd9eb229181cb Mon Sep 17 00:00:00 2001 From: AraHaan Date: Thu, 30 Jun 2022 00:32:57 -0400 Subject: [PATCH 3/4] Use ArgumentException instead of ArgumentNullException with IsNullOrEmpty. --- .../FileLoggerConfigurationExtensions.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Serilog.Sinks.File/FileLoggerConfigurationExtensions.cs b/src/Serilog.Sinks.File/FileLoggerConfigurationExtensions.cs index bcd76a1..8308f19 100644 --- a/src/Serilog.Sinks.File/FileLoggerConfigurationExtensions.cs +++ b/src/Serilog.Sinks.File/FileLoggerConfigurationExtensions.cs @@ -269,8 +269,8 @@ public static LoggerConfiguration File( if (sinkConfiguration == null) throw new ArgumentNullException(nameof(sinkConfiguration)); // check if null or empty on these. - if (string.IsNullOrEmpty(path)) throw new ArgumentNullException(nameof(path)); - if (string.IsNullOrEmpty(outputTemplate)) throw new ArgumentNullException(nameof(outputTemplate)); + if (string.IsNullOrEmpty(path)) throw new ArgumentException(nameof(path)); + if (string.IsNullOrEmpty(outputTemplate)) throw new ArgumentException(nameof(outputTemplate)); var formatter = new MessageTemplateTextFormatter(outputTemplate, formatProvider); return File(sinkConfiguration, formatter, path, restrictedToMinimumLevel, fileSizeLimitBytes, @@ -339,7 +339,7 @@ public static LoggerConfiguration File( { if (sinkConfiguration == null) throw new ArgumentNullException(nameof(sinkConfiguration)); if (formatter == null) throw new ArgumentNullException(nameof(formatter)); - if (string.IsNullOrEmpty(path)) throw new ArgumentNullException(nameof(path)); + if (string.IsNullOrEmpty(path)) throw new ArgumentException(nameof(path)); return ConfigureFile(sinkConfiguration.Sink, formatter, path, restrictedToMinimumLevel, fileSizeLimitBytes, levelSwitch, buffered, false, shared, flushToDiskInterval, encoding, rollingInterval, rollOnFileSizeLimit, @@ -451,8 +451,8 @@ public static LoggerConfiguration File( FileLifecycleHooks? hooks = null) { if (sinkConfiguration == null) throw new ArgumentNullException(nameof(sinkConfiguration)); - if (string.IsNullOrEmpty(path)) throw new ArgumentNullException(nameof(path)); - if (string.IsNullOrEmpty(outputTemplate)) throw new ArgumentNullException(nameof(outputTemplate)); + if (string.IsNullOrEmpty(path)) throw new ArgumentException(nameof(path)); + if (string.IsNullOrEmpty(outputTemplate)) throw new ArgumentException(nameof(outputTemplate)); var formatter = new MessageTemplateTextFormatter(outputTemplate, formatProvider); return File(sinkConfiguration, formatter, path, restrictedToMinimumLevel, levelSwitch, encoding, hooks); @@ -495,7 +495,7 @@ public static LoggerConfiguration File( { if (sinkConfiguration == null) throw new ArgumentNullException(nameof(sinkConfiguration)); if (formatter == null) throw new ArgumentNullException(nameof(formatter)); - if (string.IsNullOrEmpty(path)) throw new ArgumentNullException(nameof(path)); + if (string.IsNullOrEmpty(path)) throw new ArgumentException(nameof(path)); return ConfigureFile(sinkConfiguration.Sink, formatter, path, restrictedToMinimumLevel, null, levelSwitch, false, true, false, null, encoding, RollingInterval.Infinite, false, null, hooks, null); @@ -521,7 +521,7 @@ static LoggerConfiguration ConfigureFile( { if (addSink == null) throw new ArgumentNullException(nameof(addSink)); if (formatter == null) throw new ArgumentNullException(nameof(formatter)); - if (string.IsNullOrEmpty(path)) throw new ArgumentNullException(nameof(path)); + if (string.IsNullOrEmpty(path)) throw new ArgumentException(nameof(path)); if (fileSizeLimitBytes.HasValue && fileSizeLimitBytes < 1) throw new ArgumentException("Invalid value provided; file size limit must be at least 1 byte, or null.", nameof(fileSizeLimitBytes)); if (retainedFileCountLimit.HasValue && retainedFileCountLimit < 1) throw new ArgumentException("At least one file must be retained.", nameof(retainedFileCountLimit)); if (retainedFileTimeLimit.HasValue && retainedFileTimeLimit < TimeSpan.Zero) throw new ArgumentException("Negative value provided; retained file time limit must be non-negative.", nameof(retainedFileTimeLimit)); From 19e0df1e86c44ea853bf0a3f9fc00892d6431c13 Mon Sep 17 00:00:00 2001 From: AraHaan Date: Wed, 25 Oct 2023 06:20:06 -0400 Subject: [PATCH 4/4] Apply suggestions from code review --- src/Serilog.Sinks.File/FileLoggerConfigurationExtensions.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Serilog.Sinks.File/FileLoggerConfigurationExtensions.cs b/src/Serilog.Sinks.File/FileLoggerConfigurationExtensions.cs index 8308f19..5af900c 100644 --- a/src/Serilog.Sinks.File/FileLoggerConfigurationExtensions.cs +++ b/src/Serilog.Sinks.File/FileLoggerConfigurationExtensions.cs @@ -267,10 +267,8 @@ public static LoggerConfiguration File( TimeSpan? retainedFileTimeLimit = null) { if (sinkConfiguration == null) throw new ArgumentNullException(nameof(sinkConfiguration)); - - // check if null or empty on these. if (string.IsNullOrEmpty(path)) throw new ArgumentException(nameof(path)); - if (string.IsNullOrEmpty(outputTemplate)) throw new ArgumentException(nameof(outputTemplate)); + if (outputTemplate == null) throw new ArgumentNullException(nameof(outputTemplate)); var formatter = new MessageTemplateTextFormatter(outputTemplate, formatProvider); return File(sinkConfiguration, formatter, path, restrictedToMinimumLevel, fileSizeLimitBytes, @@ -452,7 +450,7 @@ public static LoggerConfiguration File( { if (sinkConfiguration == null) throw new ArgumentNullException(nameof(sinkConfiguration)); if (string.IsNullOrEmpty(path)) throw new ArgumentException(nameof(path)); - if (string.IsNullOrEmpty(outputTemplate)) throw new ArgumentException(nameof(outputTemplate)); + if (outputTemplate == null) throw new ArgumentNullException(nameof(outputTemplate)); var formatter = new MessageTemplateTextFormatter(outputTemplate, formatProvider); return File(sinkConfiguration, formatter, path, restrictedToMinimumLevel, levelSwitch, encoding, hooks);