[prev in list] [next in list] [prev in thread] [next in thread]
List: log4net-dev
Subject: Re: Fix for FileAppender's TextWriter creation in OpenFile
From: Jonas.Baehr () rohde-schwarz ! com
Date: 2016-08-22 12:10:50
Message-ID: OF8F4DB389.4888273D-ONC1258017.004171EF-C1258017.0042E9A8 () rohde-schwarz ! com
[Download RAW message or body]
This is a multipart message in MIME format.
--=_alternative 0042E94DC1258017_=
Content-Type: text/plain; charset="US-ASCII"
Hi Dominik,
Although there are no dedicated tests for the file appender as such, I did
run the test suite at the time I changed the code. The recent changes
indtroduced with r1756284 ".NET Core improvements by Peter Jas - closes
#30" broke a lot of tests becuse of calling `ToUpperInvariant` in string
objects that can be null. I'll start another thread about that issue.
With that fixed, only a single test
`EventLogAppenderTest.ActivateOptionsDisablesAppenderIfSourceDoesntExist`
fails here, but this has nothing to do with the file appender change.
Notably the RollingFileAppender tests run without issues after my change.
Regards,
Jonas
Von: Dominik Psenner <dpsenner@apache.org>
An: log4net-dev@logging.apache.org
Datum: 22.08.2016 13:30
Betreff: Re: Fix for FileAppender's TextWriter creation in OpenFile
Hi Jonas,
thanks for your patch! It looks sensible. Have you run the tests against
it?
Cheers,Dominik
On 2016-08-22 12:27, Jonas.Baehr@rohde-schwarz.com wrote:
Hi,
`FileAppender` has a vritual overload for the Method `SetQWForFiles` that
takes a `Stream` and creates a StreamWriter from it. According to the API
documentation
"""
This method can be overridden by sub classes that want to wrap the
<see cref="Stream"/> in some way, for example to encrypt the output
data using a <c>System.Security.Cryptography.CryptoStream</c>.
"""
Unfortunately, it doesn't get called. Instead it is inlined in `OpenFile`,
so derived classes have no chance in wrapping the stream before it is
passed to the writer.
This trivial patch changes this.
---------------------8<--------------8<-----------------
--- C:/temp/FileAppender.cs-revBASE.svn003.tmp.cs Sat Aug 13
18:57:44 2016
+++ C:/Users/BAEHR/Documents/log4net-trunk/src/Appender/FileAppender.cs
Mon Aug 22 11:59:41 2016
@@ -1382 +1382 @@ namespace log4net.Appender
- SetQWForFiles(new
StreamWriter(m_stream, m_encoding));
+ SetQWForFiles(m_stream);
---------------------8<--------------8<-----------------
Best Regards,
Jonas
--=_alternative 0042E94DC1258017_=
Content-Type: text/html; charset="US-ASCII"
<font size=2 face="sans-serif">Hi Dominik,</font>
<br>
<br><font size=2 face="sans-serif">Although there are no dedicated tests
for the file appender as such, I did run the test suite at the time I changed
the code. The recent changes indtroduced with r1756284 ".NET Core
improvements by Peter Jas - closes #30" broke a lot of tests becuse
of calling `ToUpperInvariant` in string objects that can be null. I'll
start another thread about that issue.</font>
<br><font size=2 face="sans-serif">With that fixed, only a single test
`EventLogAppenderTest.ActivateOptionsDisablesAppenderIfSourceDoesntExist`
fails here, but this has nothing to do with the file appender change.</font>
<br>
<br><font size=2 face="sans-serif">Notably the RollingFileAppender tests
run without issues after my change.</font>
<br>
<br><font size=2 face="sans-serif">Regards,</font>
<br><font size=2 face="sans-serif">Jonas</font>
<br>
<br>
<br>
<br><font size=1 color=#5f5f5f face="sans-serif">Von:
</font><font size=1 face="sans-serif">Dominik Psenner \
<dpsenner@apache.org></font> <br><font size=1 color=#5f5f5f \
face="sans-serif">An: </font><font size=1 \
face="sans-serif">log4net-dev@logging.apache.org</font> <br><font size=1 \
color=#5f5f5f face="sans-serif">Datum: </font><font size=1 \
face="sans-serif">22.08.2016 13:30</font> <br><font size=1 color=#5f5f5f \
face="sans-serif">Betreff: </font><font size=1 \
face="sans-serif">Re: Fix for FileAppender's TextWriter creation in OpenFile</font>
<br>
<hr noshade>
<br>
<br>
<br><font size=3>Hi Jonas,</font>
<p><font size=3>thanks for your patch! It looks sensible. Have you run
the tests against it?</font>
<p><font size=3>Cheers,Dominik</font>
<p><font size=3>On 2016-08-22 12:27, </font><a \
href="mailto:Jonas.Baehr@rohde-schwarz.com"><font size=3 \
color=blue><u>Jonas.Baehr@rohde-schwarz.com</u></font></a><font size=3> wrote:</font>
<br><font size=2 face="sans-serif">Hi,</font><font size=3> <br>
</font><font size=2 face="sans-serif"><br>
`FileAppender` has a vritual overload for the Method `SetQWForFiles` that
takes a `Stream` and creates a StreamWriter from it. According to the API
documentation</font><font size=3> </font><font size=2 face="sans-serif"><br>
"""</font><font size=3> </font><font size=2 face="sans-serif"><br>
This method can be overridden by sub classes that want to wrap the</font><font \
size=3> </font><font size=2 face="sans-serif"><br>
<see cref="Stream"/> in some way, for example to encrypt
the output</font><font size=3> </font><font size=2 face="sans-serif"><br>
data using a <c>System.Security.Cryptography.CryptoStream</c>.
</font><font size=3> </font><font size=2 \
face="sans-serif"><br> """</font><font size=3> <br>
</font><font size=2 face="sans-serif"><br>
Unfortunately, it doesn't get called. Instead it is inlined in `OpenFile`,
so derived classes have no chance in wrapping the stream before it is passed
to the writer.</font><font size=3> </font><font size=2 face="sans-serif"><br>
This trivial patch changes this.</font><font size=3> <br>
</font><font size=2 face="sans-serif"><br>
---------------------8<--------------8<-----------------</font><font size=3>
</font><font size=2 face="sans-serif"><br>
--- C:/temp/FileAppender.cs-revBASE.svn003.tmp.cs
Sat Aug 13 18:57:44 2016</font><font size=3> </font><font size=2 \
face="sans-serif"><br>
+++ C:/Users/BAEHR/Documents/log4net-trunk/src/Appender/FileAppender.cs
Mon Aug 22 11:59:41 2016</font><font size=3>
</font><font size=2 face="sans-serif"><br>
@@ -1382 +1382 @@ namespace log4net.Appender</font><font size=3> </font><font size=2 \
face="sans-serif"><br>
-
SetQWForFiles(new StreamWriter(m_stream, \
m_encoding));</font><font size=3> </font><font size=2 face="sans-serif"><br>
+
SetQWForFiles(m_stream);</font><font size=3> </font><font size=2 \
face="sans-serif"><br>
---------------------8<--------------8<-----------------</font><font size=3>
<br>
</font><font size=2 face="sans-serif"><br>
Best Regards,</font><font size=3> </font><font size=2 face="sans-serif"><br>
Jonas</font><font size=3> </font>
<br>
<br>
<br>
--=_alternative 0042E94DC1258017_=--
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic