From 205ad78369701a5e67260b421411a52b28c45440 Mon Sep 17 00:00:00 2001 From: Seth Hall Date: Tue, 14 Aug 2012 15:09:38 -0400 Subject: [PATCH 01/11] Fix some problems in logs-to-elasticsearch.bro --- scripts/policy/tuning/logs-to-elasticsearch.bro | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/policy/tuning/logs-to-elasticsearch.bro b/scripts/policy/tuning/logs-to-elasticsearch.bro index 207a9acc04..2a4b70362a 100644 --- a/scripts/policy/tuning/logs-to-elasticsearch.bro +++ b/scripts/policy/tuning/logs-to-elasticsearch.bro @@ -8,13 +8,13 @@ export { ## Optionally ignore any :bro:type:`Log::ID` from being sent to ## ElasticSearch with this script. - const excluded_log_ids: set[string] = set("Communication::LOG") &redef; + const excluded_log_ids: set[Log::ID] &redef; ## If you want to explicitly only send certain :bro:type:`Log::ID` ## streams, add them to this set. If the set remains empty, all will ## be sent. The :bro:id:`LogElasticSearch::excluded_log_ids` option will remain in ## effect as well. - const send_logs: set[string] = set() &redef; + const send_logs: set[Log::ID] &redef; } event bro_init() &priority=-5 @@ -24,8 +24,8 @@ event bro_init() &priority=-5 for ( stream_id in Log::active_streams ) { - if ( fmt("%s", stream_id) in excluded_log_ids || - (|send_logs| > 0 && fmt("%s", stream_id) !in send_logs) ) + if ( stream_id in excluded_log_ids || + (|send_logs| > 0 && stream_id !in send_logs) ) next; local filter: Log::Filter = [$name = "default-es", From b13196cbf194419836f9b7627aab5cab25c47397 Mon Sep 17 00:00:00 2001 From: Seth Hall Date: Thu, 16 Aug 2012 09:24:25 -0400 Subject: [PATCH 02/11] Fixed more potential problems with deadlocked ES threads and signals from libcurl. --- src/logging/writers/ElasticSearch.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/logging/writers/ElasticSearch.cc b/src/logging/writers/ElasticSearch.cc index e688686b35..cb3248a044 100644 --- a/src/logging/writers/ElasticSearch.cc +++ b/src/logging/writers/ElasticSearch.cc @@ -371,7 +371,11 @@ bool ElasticSearch::HTTPSend(CURL *handle) // The best (only?) way to disable that is to just use HTTP 1.0 curl_easy_setopt(handle, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_1_0); - //curl_easy_setopt(handle, CURLOPT_TIMEOUT_MS, transfer_timeout); + // Some timeout options. These will need more attention later. + curl_easy_setopt(handle, CURLOPT_NOSIGNAL, 1); + curl_easy_setopt(handle, CURLOPT_CONNECTTIMEOUT_MS, transfer_timeout); + curl_easy_setopt(handle, CURLOPT_TIMEOUT_MS, transfer_timeout*2); + curl_easy_setopt(handle, CURLOPT_DNS_CACHE_TIMEOUT, 60*60); CURLcode return_code = curl_easy_perform(handle); From 4da209d3b1fe6fa9c5118e055752843b2fb73a45 Mon Sep 17 00:00:00 2001 From: Robin Sommer Date: Thu, 16 Aug 2012 11:48:56 -0700 Subject: [PATCH 03/11] Installing a handler for running out of memory in "new". Bro will now print an error message in that case rather than abort with an uncaught exception. --- CHANGES | 6 ++++++ VERSION | 2 +- src/main.cc | 4 ++++ src/util.cc | 8 +++++++- 4 files changed, 18 insertions(+), 2 deletions(-) diff --git a/CHANGES b/CHANGES index 08998ab9f4..f0c73ce8d9 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,10 @@ +2.1-beta-21 | 2012-08-16 11:48:56 -0700 + + * Installing a handler for running out of memory in "new". Bro will + now print an error message in that case rather than abort with an + uncaught exception. (Robin Sommer) + 2.1-beta-20 | 2012-08-16 11:43:31 -0700 * Fixed potential problems with ElasticSearch output plugin. (Seth diff --git a/VERSION b/VERSION index c42c76c8ba..5d7a2a2cce 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.1-beta-20 +2.1-beta-21 diff --git a/src/main.cc b/src/main.cc index 407f67c9af..5999186240 100644 --- a/src/main.cc +++ b/src/main.cc @@ -337,6 +337,8 @@ void terminate_bro() delete log_mgr; delete thread_mgr; delete reporter; + + reporter = 0; } void termination_signal() @@ -380,6 +382,8 @@ static void bro_new_handler() int main(int argc, char** argv) { + std::set_new_handler(bro_new_handler); + brofiler.ReadStats(); bro_argc = argc; diff --git a/src/util.cc b/src/util.cc index 2d981e952e..3b6fcac76f 100644 --- a/src/util.cc +++ b/src/util.cc @@ -1383,7 +1383,13 @@ void safe_close(int fd) void out_of_memory(const char* where) { - reporter->FatalError("out of memory in %s.\n", where); + fprintf(stderr, "out of memory in %s.\n", where); + + if ( reporter ) + // Guess that might fail here if memory is really tight ... + reporter->FatalError("out of memory in %s.\n", where); + + abort(); } void get_memory_usage(unsigned int* total, unsigned int* malloced) From a6f7fd9c874ffdab31c3c79c9956857617b723d5 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Thu, 16 Aug 2012 15:59:26 -0500 Subject: [PATCH 04/11] Fix memory leak of serialized IDs when compiled with --enable-debug. When using --enable-debug, values keep track of the last identifier to which they were bound by storing a ref'd ID pointer. This could lead to some circular dependencies in which an ID is never reclaimed because the Val is bound to the ID and the ID is bound to the Val, with both holding references to each other. There might be more cases where this feature of --enable-debug caused a leak, but it showed up in particular when running the core.leaks.remote unit test due to the internal SendID("peer_description") call during the handshake between remote processes. Other tests showed the send_id() BIF leaked more generally. Tracking the ID last bound to a Val through just the identifier string instead of a ref'd ID pointer fixes the leak. --- src/RemoteSerializer.cc | 5 ----- src/Val.cc | 2 +- src/Val.h | 16 +++++++++------- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/RemoteSerializer.cc b/src/RemoteSerializer.cc index cfd20eba39..564ad2be68 100644 --- a/src/RemoteSerializer.cc +++ b/src/RemoteSerializer.cc @@ -2897,11 +2897,6 @@ void RemoteSerializer::GotID(ID* id, Val* val) (desc && *desc) ? desc : "not set"), current_peer); -#ifdef USE_PERFTOOLS_DEBUG - // May still be cached, but we don't care. - heap_checker->IgnoreObject(id); -#endif - Unref(id); return; } diff --git a/src/Val.cc b/src/Val.cc index 8a8c2b18c0..79fa8a0c69 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -64,7 +64,7 @@ Val::~Val() Unref(type); #ifdef DEBUG - Unref(bound_id); + delete [] bound_id; #endif } diff --git a/src/Val.h b/src/Val.h index 2ca18e6131..c3ec5b04fb 100644 --- a/src/Val.h +++ b/src/Val.h @@ -347,13 +347,15 @@ public: #ifdef DEBUG // For debugging, we keep a reference to the global ID to which a // value has been bound *last*. - ID* GetID() const { return bound_id; } + ID* GetID() const + { + return bound_id ? global_scope()->Lookup(bound_id) : 0; + } + void SetID(ID* id) { - if ( bound_id ) - ::Unref(bound_id); - bound_id = id; - ::Ref(bound_id); + delete [] bound_id; + bound_id = id ? copy_string(id->Name()) : 0; } #endif @@ -401,8 +403,8 @@ protected: RecordVal* attribs; #ifdef DEBUG - // For debugging, we keep the ID to which a Val is bound. - ID* bound_id; + // For debugging, we keep the name of the ID to which a Val is bound. + const char* bound_id; #endif }; From 508ac1c7ba1b9fbddc128a109b51bd6376ba4bd9 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Thu, 16 Aug 2012 16:33:46 -0500 Subject: [PATCH 05/11] Unit test tweaks/fixes. - Some baselines for tests in "leaks" group were outdated. - Changed a few of the cluster/communication tests to terminate more explicitly instead of relying on btest-bg-wait to kill processes. This makes the tests finish faster in the success case and makes the reason for failing clearer in the that case. --- .../manager-1.metrics.log | 8 +++-- .../core.leaks.remote/sender.test.failure.log | 8 +++-- .../core.leaks.remote/sender.test.log | 12 ++++--- .../core.leaks.remote/sender.test.success.log | 6 ++-- testing/btest/core/leaks/basic-cluster.bro | 23 +++++++++++++- testing/btest/core/leaks/remote.bro | 31 ++++++++++++++----- .../base/frameworks/logging/remote.bro | 23 +++++++++++--- .../base/frameworks/metrics/basic-cluster.bro | 23 +++++++++++++- .../metrics/cluster-intermediate-update.bro | 17 +++++++++- 9 files changed, 122 insertions(+), 29 deletions(-) diff --git a/testing/btest/Baseline/core.leaks.basic-cluster/manager-1.metrics.log b/testing/btest/Baseline/core.leaks.basic-cluster/manager-1.metrics.log index 42fcd6a526..cb1bd5af01 100644 --- a/testing/btest/Baseline/core.leaks.basic-cluster/manager-1.metrics.log +++ b/testing/btest/Baseline/core.leaks.basic-cluster/manager-1.metrics.log @@ -3,8 +3,10 @@ #empty_field (empty) #unset_field - #path metrics +#open 2012-07-20-01-50-41 #fields ts metric_id filter_name index.host index.str index.network value #types time enum string addr string subnet count -1331256494.591966 TEST_METRIC foo-bar 6.5.4.3 - - 4 -1331256494.591966 TEST_METRIC foo-bar 7.2.1.5 - - 2 -1331256494.591966 TEST_METRIC foo-bar 1.2.3.4 - - 6 +1342749041.601712 TEST_METRIC foo-bar 6.5.4.3 - - 4 +1342749041.601712 TEST_METRIC foo-bar 7.2.1.5 - - 2 +1342749041.601712 TEST_METRIC foo-bar 1.2.3.4 - - 6 +#close 2012-07-20-01-50-49 diff --git a/testing/btest/Baseline/core.leaks.remote/sender.test.failure.log b/testing/btest/Baseline/core.leaks.remote/sender.test.failure.log index 5a26f322f4..71e1d18c73 100644 --- a/testing/btest/Baseline/core.leaks.remote/sender.test.failure.log +++ b/testing/btest/Baseline/core.leaks.remote/sender.test.failure.log @@ -3,8 +3,10 @@ #empty_field (empty) #unset_field - #path test.failure +#open 2012-07-20-01-50-18 #fields t id.orig_h id.orig_p id.resp_h id.resp_p status country #types time addr port addr port string string -1331256472.375609 1.2.3.4 1234 2.3.4.5 80 failure US -1331256472.375609 1.2.3.4 1234 2.3.4.5 80 failure UK -1331256472.375609 1.2.3.4 1234 2.3.4.5 80 failure MX +1342749018.970682 1.2.3.4 1234 2.3.4.5 80 failure US +1342749018.970682 1.2.3.4 1234 2.3.4.5 80 failure UK +1342749018.970682 1.2.3.4 1234 2.3.4.5 80 failure MX +#close 2012-07-20-01-50-18 diff --git a/testing/btest/Baseline/core.leaks.remote/sender.test.log b/testing/btest/Baseline/core.leaks.remote/sender.test.log index 9d2ba26f48..bc3dac5a1a 100644 --- a/testing/btest/Baseline/core.leaks.remote/sender.test.log +++ b/testing/btest/Baseline/core.leaks.remote/sender.test.log @@ -3,10 +3,12 @@ #empty_field (empty) #unset_field - #path test +#open 2012-07-20-01-50-18 #fields t id.orig_h id.orig_p id.resp_h id.resp_p status country #types time addr port addr port string string -1331256472.375609 1.2.3.4 1234 2.3.4.5 80 success unknown -1331256472.375609 1.2.3.4 1234 2.3.4.5 80 failure US -1331256472.375609 1.2.3.4 1234 2.3.4.5 80 failure UK -1331256472.375609 1.2.3.4 1234 2.3.4.5 80 success BR -1331256472.375609 1.2.3.4 1234 2.3.4.5 80 failure MX +1342749018.970682 1.2.3.4 1234 2.3.4.5 80 success unknown +1342749018.970682 1.2.3.4 1234 2.3.4.5 80 failure US +1342749018.970682 1.2.3.4 1234 2.3.4.5 80 failure UK +1342749018.970682 1.2.3.4 1234 2.3.4.5 80 success BR +1342749018.970682 1.2.3.4 1234 2.3.4.5 80 failure MX +#close 2012-07-20-01-50-18 diff --git a/testing/btest/Baseline/core.leaks.remote/sender.test.success.log b/testing/btest/Baseline/core.leaks.remote/sender.test.success.log index 1b2ed452a0..f0b26454b4 100644 --- a/testing/btest/Baseline/core.leaks.remote/sender.test.success.log +++ b/testing/btest/Baseline/core.leaks.remote/sender.test.success.log @@ -3,7 +3,9 @@ #empty_field (empty) #unset_field - #path test.success +#open 2012-07-20-01-50-18 #fields t id.orig_h id.orig_p id.resp_h id.resp_p status country #types time addr port addr port string string -1331256472.375609 1.2.3.4 1234 2.3.4.5 80 success unknown -1331256472.375609 1.2.3.4 1234 2.3.4.5 80 success BR +1342749018.970682 1.2.3.4 1234 2.3.4.5 80 success unknown +1342749018.970682 1.2.3.4 1234 2.3.4.5 80 success BR +#close 2012-07-20-01-50-18 diff --git a/testing/btest/core/leaks/basic-cluster.bro b/testing/btest/core/leaks/basic-cluster.bro index f5b40c1104..d9d2f97b1e 100644 --- a/testing/btest/core/leaks/basic-cluster.bro +++ b/testing/btest/core/leaks/basic-cluster.bro @@ -9,7 +9,7 @@ # @TEST-EXEC: sleep 1 # @TEST-EXEC: btest-bg-run worker-1 HEAP_CHECK_DUMP_DIRECTORY=. HEAPCHECK=local BROPATH=$BROPATH:.. CLUSTER_NODE=worker-1 bro -m -r $TRACES/web.trace --pseudo-realtime %INPUT # @TEST-EXEC: btest-bg-run worker-2 HEAP_CHECK_DUMP_DIRECTORY=. HEAPCHECK=local BROPATH=$BROPATH:.. CLUSTER_NODE=worker-2 bro -m -r $TRACES/web.trace --pseudo-realtime %INPUT -# @TEST-EXEC: btest-bg-wait -k 30 +# @TEST-EXEC: btest-bg-wait 40 # @TEST-EXEC: btest-diff manager-1/metrics.log @TEST-START-FILE cluster-layout.bro @@ -40,3 +40,24 @@ event bro_init() &priority=5 Metrics::add_data(TEST_METRIC, [$host=7.2.1.5], 1); } } + +event remote_connection_closed(p: event_peer) + { + terminate(); + } + +@if ( Cluster::local_node_type() == Cluster::MANAGER ) + +global n = 0; + +event Metrics::log_metrics(rec: Metrics::Info) + { + n = n + 1; + if ( n == 3 ) + { + terminate_communication(); + terminate(); + } + } + +@endif diff --git a/testing/btest/core/leaks/remote.bro b/testing/btest/core/leaks/remote.bro index f888d8f6ee..8c8dc73364 100644 --- a/testing/btest/core/leaks/remote.bro +++ b/testing/btest/core/leaks/remote.bro @@ -4,17 +4,19 @@ # # @TEST-REQUIRES: bro --help 2>&1 | grep -q mem-leaks # -# @TEST-EXEC: btest-bg-run sender HEAP_CHECK_DUMP_DIRECTORY=. HEAPCHECK=local bro -m --pseudo-realtime %INPUT ../sender.bro +# @TEST-EXEC: btest-bg-run sender HEAP_CHECK_DUMP_DIRECTORY=. HEAPCHECK=local bro -b -m --pseudo-realtime %INPUT ../sender.bro # @TEST-EXEC: sleep 1 -# @TEST-EXEC: btest-bg-run receiver HEAP_CHECK_DUMP_DIRECTORY=. HEAPCHECK=local bro -m --pseudo-realtime %INPUT ../receiver.bro +# @TEST-EXEC: btest-bg-run receiver HEAP_CHECK_DUMP_DIRECTORY=. HEAPCHECK=local bro -b -m --pseudo-realtime %INPUT ../receiver.bro # @TEST-EXEC: sleep 1 -# @TEST-EXEC: btest-bg-wait -k 10 +# @TEST-EXEC: btest-bg-wait 30 # @TEST-EXEC: btest-diff sender/test.log # @TEST-EXEC: btest-diff sender/test.failure.log # @TEST-EXEC: btest-diff sender/test.success.log -# @TEST-EXEC: cmp receiver/test.log sender/test.log -# @TEST-EXEC: cmp receiver/test.failure.log sender/test.failure.log -# @TEST-EXEC: cmp receiver/test.success.log sender/test.success.log +# @TEST-EXEC: ( cd sender && for i in *.log; do cat $i | $SCRIPTS/diff-remove-timestamps >c.$i; done ) +# @TEST-EXEC: ( cd receiver && for i in *.log; do cat $i | $SCRIPTS/diff-remove-timestamps >c.$i; done ) +# @TEST-EXEC: cmp receiver/c.test.log sender/c.test.log +# @TEST-EXEC: cmp receiver/c.test.failure.log sender/c.test.failure.log +# @TEST-EXEC: cmp receiver/c.test.success.log sender/c.test.success.log # This is the common part loaded by both sender and receiver. module Test; @@ -43,10 +45,10 @@ event bro_init() @TEST-START-FILE sender.bro -module Test; - @load frameworks/communication/listen +module Test; + function fail(rec: Log): bool { return rec$status != "success"; @@ -68,14 +70,27 @@ event remote_connection_handshake_done(p: event_peer) Log::write(Test::LOG, [$t=network_time(), $id=cid, $status="failure", $country="MX"]); disconnect(p); } + +event remote_connection_closed(p: event_peer) + { + terminate(); + } + @TEST-END-FILE @TEST-START-FILE receiver.bro ##### +@load base/frameworks/communication + redef Communication::nodes += { ["foo"] = [$host = 127.0.0.1, $connect=T, $request_logs=T] }; +event remote_connection_closed(p: event_peer) + { + terminate(); + } + @TEST-END-FILE diff --git a/testing/btest/scripts/base/frameworks/logging/remote.bro b/testing/btest/scripts/base/frameworks/logging/remote.bro index 48683148f5..ba577cc92b 100644 --- a/testing/btest/scripts/base/frameworks/logging/remote.bro +++ b/testing/btest/scripts/base/frameworks/logging/remote.bro @@ -1,10 +1,10 @@ # @TEST-SERIALIZE: comm # -# @TEST-EXEC: btest-bg-run sender bro --pseudo-realtime %INPUT ../sender.bro +# @TEST-EXEC: btest-bg-run sender bro -b --pseudo-realtime %INPUT ../sender.bro # @TEST-EXEC: sleep 1 -# @TEST-EXEC: btest-bg-run receiver bro --pseudo-realtime %INPUT ../receiver.bro +# @TEST-EXEC: btest-bg-run receiver bro -b --pseudo-realtime %INPUT ../receiver.bro # @TEST-EXEC: sleep 1 -# @TEST-EXEC: btest-bg-wait -k 10 +# @TEST-EXEC: btest-bg-wait 15 # @TEST-EXEC: btest-diff sender/test.log # @TEST-EXEC: btest-diff sender/test.failure.log # @TEST-EXEC: btest-diff sender/test.success.log @@ -41,10 +41,10 @@ event bro_init() @TEST-START-FILE sender.bro -module Test; - @load frameworks/communication/listen +module Test; + function fail(rec: Log): bool { return rec$status != "success"; @@ -66,14 +66,27 @@ event remote_connection_handshake_done(p: event_peer) Log::write(Test::LOG, [$t=network_time(), $id=cid, $status="failure", $country="MX"]); disconnect(p); } + +event remote_connection_closed(p: event_peer) + { + terminate(); + } + @TEST-END-FILE @TEST-START-FILE receiver.bro ##### +@load base/frameworks/communication + redef Communication::nodes += { ["foo"] = [$host = 127.0.0.1, $connect=T, $request_logs=T] }; +event remote_connection_closed(p: event_peer) + { + terminate(); + } + @TEST-END-FILE diff --git a/testing/btest/scripts/base/frameworks/metrics/basic-cluster.bro b/testing/btest/scripts/base/frameworks/metrics/basic-cluster.bro index 09479b7a2f..4aa1afa96f 100644 --- a/testing/btest/scripts/base/frameworks/metrics/basic-cluster.bro +++ b/testing/btest/scripts/base/frameworks/metrics/basic-cluster.bro @@ -5,7 +5,7 @@ # @TEST-EXEC: sleep 1 # @TEST-EXEC: btest-bg-run worker-1 BROPATH=$BROPATH:.. CLUSTER_NODE=worker-1 bro %INPUT # @TEST-EXEC: btest-bg-run worker-2 BROPATH=$BROPATH:.. CLUSTER_NODE=worker-2 bro %INPUT -# @TEST-EXEC: btest-bg-wait -k 10 +# @TEST-EXEC: btest-bg-wait 20 # @TEST-EXEC: btest-diff manager-1/metrics.log @TEST-START-FILE cluster-layout.bro @@ -36,3 +36,24 @@ event bro_init() &priority=5 Metrics::add_data(TEST_METRIC, [$host=7.2.1.5], 1); } } + +event remote_connection_closed(p: event_peer) + { + terminate(); + } + +@if ( Cluster::local_node_type() == Cluster::MANAGER ) + +global n = 0; + +event Metrics::log_metrics(rec: Metrics::Info) + { + n = n + 1; + if ( n == 3 ) + { + terminate_communication(); + terminate(); + } + } + +@endif diff --git a/testing/btest/scripts/base/frameworks/metrics/cluster-intermediate-update.bro b/testing/btest/scripts/base/frameworks/metrics/cluster-intermediate-update.bro index 654e42976a..db2c7e9f5d 100644 --- a/testing/btest/scripts/base/frameworks/metrics/cluster-intermediate-update.bro +++ b/testing/btest/scripts/base/frameworks/metrics/cluster-intermediate-update.bro @@ -5,7 +5,7 @@ # @TEST-EXEC: sleep 1 # @TEST-EXEC: btest-bg-run worker-1 BROPATH=$BROPATH:.. CLUSTER_NODE=worker-1 bro %INPUT # @TEST-EXEC: btest-bg-run worker-2 BROPATH=$BROPATH:.. CLUSTER_NODE=worker-2 bro %INPUT -# @TEST-EXEC: btest-bg-wait -k 10 +# @TEST-EXEC: btest-bg-wait 20 # @TEST-EXEC: btest-diff manager-1/notice.log @TEST-START-FILE cluster-layout.bro @@ -37,6 +37,21 @@ event bro_init() &priority=5 $log=T]); } +event remote_connection_closed(p: event_peer) + { + terminate(); + } + +@if ( Cluster::local_node_type() == Cluster::MANAGER ) + +event Notice::log_notice(rec: Notice::Info) + { + terminate_communication(); + terminate(); + } + +@endif + @if ( Cluster::local_node_type() == Cluster::WORKER ) event do_metrics(i: count) From 907c92e1ccd692023ea305fa9e1acba5f4819aa9 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Fri, 17 Aug 2012 15:22:51 -0500 Subject: [PATCH 06/11] Fix mime type diff canonifier to also skip mime_desc columns In particular, the ftp.log baseline in the new ipv6 test in bro-testing was failign on various platforms because of this. --- testing/scripts/diff-remove-mime-types | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/testing/scripts/diff-remove-mime-types b/testing/scripts/diff-remove-mime-types index fb447a9989..b8cc3d1e6d 100755 --- a/testing/scripts/diff-remove-mime-types +++ b/testing/scripts/diff-remove-mime-types @@ -3,20 +3,27 @@ # A diff canonifier that removes all MIME types because libmagic output # can differ between installations. -BEGIN { FS="\t"; OFS="\t"; column = -1; } +BEGIN { FS="\t"; OFS="\t"; type_col = -1; desc_col = -1 } /^#fields/ { for ( i = 2; i < NF; ++i ) + { if ( $i == "mime_type" ) - column = i-1; + type_col = i-1; + if ( $i == "mime_desc" ) + desc_col = i-1; + } } -column >= 0 { - if ( $column != "-" ) +function remove_mime (n) { + if ( n >= 0 && $n != "-" ) # Mark that it's set, but ignore content. - $column = "+"; + $n = "+" } +remove_mime(type_col) +remove_mime(desc_col) + { print; } From f201a9f1a7f52329f1c8db35ab46dbfa50f0bda4 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Fri, 17 Aug 2012 17:27:02 -0500 Subject: [PATCH 07/11] Fix portability of printing to files returned by open("/dev/stderr"). The BroFile ctor now wraps /dev/std{in,out,err} string arguments into the actual FILE* provided by stdio.h because use of the former directly isn't POSIX compliant and led to subtle differences that broke unit tests on certain platforms (e.g. OS X redirection of stderr behavior started differing from Linux). The BroFile (un)serialization methods already did this kind of logic, so adding it in the ctor also should make things more consistent. Some of the reporter-related unit tests looked like they were missing output because of this, and the coverage test for bare-mode errors needed tweaking to branch on whether or not libcurl was available (since the error output differs when elasticsearch isn't there). --- src/File.cc | 14 ++++++++++++-- .../Baseline/core.reporter-error-in-handler/output | 4 ++-- .../Baseline/core.reporter-runtime-error/output | 3 ++- .../btest/Baseline/core.reporter/logger-test.log | 12 ++++++------ testing/btest/Baseline/core.reporter/output | 11 +++++++---- .../unique_errors_no_elasticsearch | 1 + testing/btest/coverage/bare-mode-errors.test | 3 ++- 7 files changed, 32 insertions(+), 16 deletions(-) create mode 100644 testing/btest/Baseline/coverage.bare-mode-errors/unique_errors_no_elasticsearch diff --git a/src/File.cc b/src/File.cc index 20e845c09f..20ab2e1013 100644 --- a/src/File.cc +++ b/src/File.cc @@ -138,11 +138,21 @@ BroFile::BroFile(FILE* arg_f, const char* arg_name, const char* arg_access) BroFile::BroFile(const char* arg_name, const char* arg_access, BroType* arg_t) { Init(); - + f = 0; name = copy_string(arg_name); access = copy_string(arg_access); t = arg_t ? arg_t : base_type(TYPE_STRING); - if ( ! Open() ) + + if ( streq(name, "/dev/stdin") ) + f = stdin; + else if ( streq(name, "/dev/stdout") ) + f = stdout; + else if ( streq(name, "/dev/stderr") ) + f = stderr; + + if ( f ) + is_open = 1; + else if ( ! Open() ) { reporter->Error("cannot open %s: %s", name, strerror(errno)); is_open = 0; diff --git a/testing/btest/Baseline/core.reporter-error-in-handler/output b/testing/btest/Baseline/core.reporter-error-in-handler/output index 190631f4d1..b20b1b2292 100644 --- a/testing/btest/Baseline/core.reporter-error-in-handler/output +++ b/testing/btest/Baseline/core.reporter-error-in-handler/output @@ -1,3 +1,3 @@ -ERROR: no such index (a[1]) (/da/home/robin/bro/master/testing/btest/.tmp/core.reporter-error-in-handler/reporter-error-in-handler.bro, line 28) - +error in /home/jsiwek/bro/testing/btest/.tmp/core.reporter-error-in-handler/reporter-error-in-handler.bro, line 22: no such index (a[2]) +ERROR: no such index (a[1]) (/home/jsiwek/bro/testing/btest/.tmp/core.reporter-error-in-handler/reporter-error-in-handler.bro, line 28) 1st error printed on script level diff --git a/testing/btest/Baseline/core.reporter-runtime-error/output b/testing/btest/Baseline/core.reporter-runtime-error/output index 94f7860cb4..5a03f5feb2 100644 --- a/testing/btest/Baseline/core.reporter-runtime-error/output +++ b/testing/btest/Baseline/core.reporter-runtime-error/output @@ -1 +1,2 @@ -ERROR: no such index (a[2]) (/da/home/robin/bro/master/testing/btest/.tmp/core.reporter-runtime-error/reporter-runtime-error.bro, line 9) +error in /home/jsiwek/bro/testing/btest/.tmp/core.reporter-runtime-error/reporter-runtime-error.bro, line 12: no such index (a[1]) +ERROR: no such index (a[2]) (/home/jsiwek/bro/testing/btest/.tmp/core.reporter-runtime-error/reporter-runtime-error.bro, line 9) diff --git a/testing/btest/Baseline/core.reporter/logger-test.log b/testing/btest/Baseline/core.reporter/logger-test.log index 6f7ba1d8c7..5afd904b63 100644 --- a/testing/btest/Baseline/core.reporter/logger-test.log +++ b/testing/btest/Baseline/core.reporter/logger-test.log @@ -1,6 +1,6 @@ -reporter_info|init test-info|/da/home/robin/bro/master/testing/btest/.tmp/core.reporter/reporter.bro, line 8|0.000000 -reporter_warning|init test-warning|/da/home/robin/bro/master/testing/btest/.tmp/core.reporter/reporter.bro, line 9|0.000000 -reporter_error|init test-error|/da/home/robin/bro/master/testing/btest/.tmp/core.reporter/reporter.bro, line 10|0.000000 -reporter_info|done test-info|/da/home/robin/bro/master/testing/btest/.tmp/core.reporter/reporter.bro, line 15|0.000000 -reporter_warning|done test-warning|/da/home/robin/bro/master/testing/btest/.tmp/core.reporter/reporter.bro, line 16|0.000000 -reporter_error|done test-error|/da/home/robin/bro/master/testing/btest/.tmp/core.reporter/reporter.bro, line 17|0.000000 +reporter_info|init test-info|/home/jsiwek/bro/testing/btest/.tmp/core.reporter/reporter.bro, line 8|0.000000 +reporter_warning|init test-warning|/home/jsiwek/bro/testing/btest/.tmp/core.reporter/reporter.bro, line 9|0.000000 +reporter_error|init test-error|/home/jsiwek/bro/testing/btest/.tmp/core.reporter/reporter.bro, line 10|0.000000 +reporter_info|done test-info|/home/jsiwek/bro/testing/btest/.tmp/core.reporter/reporter.bro, line 15|0.000000 +reporter_warning|done test-warning|/home/jsiwek/bro/testing/btest/.tmp/core.reporter/reporter.bro, line 16|0.000000 +reporter_error|done test-error|/home/jsiwek/bro/testing/btest/.tmp/core.reporter/reporter.bro, line 17|0.000000 diff --git a/testing/btest/Baseline/core.reporter/output b/testing/btest/Baseline/core.reporter/output index b4f89bad2f..f2c59259c2 100644 --- a/testing/btest/Baseline/core.reporter/output +++ b/testing/btest/Baseline/core.reporter/output @@ -1,4 +1,7 @@ -WARNING: init test-warning (/da/home/robin/bro/master/testing/btest/.tmp/core.reporter/reporter.bro, line 9) -ERROR: init test-error (/da/home/robin/bro/master/testing/btest/.tmp/core.reporter/reporter.bro, line 10) -WARNING: done test-warning (/da/home/robin/bro/master/testing/btest/.tmp/core.reporter/reporter.bro, line 16) -ERROR: done test-error (/da/home/robin/bro/master/testing/btest/.tmp/core.reporter/reporter.bro, line 17) +/home/jsiwek/bro/testing/btest/.tmp/core.reporter/reporter.bro, line 52: pre test-info +warning in /home/jsiwek/bro/testing/btest/.tmp/core.reporter/reporter.bro, line 53: pre test-warning +error in /home/jsiwek/bro/testing/btest/.tmp/core.reporter/reporter.bro, line 54: pre test-error +WARNING: init test-warning (/home/jsiwek/bro/testing/btest/.tmp/core.reporter/reporter.bro, line 9) +ERROR: init test-error (/home/jsiwek/bro/testing/btest/.tmp/core.reporter/reporter.bro, line 10) +WARNING: done test-warning (/home/jsiwek/bro/testing/btest/.tmp/core.reporter/reporter.bro, line 16) +ERROR: done test-error (/home/jsiwek/bro/testing/btest/.tmp/core.reporter/reporter.bro, line 17) diff --git a/testing/btest/Baseline/coverage.bare-mode-errors/unique_errors_no_elasticsearch b/testing/btest/Baseline/coverage.bare-mode-errors/unique_errors_no_elasticsearch new file mode 100644 index 0000000000..e95f88e74b --- /dev/null +++ b/testing/btest/Baseline/coverage.bare-mode-errors/unique_errors_no_elasticsearch @@ -0,0 +1 @@ +error: unknown writer type requested diff --git a/testing/btest/coverage/bare-mode-errors.test b/testing/btest/coverage/bare-mode-errors.test index 21e7d4f4a9..7084d74e83 100644 --- a/testing/btest/coverage/bare-mode-errors.test +++ b/testing/btest/coverage/bare-mode-errors.test @@ -10,4 +10,5 @@ # @TEST-EXEC: test -d $DIST/scripts # @TEST-EXEC: for script in `find $DIST/scripts -name \*\.bro -not -path '*/site/*'`; do echo $script; if echo "$script" | egrep -q 'communication/listen|controllee'; then rm -rf load_attempt .bgprocs; btest-bg-run load_attempt bro -b $script; btest-bg-wait -k 2; cat load_attempt/.stderr >>allerrors; else bro -b $script 2>>allerrors; fi done || exit 0 # @TEST-EXEC: cat allerrors | grep -v "received termination signal" | sort | uniq > unique_errors -# @TEST-EXEC: btest-diff unique_errors +# @TEST-EXEC: if [ $(grep -c CURL_INCLUDE_DIR-NOTFOUND $BUILD/CMakeCache.txt) -ne 0 ]; then cp unique_errors unique_errors_no_elasticsearch; fi +# @TEST-EXEC: if [ $(grep -c CURL_INCLUDE_DIR-NOTFOUND $BUILD/CMakeCache.txt) -ne 0 ]; then btest-diff unique_errors_no_elasticsearch; else btest-diff unique_errors; fi From 0dbf2f18fa679a1231f957e474a1bb1bb59e5042 Mon Sep 17 00:00:00 2001 From: Seth Hall Date: Mon, 20 Aug 2012 13:26:17 -0400 Subject: [PATCH 08/11] Add the Stream record to Log:active_streams to make more dynamic logging possible. --- scripts/base/frameworks/logging/main.bro | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/base/frameworks/logging/main.bro b/scripts/base/frameworks/logging/main.bro index ccc65ddf67..bed76a1ae5 100644 --- a/scripts/base/frameworks/logging/main.bro +++ b/scripts/base/frameworks/logging/main.bro @@ -329,9 +329,9 @@ export { global run_rotation_postprocessor_cmd: function(info: RotationInfo, npath: string) : bool; ## The streams which are currently active and not disabled. - ## This set is not meant to be modified by users! Only use it for + ## This table is not meant to be modified by users! Only use it for ## examining which streams are active. - global active_streams: set[ID] = set(); + global active_streams: table[ID] of Stream = table(); } # We keep a script-level copy of all filters so that we can manipulate them. @@ -417,7 +417,7 @@ function create_stream(id: ID, stream: Stream) : bool if ( ! __create_stream(id, stream) ) return F; - add active_streams[id]; + active_streams[id] = stream; return add_default_filter(id); } From 434d6a84d8bb73cef7704799fcd391375bba5862 Mon Sep 17 00:00:00 2001 From: Robin Sommer Date: Tue, 21 Aug 2012 08:32:42 -0700 Subject: [PATCH 09/11] Linking ES docs into logging document. --- CHANGES | 4 ++++ VERSION | 2 +- doc/logging.rst | 1 + 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index 7b381b5c5d..b6225097db 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,8 @@ +2.1-beta-28 | 2012-08-21 08:32:42 -0700 + + * Linking ES docs into logging document. (Robin Sommer) + 2.1-beta-27 | 2012-08-20 20:06:20 -0700 * Add the Stream record to Log:active_streams to make more dynamic diff --git a/VERSION b/VERSION index e82f524ce7..c403b714f8 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.1-beta-27 +2.1-beta-28 diff --git a/doc/logging.rst b/doc/logging.rst index cc6cb1e54d..7fb4205b9a 100644 --- a/doc/logging.rst +++ b/doc/logging.rst @@ -383,3 +383,4 @@ Bro supports the following output formats other than ASCII: :maxdepth: 1 logging-dataseries + logging-elasticsearch From 06b7379bc3f112faab220d59663844d449add3a8 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Tue, 21 Aug 2012 14:54:57 -0500 Subject: [PATCH 10/11] Ignore small mem leak every rotation interval for dataseries logs. Not sure if more can be done to work around it, but reported to dataseries devs here: https://github.com/dataseries/DataSeries/issues/1 The core/leaks/dataseries-rotate.bro unit test fails without this. --- src/logging/writers/DataSeries.cc | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/logging/writers/DataSeries.cc b/src/logging/writers/DataSeries.cc index 7d3053e341..bc5a82ec54 100644 --- a/src/logging/writers/DataSeries.cc +++ b/src/logging/writers/DataSeries.cc @@ -243,8 +243,25 @@ bool DataSeries::OpenLog(string path) log_file->writeExtentLibrary(log_types); for( size_t i = 0; i < schema_list.size(); ++i ) - extents.insert(std::make_pair(schema_list[i].field_name, - GeneralField::create(log_series, schema_list[i].field_name))); + { + string fn = schema_list[i].field_name; + GeneralField* gf = 0; +#ifdef USE_PERFTOOLS_DEBUG + { + // GeneralField isn't cleaning up some results of xml parsing, reported + // here: https://github.com/dataseries/DataSeries/issues/1 + // Ignore for now to make leak tests pass. There's confidence that + // we do clean up the GeneralField* since the ExtentSeries dtor for + // member log_series would trigger an assert if dynamically allocated + // fields aren't deleted beforehand. + HeapLeakChecker::Disabler disabler; +#endif + gf = GeneralField::create(log_series, fn); +#ifdef USE_PERFTOOLS_DEBUG + } +#endif + extents.insert(std::make_pair(fn, gf)); + } if ( ds_extent_size < ROW_MIN ) { From bb4b68946f9530b119a0144191e8e72a27896b9d Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Tue, 21 Aug 2012 15:22:54 -0500 Subject: [PATCH 11/11] Tweak to rotate-custom.bro unit test. This one would fail intermittently in the cases where log files were opened or closed on a different second of the time of day from each other since the "out" baseline contains only a single "#open" and "#close" tag (indicating all logs opened/closed on same second of time of day). Piping aggregated log output through the timestamp canonifier before `uniq` makes it so "#open" and "#close" tags for different seconds of the time of day are reduced to a single one. --- testing/btest/scripts/base/frameworks/logging/rotate-custom.bro | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/btest/scripts/base/frameworks/logging/rotate-custom.bro b/testing/btest/scripts/base/frameworks/logging/rotate-custom.bro index 07fc8cef7c..c0f0ef8643 100644 --- a/testing/btest/scripts/base/frameworks/logging/rotate-custom.bro +++ b/testing/btest/scripts/base/frameworks/logging/rotate-custom.bro @@ -1,7 +1,7 @@ # # @TEST-EXEC: bro -b -r ${TRACES}/rotation.trace %INPUT | egrep "test|test2" | sort >out.tmp # @TEST-EXEC: cat out.tmp pp.log | sort >out -# @TEST-EXEC: for i in `ls test*.log | sort`; do printf '> %s\n' $i; cat $i; done | sort | uniq >>out +# @TEST-EXEC: for i in `ls test*.log | sort`; do printf '> %s\n' $i; cat $i; done | sort | $SCRIPTS/diff-remove-timestamps | uniq >>out # @TEST-EXEC: btest-diff out # @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-sort btest-diff .stderr