From: Russ Combs <rucombs@cisco.com>
Date: Tue, 7 Oct 2014 13:41:43 +0000 (-0400)
Subject: various parse error handling changes
X-Git-Tag: 3.0.0-233~1383^2~36
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b1f2ef4c828dae90ab18c1ee50c0aa942e984abe;p=thirdparty%2Fsnort3.git

various parse error handling changes
---

diff --git a/ChangeLog b/ChangeLog
index f191d9ad4..bf8eef6ac 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -20,6 +20,7 @@
 -- added path to dynamically loaded plugins output with --list-plugins
 -- commented out mpse start up foo - needs reformatting
 -- added snort.detach() and fixed -j parsing
+-- various parsing changes for improved error handling
 
 122
 -- pulled thread pinning from Josh
diff --git a/src/main/shell.cc b/src/main/shell.cc
index 3dc56eea2..daca3d2b8 100644
--- a/src/main/shell.cc
+++ b/src/main/shell.cc
@@ -109,12 +109,6 @@ static void config_lua(
         load_overrides(L, s);
 
     run_config(L, "_G");
-
-    if ( int k = ModuleManager::get_errors() )
-    {
-        if ( snort_is_starting() )
-            FatalError("see prior %d errors\n", k);
-    }
 }
 
 //-------------------------------------------------------------------------
diff --git a/src/main/snort.cc b/src/main/snort.cc
index 2887634ad..e8439f0c7 100644
--- a/src/main/snort.cc
+++ b/src/main/snort.cc
@@ -333,9 +333,9 @@ static void SnortInit(int argc, char **argv)
 
     // Must be after CodecManager::instantiate()
     if ( !InspectorManager::configure(snort_conf) )
-        FatalError("can't initialize inspectors\n");
+        ParseError("can't initialize inspectors");
 
-    if ( ScLogVerbose() )
+    else if ( ScLogVerbose() )
         InspectorManager::print_config(snort_conf);
 
     ParseRules(snort_conf);
@@ -368,7 +368,7 @@ static void SnortInit(int argc, char **argv)
     SFAT_Start();
 
 #ifdef PPM_MGR
-    PPM_PRINT_CFG(&snort_conf->ppm_cfg);
+    //PPM_PRINT_CFG(&snort_conf->ppm_cfg);
 #endif
 
     /* Finish up the pcap list and put in the queues */
@@ -377,7 +377,7 @@ static void SnortInit(int argc, char **argv)
     // FIXIT-L stuff like this that is also done in snort_config.cc::VerifyReload()
     // should be refactored
     if ((snort_conf->bpf_filter == NULL) && (snort_conf->bpf_file != NULL))
-        snort_conf->bpf_filter = read_infile(snort_conf->bpf_file);
+        snort_conf->bpf_filter = read_infile("packets.bpf_file", snort_conf->bpf_file);
 
     if (snort_conf->bpf_filter != NULL)
         LogMessage("Snort BPF option: %s\n", snort_conf->bpf_filter);
@@ -649,7 +649,7 @@ SnortConfig* get_reload_config()
     }
 
 #ifdef PPM_MGR
-    PPM_PRINT_CFG(&sc->ppm_cfg);
+    //PPM_PRINT_CFG(&sc->ppm_cfg);
 #endif
 
     return sc;
diff --git a/src/main/snort_config.cc b/src/main/snort_config.cc
index df69c0874..0a3f46994 100644
--- a/src/main/snort_config.cc
+++ b/src/main/snort_config.cc
@@ -498,7 +498,7 @@ int VerifyReload(SnortConfig *sc)
     }
 
     if ((sc->bpf_filter == NULL) && (sc->bpf_file != NULL))
