Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

MailTarget: fix "From" errors (bug introduced in NLog 4.3.2) #1411

Merged
merged 1 commit into from
Apr 28, 2016
Merged

MailTarget: fix "From" errors (bug introduced in NLog 4.3.2) #1411

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 54 additions & 28 deletions src/NLog/Targets/MailTarget.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ public class MailTarget : TargetWithLayoutHeaderAndFooter
{
private const string RequiredPropertyIsEmptyFormat = "After the processing of the MailTarget's '{0}' property it appears to be empty. The email message will not be sent.";

private Layout _from;

/// <summary>
/// Initializes a new instance of the <see cref="MailTarget" /> class.
/// </summary>
Expand All @@ -116,18 +118,31 @@ public MailTarget()
/// Gets the mailSettings/smtp configuration from app.config in cases when we need those configuration.
/// E.g when UseSystemNetMailSettings is enabled and we need to read the From attribute from system.net/mailSettings/smtp
/// </summary>
public SmtpSection MailSettings
/// <remarks>Internal for mocking</remarks>
internal SmtpSection SmtpSection
{
get
{
if (null == _currentailSettings)
if (_currentailSettings == null)
{
_currentailSettings = System.Configuration.ConfigurationManager.OpenExeConfiguration(ConfigurationUserLevel.None).GetSection("system.net/mailSettings/smtp") as SmtpSection;
try
{
_currentailSettings = System.Configuration.ConfigurationManager.GetSection("system.net/mailSettings/smtp") as SmtpSection;
}
catch (Exception ex)
{
InternalLogger.Warn(ex, "reading 'From' from .config failed.");

if (ex.MustBeRethrown())
{
throw;
}
_currentailSettings = new SmtpSection();
}
}

return _currentailSettings;
}

set { _currentailSettings = value; }
}
#endif
Expand All @@ -136,8 +151,30 @@ public SmtpSection MailSettings
/// Gets or sets sender's email address (e.g. joe@domain.com).
/// </summary>
/// <docgen category='Message Options' order='10' />
[RequiredParameter]
public Layout From { get; set; }
public Layout From
{
get
{
#if !__ANDROID__ && !__IOS__
if (UseSystemNetMailSettings)
{
// In contrary to other settings, System.Net.Mail.SmtpClient doesn't read the 'From' attribute from the system.net/mailSettings/smtp section in the config file.
// Thus, when UseSystemNetMailSettings is enabled we have to read the configuration section of system.net/mailSettings/smtp to initialize the 'From' address.
// It will do so only if the 'From' attribute in system.net/mailSettings/smtp is not empty.

//only use from config when not set in current
if (_from == null)
{
var from = SmtpSection.From;
if (from == null) return null;
return from;
}
}
#endif
return _from;
}
set { _from = value; }
}

/// <summary>
/// Gets or sets recipients' email addresses separated by semicolons (e.g. john@domain.com;jane@domain.com).
Expand Down Expand Up @@ -317,20 +354,7 @@ protected override void InitializeTarget()
base.InitializeTarget();
}

/// <summary>
/// In contrary to other settings, System.Net.Mail.SmtpClient doesn't read the 'From' attribute from the system.net/mailSettings/smtp section in the config file.
/// Thus, when UseSystemNetMailSettings is enabled we have to read the configuration section of system.net/mailSettings/smtp to initialize the 'From' address.
/// It will do so only if the 'From' attribute in system.net/mailSettings/smtp is not empty.
/// </summary>
private void ConfigureFrom()
{
#if !__ANDROID__ && !__IOS__
if (!string.IsNullOrEmpty(MailSettings.From))
{
From = MailSettings.From;
}
#endif
}


/// <summary>
/// Create mail and send with SMTP
Expand Down Expand Up @@ -360,10 +384,6 @@ private void ProcessSingleMailMessage([NotNull] List<AsyncLogEventInfo> events)
{
ConfigureMailClient(lastEvent, client);
}
else
{
ConfigureFrom();
}

