Skip to content

Commit

Permalink
Merge pull request #1553 from NLog/pr-1517
Browse files Browse the repository at this point in the history
Bugfix: Throw configException when registering invalid extension assembly/type.
  • Loading branch information
304NotModified committed Jul 23, 2016
2 parents 698ca65 + 0750c5e commit ee98bf2
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 27 deletions.
50 changes: 36 additions & 14 deletions src/NLog/Config/XmlLoggingConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ private void Initialize(XmlReader reader, string fileName, bool ignoreErrors)

if (!ignoreErrors)
{
if (exception.MustBeRethrown())
if (configurationException.MustBeRethrown())
{
throw configurationException;
}
Expand Down Expand Up @@ -909,7 +909,26 @@ private void ParseExtensionsElement(NLogXmlElement extensionsElement, string bas
string type = StripOptionalNamespacePrefix(addElement.GetOptionalAttribute("type", null));
if (type != null)
{
this.ConfigurationItemFactory.RegisterType(Type.GetType(type, true), prefix);
try
{
this.ConfigurationItemFactory.RegisterType(Type.GetType(type, true), prefix);
}
catch (Exception exception)
{
if (exception.MustBeRethrownImmediately())
{
throw;
}

InternalLogger.Error(exception, "Error loading extensions.");
NLogConfigurationException configException =
new NLogConfigurationException("Error loading extensions: " + type, exception);

if (configException.MustBeRethrown())
{
throw configException;
}
}
}

string assemblyFile = addElement.GetOptionalAttribute("assemblyFile", null);
Expand All @@ -918,17 +937,17 @@ private void ParseExtensionsElement(NLogXmlElement extensionsElement, string bas
try
{
#if SILVERLIGHT && !WINDOWS_PHONE
var si = Application.GetResourceStream(new Uri(assemblyFile, UriKind.Relative));
var assemblyPart = new AssemblyPart();
Assembly asm = assemblyPart.Load(si.Stream);
var si = Application.GetResourceStream(new Uri(assemblyFile, UriKind.Relative));
var assemblyPart = new AssemblyPart();
Assembly asm = assemblyPart.Load(si.Stream);
#else

string fullFileName = Path.Combine(baseDirectory, assemblyFile);
InternalLogger.Info("Loading assembly file: {0}", fullFileName);

Assembly asm = Assembly.LoadFrom(fullFileName);
#endif
this.ConfigurationItemFactory.RegisterItemsFromAssembly(asm, prefix);

}
catch (Exception exception)
{
Expand All @@ -938,10 +957,12 @@ private void ParseExtensionsElement(NLogXmlElement extensionsElement, string bas
}

InternalLogger.Error(exception, "Error loading extensions.");
NLogConfigurationException configException =
new NLogConfigurationException("Error loading extensions: " + assemblyFile, exception);

if (exception.MustBeRethrown())
if (configException.MustBeRethrown())
{
throw new NLogConfigurationException("Error loading extensions: " + assemblyFile, exception);
throw configException;
}
}

Expand All @@ -955,9 +976,9 @@ private void ParseExtensionsElement(NLogXmlElement extensionsElement, string bas
{
InternalLogger.Info("Loading assembly name: {0}", assemblyName);
#if SILVERLIGHT && !WINDOWS_PHONE
var si = Application.GetResourceStream(new Uri(assemblyName + ".dll", UriKind.Relative));
var assemblyPart = new AssemblyPart();
Assembly asm = assemblyPart.Load(si.Stream);
var si = Application.GetResourceStream(new Uri(assemblyName + ".dll", UriKind.Relative));
var assemblyPart = new AssemblyPart();
Assembly asm = assemblyPart.Load(si.Stream);
#else
Assembly asm = Assembly.Load(assemblyName);
#endif
Expand All @@ -966,17 +987,18 @@ private void ParseExtensionsElement(NLogXmlElement extensionsElement, string bas
}
catch (Exception exception)
{

if (exception.MustBeRethrownImmediately())
{
throw;
}

InternalLogger.Error(exception, "Error loading extensions.");
NLogConfigurationException configException =
new NLogConfigurationException("Error loading extensions: " + assemblyName, exception);

if (exception.MustBeRethrown())
if (configException.MustBeRethrown())
{
throw new NLogConfigurationException("Error loading extensions: " + assemblyName, exception);
throw configException;
}
}
}
Expand Down
74 changes: 74 additions & 0 deletions tests/NLog.UnitTests/Config/ExtensionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,80 @@ public void ExtensionTest_extensions_not_top_and_used()
Assert.Equal("MyExtensionNamespace.WhenFooFilter", configuration.LoggingRules[0].Filters[0].GetType().FullName);
}

[Fact]
public void ExtensionShouldThrowNLogConfiguratonExceptionWhenRegisteringInvalidType()
{
var configXml = @"
<nlog throwConfigExceptions='true'>
<extensions>
<add type='some_type_that_doesnt_exist'/>
</extensions>
</nlog>";
Assert.Throws<NLogConfigurationException>(()=>CreateConfigurationFromString(configXml));
}

[Fact]
public void ExtensionShouldThrowNLogConfiguratonExceptionWhenRegisteringInvalidAssembly()
{
var configXml = @"
<nlog throwConfigExceptions='true'>
<extensions>
<add assembly='some_assembly_that_doesnt_exist'/>
</extensions>
</nlog>";
Assert.Throws<NLogConfigurationException>(() => CreateConfigurationFromString(configXml));
}

[Fact]
public void ExtensionShouldThrowNLogConfiguratonExceptionWhenRegisteringInvalidAssemblyFile()
{
var configXml = @"
<nlog throwConfigExceptions='true'>
<extensions>
<add assemblyfile='some_file_that_doesnt_exist'/>
</extensions>
</nlog>";
Assert.Throws<NLogConfigurationException>(() => CreateConfigurationFromString(configXml));
}

[Fact]
public void ExtensionShouldNotThrowWhenRegisteringInvalidTypeIfThrowConfigExceptionsFalse()
{
var configXml = @"
<nlog throwConfigExceptions='false'>
<extensions>
<add type='some_type_that_doesnt_exist'/>
<add assembly='NLog'/>
</extensions>
</nlog>";
CreateConfigurationFromString(configXml);
}

[Fact]
public void ExtensionShouldNotThrowWhenRegisteringInvalidAssemblyIfThrowConfigExceptionsFalse()
{
var configXml = @"
<nlog throwConfigExceptions='false'>
<extensions>
<add assembly='some_assembly_that_doesnt_exist'/>
</extensions>
</nlog>";
CreateConfigurationFromString(configXml);
}

[Fact]
public void ExtensionShouldNotThrowWhenRegisteringInvalidAssemblyFileIfThrowConfigExceptionsFalse()
{
var configXml = @"
<nlog throwConfigExceptions='false'>
<extensions>
<add assemblyfile='some_file_that_doesnt_exist'/>
</extensions>
</nlog>";
CreateConfigurationFromString(configXml);
}


[Fact]
public void CustomXmlNamespaceTest()
{
Expand Down
18 changes: 5 additions & 13 deletions tests/NLog.UnitTests/LogFactoryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -63,25 +63,17 @@ public void Flush_DoNotThrowExceptionsAndTimeout_DoesNotThrow()
[Fact]
public void InvalidXMLConfiguration_DoesNotThrowErrorWhen_ThrowExceptionFlagIsNotSet()
{
Boolean ExceptionThrown = false;
try
{
LogManager.ThrowExceptions = false;

LogManager.Configuration = CreateConfigurationFromString(@"
LogManager.ThrowExceptions = false;

LogManager.Configuration = CreateConfigurationFromString(@"
<nlog internalLogToConsole='IamNotBooleanValue'>
<targets><target type='MethodCall' name='test' methodName='Throws' className='NLog.UnitTests.LogFactoryTests, NLog.UnitTests.netfx40' /></targets>
<rules>
<logger name='*' minlevel='Debug' writeto='test'></logger>
</rules>
</nlog>");
}
catch (Exception)
{
ExceptionThrown = true;
}

Assert.False(ExceptionThrown);

}

Expand Down Expand Up @@ -239,7 +231,7 @@ public void NewAttrOnNLogLevelShouldNotThrowError()
public void ValueWithVariableMustNotCauseInfiniteRecursion()
{
LogManager.Configuration = null;

File.WriteAllText("NLog.config", @"
<nlog>
<variable name='dir' value='c:\mylogs' />
Expand All @@ -259,7 +251,7 @@ public void ValueWithVariableMustNotCauseInfiniteRecursion()
File.Delete("NLog.config");
}
}

[Fact]
public void EnableAndDisableLogging()
{
Expand Down
1 change: 1 addition & 0 deletions tests/NLog.UnitTests/NLogTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ protected NLogTestBase()
LogManager.Configuration = null;
InternalLogger.Reset();
LogManager.ThrowExceptions = false;
LogManager.ThrowConfigExceptions = null;
}

protected void AssertDebugCounter(string targetName, int val)
Expand Down

0 comments on commit ee98bf2

Please sign in to comment.