-        sc->bpf_filter = read_infile(sc->bpf_file);
+        sc->bpf_filter = read_infile("packets.bpf_file", sc->bpf_file);
 
     if ((sc->bpf_filter != NULL) && (snort_conf->bpf_filter != NULL))
     {
diff --git a/src/managers/module_manager.cc b/src/managers/module_manager.cc
index 413806310..a5cb5d7e0 100644
--- a/src/managers/module_manager.cc
+++ b/src/managers/module_manager.cc
@@ -322,7 +322,7 @@ static bool set_param(Module* mod, const char* fqn, Value& val)
 {
     if ( !mod->set(fqn, val, s_config) )
     {
-        ErrorMessage("ERROR: %s is invalid\n", fqn);
+        ParseError("%s is invalid", fqn);
         ++s_errors;
     }
 
@@ -350,7 +350,7 @@ static bool set_value(const char* fqn, Value& v)
  
     if ( !p )
     {
-        ErrorMessage("ERROR can't find %s\n", fqn);
+        ParseError("can't find %s", fqn);
         ++s_errors;
         return false;
     }
@@ -363,11 +363,11 @@ static bool set_value(const char* fqn, Value& v)
     }
 
     if ( v.get_type() == Value::VT_STR )
-        ErrorMessage("ERROR invalid %s = '%s'\n", fqn, v.get_string());
+        ParseError("invalid %s = '%s'", fqn, v.get_string());
     else if ( v.get_real() == v.get_long() )
-        ErrorMessage("ERROR invalid %s = %ld\n", fqn, v.get_long());
+        ParseError("invalid %s = %ld", fqn, v.get_long());
     else
-        ErrorMessage("ERROR invalid %s = %g\n", fqn, v.get_real());
+        ParseError("invalid %s = %g", fqn, v.get_real());
 
     ++s_errors;
     return false;
@@ -423,12 +423,12 @@ SO_PUBLIC bool open_table(const char* s, int idx)
 
         if ( !p )
         {
-            ParseError("can't find %s\n", s);
+            ParseError("can't find %s", s);
             return false;
         }
         else if ((idx > 0) && (p->type == Parameter::PT_TABLE))
         {
-            ParseError("%s is a table. All elements must be named\n", s);
+            ParseError("%s is a table; all elements must be named", s);
             return false;
         }
     }