InternalLogger.Debug("Sending mail to {0} using {1}:{2} (ssl={3})", msg.To, client.Host, client.Port, client.EnableSsl);
InternalLogger.Trace(" Subject: '{0}'", msg.Subject);
Expand Down Expand Up @@ -440,6 +460,7 @@ private StringBuilder CreateBodyBuffer(IEnumerable<AsyncLogEventInfo> events, Lo
/// </summary>
/// <param name="lastEvent">last event for username/password</param>
/// <param name="client">client to set properties on</param>
/// <remarks>Configure not at <see cref="InitializeTarget"/>, as the properties could have layout renderers.</remarks>
internal void ConfigureMailClient(LogEventInfo lastEvent, ISmtpClient client)
{
CheckRequiredParameters();
Expand Down Expand Up @@ -523,14 +544,17 @@ private void CheckRequiredParameters()
{
if (!this.UseSystemNetMailSettings && this.SmtpServer == null && this.DeliveryMethod == SmtpDeliveryMethod.Network)
{
throw new NLogConfigurationException(
string.Format("The MailTarget's '{0}' properties are not set - but needed because useSystemNetMailSettings=false and DeliveryMethod=Network. The email message will not be sent.", "SmtpServer"));
throw new NLogConfigurationException("The MailTarget's '{0}' properties are not set - but needed because useSystemNetMailSettings=false and DeliveryMethod=Network. The email message will not be sent.", "SmtpServer");
}

if (!this.UseSystemNetMailSettings && string.IsNullOrEmpty(this.PickupDirectoryLocation) && this.DeliveryMethod == SmtpDeliveryMethod.SpecifiedPickupDirectory)
{
throw new NLogConfigurationException(
string.Format("The MailTarget's '{0}' properties are not set - but needed because useSystemNetMailSettings=false and DeliveryMethod=SpecifiedPickupDirectory. The email message will not be sent.", "PickupDirectoryLocation"));
throw new NLogConfigurationException("The MailTarget's '{0}' properties are not set - but needed because useSystemNetMailSettings=false and DeliveryMethod=SpecifiedPickupDirectory. The email message will not be sent.", "PickupDirectoryLocation");
}

if (this.From == null)
{
throw new NLogConfigurationException(RequiredPropertyIsEmptyFormat, "From");
}
}

Expand Down Expand Up @@ -568,6 +592,8 @@ private static void AppendLayout(StringBuilder sb, LogEventInfo logEvent, Layout
sb.Append(layout.Render(logEvent));
}



/// <summary>
/// Create the mailmessage with the addresses, properties and body.
/// </summary>
Expand Down
100 changes: 40 additions & 60 deletions tests/NLog.UnitTests/Targets/MailTargetTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
// THE POSSIBILITY OF SUCH DAMAGE.
//


#if !SILVERLIGHT

namespace NLog.UnitTests.Targets
Expand All @@ -47,10 +46,16 @@ namespace NLog.UnitTests.Targets
using System.IO;
#if !__ANDROID__ && !__IOS__
using System.Configuration;
using System.Net.Configuration;
#endif

public class MailTargetTests : NLogTestBase
{
public MailTargetTests()
{
LogManager.ThrowExceptions = true;
}

[Fact]
public void SimpleEmailTest()
{
Expand Down Expand Up @@ -270,6 +275,7 @@ public void PerMessageServer()
[Fact]
public void ErrorHandlingTest()
{
LogManager.ThrowExceptions = false;
var mmt = new MockMailTarget
{
From = "foo@bar.com",
Expand Down Expand Up @@ -441,7 +447,7 @@ public void NoReplaceNewlinesWithBreakInHtmlMail()

var messageSent = mmt.CreatedMocks[0].MessagesSent[0];
Assert.True(messageSent.IsBodyHtml);
var lines = messageSent.Body.Split(new[] {Environment.NewLine }, StringSplitOptions.RemoveEmptyEntries);
var lines = messageSent.Body.Split(new[] { Environment.NewLine }, StringSplitOptions.RemoveEmptyEntries);
Assert.True(lines.Length == 3);
}

Expand Down Expand Up @@ -541,8 +547,6 @@ public void MailTarget_WithValidToAndEmptyBcc_SendsMail()

var exceptions = new List<Exception>();
mmt.WriteAsyncLogEvent(new LogEventInfo(LogLevel.Info, "MyLogger", "log message 1").WithContinuation(exceptions.Add));

Assert.Null(exceptions[0]);
Assert.Equal(1, mmt.CreatedMocks.Count);
Assert.Equal(1, mmt.CreatedMocks[0].MessagesSent.Count);
}
Expand All @@ -560,12 +564,9 @@ public void MailTarget_WithEmptyTo_ThrowsNLogRuntimeException()
Body = "${level} ${logger} ${message}",
};
mmt.Initialize(null);

var exceptions = new List<Exception>();
mmt.WriteAsyncLogEvent(new LogEventInfo(LogLevel.Info, "MyLogger", "log message 1").WithContinuation(exceptions.Add));
Assert.Throws<NLogRuntimeException>(() => mmt.WriteAsyncLogEvent(new LogEventInfo(LogLevel.Info, "MyLogger", "log message 1").WithContinuation(exceptions.Add)));

Assert.NotNull(exceptions[0]);
Assert.IsType<NLogRuntimeException>(exceptions[0]);
}

[Fact]
Expand All @@ -583,10 +584,7 @@ public void MailTarget_WithEmptyFrom_ThrowsNLogRuntimeException()
mmt.Initialize(null);

var exceptions = new List<Exception>();
mmt.WriteAsyncLogEvent(new LogEventInfo(LogLevel.Info, "MyLogger", "log message 1").WithContinuation(exceptions.Add));

Assert.NotNull(exceptions[0]);
Assert.IsType<NLogRuntimeException>(exceptions[0]);
Assert.Throws<NLogRuntimeException>(() => mmt.WriteAsyncLogEvent(new LogEventInfo(LogLevel.Info, "MyLogger", "log message 1").WithContinuation(exceptions.Add)));
}

[Fact]
Expand All @@ -604,10 +602,7 @@ public void MailTarget_WithEmptySmtpServer_ThrowsNLogRuntimeException()
mmt.Initialize(null);

var exceptions = new List<Exception>();
mmt.WriteAsyncLogEvent(new LogEventInfo(LogLevel.Info, "MyLogger", "log message 1").WithContinuation(exceptions.Add));

Assert.NotNull(exceptions[0]);
Assert.IsType<NLogRuntimeException>(exceptions[0]);
Assert.Throws<NLogRuntimeException>(() => mmt.WriteAsyncLogEvent(new LogEventInfo(LogLevel.Info, "MyLogger", "log message 1").WithContinuation(exceptions.Add)));
}

