[log4perl-devel] Log4perl config file integrity check

[Mike S. <m...@pe...>]

Hi all,

there was a complaint a while ago that even nonsensical Log4perl config 
files are parsed and loaded without complaints.

The patch below implements a check in Log4perl::init() to verify that 
what we've read is a somewhat valid configuration. It issues a warning 
if no appenders have been defined at all.

Let me know if you see any issues, just want to make sure we're not 
breaking any old code.

--
-- Mike
Mike Schilli
m...@pe...

Index: Changes
===================================================================
RCS file: /cvsroot/log4perl/Log-Log4perl/Changes,v
retrieving revision 1.198
diff -a -u -r1.198 Changes
--- Changes	16 Aug 2004 01:28:10 -0000	1.198
+++ Changes	16 Aug 2004 02:18:37 -0000
@@ -17,6 +17,11 @@
             $parser->file($name) before calling L4p->init($parser).
             The Property, DOM and LDAP configurators have been
             adapted, check their implementation for details.
+    * (ms) Added integrity check for Log4perl configurations: Log4perl
+           now issues a warning if a configuration doesn't define any
+           appenders. Should anyone not like this, it can be turned
+           off by setting $L4p::Config::CONFIG_INTEGRITY_CHECK = 0
+           before calling init().

  0.47 (07/11/2004)
      * (ms) Added suggestion by Hutton Davidson <Dav...@ft...>
Index: lib/Log/Log4perl/Config.pm
===================================================================
RCS file: /cvsroot/log4perl/Log-Log4perl/lib/Log/Log4perl/Config.pm,v
retrieving revision 1.62
diff -a -u -r1.62 Config.pm
--- lib/Log/Log4perl/Config.pm	16 Aug 2004 01:28:11 -0000	1.62
+++ lib/Log/Log4perl/Config.pm	16 Aug 2004 02:18:39 -0000
@@ -16,7 +16,9 @@

  use constant _INTERNAL_DEBUG => 0;

-our $CONFIG_FILE_READS = 0;
+our $CONFIG_FILE_READS       = 0;
+our $CONFIG_INTEGRITY_CHECK  = 1;
+our $CONFIG_INTEGRITY_ERROR  = undef;

  # How to map lib4j levels to Log::Dispatch levels
  my @LEVEL_MAP_A = qw(
@@ -269,6 +271,24 @@

      #now we're done, set up all the output methods (e.g. ->debug('...'))
      Log::Log4perl::Logger::reset_all_output_methods();
+
+    #Run a sanity test on the config not disabled
+    if($Log::Log4perl::Config::CONFIG_INTEGRITY_CHECK and
+       !config_is_sane()) {
+        warn "Log::Log4perl configuration looks suspicious: ",
+             "$CONFIG_INTEGRITY_ERROR";
+    }
+}
+
+##################################################
+sub config_is_sane {
+##################################################
+    if(scalar keys %Log::Log4perl::Logger::APPENDER_BY_NAME == 0) {
+        $CONFIG_INTEGRITY_ERROR = "No appenders defined";
+        return 0;
+    }
+
+    return 1;
  }

  ##################################################
Index: t/004Config.t
===================================================================
RCS file: /cvsroot/log4perl/Log-Log4perl/t/004Config.t,v
retrieving revision 1.18
diff -a -u -r1.18 004Config.t
--- t/004Config.t	16 Aug 2004 01:28:11 -0000	1.18
+++ t/004Config.t	16 Aug 2004 02:18:39 -0000
@@ -7,15 +7,19 @@
  # change 'tests => 1' to 'tests => last_test_to_print';
  #########################
  use Test::More;
-BEGIN { plan tests => 12 };
+BEGIN { plan tests => 13 };

  use Log::Log4perl;
  use Log::Log4perl::Appender::TestBuffer;
+use File::Spec;

  my $EG_DIR = "eg";
  $EG_DIR = "../eg" unless -d $EG_DIR;

-ok(1); # If we made it this far, we are ok.
+my $TMP_FILE = File::Spec->catfile(qw(t tmp warnings));
+$TMP_FILE = "tmp/warnings" if ! -d "t";
+
+ok(1, "Startup"); # If we made it this far, we are ok.

  ######################################################################
  # Test the root logger on a configuration file defining a file appender