@@ -862,7 +862,7 @@ static void make_rule(ostream& os, const Module* m, const RuleMap* r)
 // (we don't want to suppress it because it could mean something is broken)
 void ModuleManager::load_rules(SnortConfig* sc)
 {
-    // FIXIT-M callers of ParseConfigString() should not have to push parse loc
+    s_modules.sort(comp_gids);
     push_parse_location("builtin");
 
     for ( auto p : s_modules )
diff --git a/src/managers/plugin_manager.cc b/src/managers/plugin_manager.cc
index 60aec25e2..766012815 100644
--- a/src/managers/plugin_manager.cc
+++ b/src/managers/plugin_manager.cc
@@ -475,7 +475,14 @@ void PluginManager::instantiate(
 void PluginManager::instantiate(
     const BaseApi* api, Module* mod, SnortConfig* sc, const char* name)
 {
-    assert(api->type == PT_INSPECTOR);
-    InspectorManager::instantiate((InspectApi*)api, mod, sc, name);
+    if ( api->type == PT_INSPECTOR )
+        InspectorManager::instantiate((InspectApi*)api, mod, sc, name);
+
+    else if ( api->type == PT_DATA )
+        // FIXIT-H instantiate PT_DATA with name
+        DataManager::instantiate((DataApi*)api, mod, sc/*, name*/);
+
+    else
+        assert(false);
 }
 
diff --git a/src/parser/parse_conf.cc b/src/parser/parse_conf.cc
index 302db4bd5..46e31a371 100644
--- a/src/parser/parse_conf.cc
+++ b/src/parser/parse_conf.cc
@@ -112,12 +112,17 @@ void push_parse_location(const char* file, unsigned line)
 
     Location loc(file, line);
     files.push(loc);
+    LogMessage("Loading %s:\n", file);
 }
 
 void pop_parse_location()
 {
     if ( !files.empty() )
+    {
+        Location& loc = files.top();
+        LogMessage("Finished %s.\n", loc.file.c_str());
         files.pop();
+    }
 }
 
 void inc_parse_position()
diff --git a/src/parser/parser.cc b/src/parser/parser.cc
index 2f51dcd63..b8ec94fc5 100644
--- a/src/parser/parser.cc
+++ b/src/parser/parser.cc
@@ -580,7 +580,6 @@ static void parse_file(SnortConfig* sc, Shell* sh)
     if ( !fname || !*fname )
         return;
 
-    LogMessage("Loading %s:\n", fname);
     push_parse_location(fname);
     sh->configure(sc);
     pop_parse_location();
@@ -1118,8 +1117,8 @@ void ParseError(const char *format, ...)
     unsigned file_line;
     get_parse_location(file_name, file_line);
 
-    if (file_name != NULL)
-        LogMessage("ERROR: %s(%d) %s\n", file_name, file_line, buf);
+    if (file_line )
+        LogMessage("ERROR: %s:%d %s\n", file_name, file_line, buf);
     else
         LogMessage("ERROR: %s\n", buf);
 
@@ -1141,8 +1140,8 @@ void ParseWarning(const char *format, ...)
     unsigned file_line;
     get_parse_location(file_name, file_line);
 
-    if (file_name != NULL)
-        LogMessage("WARNING: %s(%d) %s\n", file_name, file_line, buf);
+    if ( file_line )
+        LogMessage("WARNING: %s:%d %s\n", file_name, file_line, buf);
     else
         LogMessage("WARNING: %s\n", buf);
 
diff --git a/src/parser/parser.h b/src/parser/parser.h
index e50d18015..0a8362c7d 100644
--- a/src/parser/parser.h
+++ b/src/parser/parser.h
@@ -42,7 +42,7 @@ unsigned get_parse_warnings();
 
 const char* get_parse_file();
 void get_parse_location(const char*& name, unsigned& line);
-void push_parse_location(const char* name, unsigned line = 1);
+void push_parse_location(const char* name, unsigned line = 0);
 
 void pop_parse_location();
 void inc_parse_position();
diff --git a/src/service_inspectors/ftp_telnet/ftp_module.cc b/src/service_inspectors/ftp_telnet/ftp_module.cc
index 07a1f4e6b..8cdf04ac7 100644
--- a/src/service_inspectors/ftp_telnet/ftp_module.cc
+++ b/src/service_inspectors/ftp_telnet/ftp_module.cc
@@ -58,7 +58,7 @@ static const Parameter ftp_client_params[] =
     { "bounce", Parameter::PT_BOOL, nullptr, "false",
       "check for bounces" },
 
-    { "bounce_to", Parameter::PT_TABLE, client_bounce_params, nullptr,
+    { "bounce_to", Parameter::PT_LIST, client_bounce_params, nullptr,
       "allow bounces to CIDRs / ports" },
 
     { "ignore_telnet_erase_cmds", Parameter::PT_BOOL, nullptr, "false",
diff --git a/src/utils/util.cc b/src/utils/util.cc
index 33fcb48fa..ae6658fed 100644
--- a/src/utils/util.cc
+++ b/src/utils/util.cc
@@ -487,7 +487,7 @@ void CleanupProtoNames(void)
 
 /****************************************************************************
  *
- * Function: read_infile(char *)
+ * Function: read_infile(const char* key, const char* file)
  *
  * Purpose: Reads the BPF filters in from a file.  Ripped from tcpdump.
  *
@@ -496,7 +496,7 @@ void CleanupProtoNames(void)
  * Returns: the processed BPF string
  *
  ****************************************************************************/
-char *read_infile(char *fname)
+char *read_infile(const char* key, const char* fname)
 {
     int fd, cc;
     char *cp, *cmt;
@@ -506,7 +506,7 @@ char *read_infile(char *fname)
 
     if(fd < 0)
     {
-        ParseError("can't open %s: %s\n", fname, get_error(errno));
+        ParseError("can't open %s = %s: %s\n", key, fname, get_error(errno));
         return nullptr;
     }
 
diff --git a/src/utils/util.h b/src/utils/util.h
index 6e64882c7..58fe8c6a3 100644
--- a/src/utils/util.h
+++ b/src/utils/util.h
@@ -81,7 +81,7 @@ int gmt2local(time_t);
 void ts_print(register const struct timeval *, char *);
 void strip(char *);
 void CheckLogDir(void);
-char *read_infile(char *);
+char *read_infile(const char* key, const char* fname);
 void CleanupProtoNames(void);
 void CreatePidFile(pid_t);
 void ClosePidFile(void);