[Fact]
Expand All @@ -633,7 +628,8 @@ public void MailTargetInitialize_WithoutSpecifiedFrom_ThrowsConfigException()
Subject = "Hello from NLog",
SmtpServer = "server1",
SmtpPort = 27,
Body = "${level} ${logger} ${message}"
Body = "${level} ${logger} ${message}",
UseSystemNetMailSettings = false
};
Assert.Throws<NLogConfigurationException>(() => mmt.Initialize(null));
}
Expand All @@ -650,7 +646,7 @@ public void MailTargetInitialize_WithoutSpecifiedSmtpServer_should_not_ThrowsCon
Body = "${level} ${logger} ${message}",
UseSystemNetMailSettings = true
};

mmt.Initialize(null);
}

[Fact]
Expand Down Expand Up @@ -729,7 +725,7 @@ public void MailTarget_UseSystemNetMailSettings_True()

Assert.Equal(mmt.SmtpClientPickUpDirectory, inConfigVal);
}

[Fact]
public void MailTarget_UseSystemNetMailSettings_True_WithVirtualPath()
{
Expand All @@ -745,7 +741,7 @@ public void MailTarget_UseSystemNetMailSettings_True_WithVirtualPath()
DeliveryMethod = SmtpDeliveryMethod.SpecifiedPickupDirectory
};
mmt.ConfigureMailClient();

Assert.NotEqual(inConfigVal, mmt.SmtpClientPickUpDirectory);
var separator = Path.DirectorySeparatorChar;
Assert.Contains(string.Format("{0}App_Data{0}Mail", separator), mmt.SmtpClientPickUpDirectory);
Expand All @@ -754,84 +750,67 @@ public void MailTarget_UseSystemNetMailSettings_True_WithVirtualPath()
#if !__ANDROID__ && !__IOS__