@@ -27,7 +31,7 @@


  like(Log::Log4perl::Appender::TestBuffer->by_name("A1")->buffer(),
-   qr#^\d+\s+\[N/A\] DEBUG  N/A - Gurgel$#);
+   qr#^\d+\s+\[N/A\] DEBUG  N/A - Gurgel$#, "Root logger");

  ######################################################################
  # Test the root logger via inheritance (discovered by Kevin Goess)
@@ -40,7 +44,7 @@
  $logger->debug("Gurgel");

  like(Log::Log4perl::Appender::TestBuffer->by_name("A1")->buffer(),
-    qr#^\d+\s+\[N/A\] DEBUG foo N/A - Gurgel$#);
+    qr#^\d+\s+\[N/A\] DEBUG foo N/A - Gurgel$#, "Root logger inherited");

  ######################################################################
  # Test init with a string
@@ -58,7 +62,7 @@
  $logger->debug("Gurgel");

  like(Log::Log4perl::Appender::TestBuffer->by_name("A1")->buffer(),
-    qr#^\d+\s+\[N/A\] DEBUG foo - Gurgel$#);
+    qr#^\d+\s+\[N/A\] DEBUG foo - Gurgel$#, "Init via string");

  ######################################################################
  # Test init with a hashref
@@ -79,7 +83,7 @@
  $logger->debug("Gurgel");

  like(Log::Log4perl::Appender::TestBuffer->by_name("A1")->buffer(),
-    qr#^\d+\s+\[N/A\] DEBUG foo - Gurgel$#);
+    qr#^\d+\s+\[N/A\] DEBUG foo - Gurgel$#, "Init via hashref");


  ############################################################
@@ -137,10 +141,10 @@
  #  },


-is($stub_hook->{P}{login}{hostname}, 'a.jabber.server');
-is($stub_hook->{P}{login}{password}, 'bunny');
-is($stub_hook->{P}{to}[0], 'elmer@a.jabber.server');
-is($stub_hook->{P}{to}[1], 'sa...@an...rver');
+is($stub_hook->{P}{login}{hostname}, 'a.jabber.server', "Config and 
Jabber");
+is($stub_hook->{P}{login}{password}, 'bunny', "Config and Jabber");
+is($stub_hook->{P}{to}[0], 'elmer@a.jabber.server', "Config and Jabber");
+is($stub_hook->{P}{to}[1], 'sa...@an...rver', "Config and 
Jabber");

  ##########################################################################
  # Test what happens if we define a PatternLayout without ConversionPattern
@@ -160,7 +164,7 @@
  #actually, it turns out that log4j handles this, if no ConversionPattern
  #specified is uses DEFAULT_LAYOUT_PATTERN, %m%n
  #ok($@, '/No ConversionPattern given for PatternLayout/');
-is($@, '');
+is($@, '', 'PatternLayout without ConversionPattern');

  ######################################################################
  # Test with $/ set to undef
@@ -172,7 +176,7 @@
  $logger->debug("Gurgel");

  like(Log::Log4perl::Appender::TestBuffer->by_name("A1")->buffer(),
-     qr#^\d+\s+\[N/A\] DEBUG  N/A - Gurgel$#);
+     qr#^\d+\s+\[N/A\] DEBUG  N/A - Gurgel$#, "Config in slurp mode");

  ######################################################################
  # Test init with a config parser object
@@ -194,5 +198,21 @@
  $logger->debug("Gurgel");

  is(Log::Log4perl::Appender::TestBuffer->by_name("A1")->buffer(),
-   "objectGurgel\n");
+   "objectGurgel\n", "Init with parser object");
+
+######################################################################
+# Test integrity check
+######################################################################
+open STDERR, ">$TMP_FILE";
+open IN, "<$TMP_FILE" or die "Cannot open $TMP_FILE";
+sub readwarn { return scalar <IN>; }
+END { close IN }
+
+Log::Log4perl->init(\ <<EOT);
+    # Just an empty configuration
+EOT
+
+like(readwarn(), qr/looks suspicious: No appenders/,
+     "Test integrity check on empty conf file");

+unlink $TMP_FILE;