[Fact]
public void MailTarget_UseSystemNetMailSettings_True_ReadFromFromConfigFile()
public void MailTarget_UseSystemNetMailSettings_True_ReadFromFromConfigFile_dontoverride()
{
var configuration = System.Configuration.ConfigurationManager.OpenExeConfiguration(ConfigurationUserLevel.None);
var section = configuration.GetSection("system.net/mailSettings/smtp") as System.Net.Configuration.SmtpSection;
section.From = "foo@bar.com";

var mmt = new MockMailTarget()
var mmt = new MailTarget()
{
From = "foo@foo.com",
From = "nlog@foo.com",
To = "bar@bar.com",
SmtpServer = "server1",
SmtpPort = 27,
Body = "${level} ${logger} ${message}",
UseSystemNetMailSettings = true,
MailSettings = section
SmtpSection = new SmtpSection { From = "config@foo.com" }
};
mmt.Initialize(null);
mmt.ConfigureMailClient();
Assert.Equal("'nlog@foo.com'", mmt.From.ToString());

var exceptions = new List<Exception>();
mmt.WriteAsyncLogEvent(new LogEventInfo(LogLevel.Debug, "MyLogger", "log message").WithContinuation(exceptions.Add));
mmt.Initialize(null);

Assert.Equal("'foo@bar.com'", mmt.From.ToString());
Assert.Equal("'nlog@foo.com'", mmt.From.ToString());
}

[Fact]
public void MailTarget_UseSystemNetMailSettings_False_ReadFromFromConfigFile()
public void MailTarget_UseSystemNetMailSettings_True_ReadFromFromConfigFile()
{
var configuration = System.Configuration.ConfigurationManager.OpenExeConfiguration(ConfigurationUserLevel.None);
var section = configuration.GetSection("system.net/mailSettings/smtp") as System.Net.Configuration.SmtpSection;
section.From = "foo@bar.com";

var mmt = new MockMailTarget()
var mmt = new MailTarget()
{
From = "foo@foo.com",
From = null,
To = "bar@bar.com",
SmtpServer = "server1",
SmtpPort = 27,
Body = "${level} ${logger} ${message}",
UseSystemNetMailSettings = false,
MailSettings = section
UseSystemNetMailSettings = true,
SmtpSection = new SmtpSection { From = "config@foo.com" }
};
mmt.Initialize(null);
mmt.ConfigureMailClient();
Assert.Equal("'config@foo.com'", mmt.From.ToString());

var exceptions = new List<Exception>();
mmt.WriteAsyncLogEvent(new LogEventInfo(LogLevel.Debug, "MyLogger", "log message").WithContinuation(exceptions.Add));
mmt.Initialize(null);

Assert.Equal("'foo@foo.com'", mmt.From.ToString());
Assert.Equal("'config@foo.com'", mmt.From.ToString());
}

[Fact]
public void MailTarget_UseSystemNetMailSettings_True_KeepOverridenFiles()
public void MailTarget_UseSystemNetMailSettings_False_ReadFromFromConfigFile()
{
var configuration = System.Configuration.ConfigurationManager.OpenExeConfiguration(ConfigurationUserLevel.None);
var section = configuration.GetSection("system.net/mailSettings/smtp") as System.Net.Configuration.SmtpSection;
section.From = null;

var mmt = new MockMailTarget()
var mmt = new MailTarget()
{
From = "foo@foo.com",
From = null,
To = "bar@bar.com",
SmtpServer = "server1",
SmtpPort = 27,
Body = "${level} ${logger} ${message}",
UseSystemNetMailSettings = true,
MailSettings = section
UseSystemNetMailSettings = false,
SmtpSection = new SmtpSection { From = "config@foo.com" }
};
Assert.Null(mmt.From);

mmt.Initialize(null);
mmt.ConfigureMailClient();

var exceptions = new List<Exception>();
mmt.WriteAsyncLogEvent(new LogEventInfo(LogLevel.Debug, "MyLogger", "log message").WithContinuation(exceptions.Add));
Assert.Throws <NLogConfigurationException>(() => mmt.Initialize(null));

Assert.Equal("'foo@foo.com'", mmt.From.ToString());
}

#endif

[Fact]
Expand Down Expand Up @@ -925,6 +904,7 @@ internal override ISmtpClient CreateSmtpClient()
return client;
}


public void ConfigureMailClient()
{
if (UseSystemNetMailSettings) return;
Expand Down