From e3cc7aa48f04e1ea474bb5f67a88042c68505e19 Mon Sep 17 00:00:00 2001 From: Aaron Eppert Date: Wed, 18 Mar 2015 00:28:19 -0400 Subject: [PATCH 1/6] Seems to fix a case where an entry in the table may be null on insert. #0 0x0000000000713b87 in Dictionary::Insert (this=0x1339840, new_entry=0xb18a9d0, copy_key=0) at /root/psdev/bro/src/Dict.cc:419 #1 0x00000000007130b0 in Dictionary::Insert (this=0x1339840, key=0xa23f6d0, key_size=36, hash=658668102, val=0x67fde40, copy_key=0) at /root/psdev/bro/src/Dict.cc:158 #2 0x00000000006cb508 in Dictionary::Insert (this=0x1339840, key=0x7ffff4ba81b0, val=0x67fde40) at /root/psdev/bro/src/Dict.h:47 (gdb) print *this $59 = {_vptr.Dictionary = 0xaf7810, tbl = 0x215b400, num_buckets = 1347, num_entries = 3879, max_num_entries = 4042, den_thresh = 3, thresh_entries = 4041, tbl2 = 0x1afcc9e0, num_buckets2 = 2695, num_entries2 = 181, max_num_entries2 = 181, den_thresh2 = 3, thresh_entries2 = 8085, tbl_next_ind = 60, order = 0x133bfb0, delete_func = 0, cookies = { = {entry = 0x133d790, chunk_size = 10, max_entries = 10, num_entries = 0}, }} (gdb) print *tbl $60 = (DictEntryPList *) 0x0 --- src/Dict.cc | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/Dict.cc b/src/Dict.cc index cd7792b539..15ac1b48f7 100644 --- a/src/Dict.cc +++ b/src/Dict.cc @@ -416,13 +416,15 @@ void* Dictionary::Insert(DictEntry* new_entry, int copy_key) { DictEntry* entry = (*chain)[i]; - if ( entry->hash == new_entry->hash && - entry->len == n && - ! memcmp(entry->key, new_entry->key, n) ) - { - void* old_value = entry->value; - entry->value = new_entry->value; - return old_value; + if ( entry ) { + if ( entry->hash == new_entry->hash && + entry->len == n && + ! memcmp(entry->key, new_entry->key, n) ) + { + void* old_value = entry->value; + entry->value = new_entry->value; + return old_value; + } } } } From 2088928fb603d2671d57f5f6a300e3d4df591cb4 Mon Sep 17 00:00:00 2001 From: Aaron Eppert Date: Wed, 18 Mar 2015 11:15:38 -0400 Subject: [PATCH 2/6] A fatal error, especially in DEBUG, should result in a core. This issue is especially helpful in the case of the Val::CONVERTER error and having: "fatal error in : Val::CONVERTER ..." Nebulous error and sans location, it is extremely hard to figure out the culprit. Thus, if Bro is built DEBUG, fatal should provide a core. This subtle change prevents having to change FatalErrors to FatalErrorWithCore everywhere. --- src/Reporter.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Reporter.cc b/src/Reporter.cc index cd1aa09d4c..d138e23b88 100644 --- a/src/Reporter.cc +++ b/src/Reporter.cc @@ -88,7 +88,11 @@ void Reporter::FatalError(const char* fmt, ...) va_end(ap); set_processing_status("TERMINATED", "fatal_error"); +#ifdef DEBUG + abort(); +#else exit(1); +#endif // DEBUG } void Reporter::FatalErrorWithCore(const char* fmt, ...) @@ -393,4 +397,3 @@ void Reporter::DoLog(const char* prefix, EventHandlerPtr event, FILE* out, if ( alloced ) free(alloced); } - From 295dbc3055af7a6bf9d61a0a0b72f66735330a78 Mon Sep 17 00:00:00 2001 From: Aaron Eppert Date: Mon, 26 Oct 2015 17:55:01 -0400 Subject: [PATCH 3/6] Fix for JSON formatter In the event that the first entry in a record is optional AND not present, the serializer will incorrectly add a leading comma. This leading common is invalid JSON and will, more often than not, cause parser failures downstream. --- src/threading/formatters/JSON.cc | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/threading/formatters/JSON.cc b/src/threading/formatters/JSON.cc index 1a2fd84c4a..3e5bfe9391 100644 --- a/src/threading/formatters/JSON.cc +++ b/src/threading/formatters/JSON.cc @@ -35,8 +35,14 @@ bool JSON::Describe(ODesc* desc, int num_fields, const Field* const * fields, const u_char* bytes = desc->Bytes(); int len = desc->Len(); - if ( i > 0 && len > 0 && bytes[len-1] != ',' && vals[i]->present ) - desc->AddRaw(","); + if ( i > 0 && len > 0 && bytes[len-1] != ',' && vals[i]->present ) { + // Issue if the first value of a record is optional AND not present + // then an empty json field will be produced, which is invalid. + // - ANE - 10/26/2015 + if (len > 1 && bytes[len-1] != '{') { + desc->AddRaw(","); + } + } if ( ! Describe(desc, vals[i], fields[i]->name) ) return false; From 1b09734b31c767c70d7e909ca203d4d99caf74f3 Mon Sep 17 00:00:00 2001 From: Aaron Eppert Date: Mon, 26 Oct 2015 18:06:41 -0400 Subject: [PATCH 4/6] Remove. --- src/Reporter.cc | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Reporter.cc b/src/Reporter.cc index 02dd11adf1..6020b6569c 100644 --- a/src/Reporter.cc +++ b/src/Reporter.cc @@ -88,11 +88,7 @@ void Reporter::FatalError(const char* fmt, ...) va_end(ap); set_processing_status("TERMINATED", "fatal_error"); -#ifdef DEBUG - abort(); -#else exit(1); -#endif // DEBUG } void Reporter::FatalErrorWithCore(const char* fmt, ...) From 053aa40335f881e358c8e76b7494e30ec4432fa2 Mon Sep 17 00:00:00 2001 From: Aaron Eppert Date: Mon, 26 Oct 2015 18:09:38 -0400 Subject: [PATCH 5/6] Remove --- src/Dict.cc | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/Dict.cc b/src/Dict.cc index b773fc54f3..4bf9840b3a 100644 --- a/src/Dict.cc +++ b/src/Dict.cc @@ -416,19 +416,17 @@ void* Dictionary::Insert(DictEntry* new_entry, int copy_key) { DictEntry* entry = (*chain)[i]; - if ( entry ) { - if ( entry->hash == new_entry->hash && - entry->len == n && - ! memcmp(entry->key, new_entry->key, n) ) - { - void* old_value = entry->value; - entry->value = new_entry->value; - return old_value; - } + if ( entry->hash == new_entry->hash && + entry->len == n && + ! memcmp(entry->key, new_entry->key, n) ) + { + void* old_value = entry->value; + entry->value = new_entry->value; + return old_value; } } } - else + else // Create new chain. chain = ttbl[h] = new PList(DictEntry); From 3b027fdebbdd48db385227059edb01bfe6f2e653 Mon Sep 17 00:00:00 2001 From: Aaron Eppert Date: Mon, 26 Oct 2015 18:10:26 -0400 Subject: [PATCH 6/6] Whitespace --- src/Dict.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Dict.cc b/src/Dict.cc index 4bf9840b3a..1d32eccde3 100644 --- a/src/Dict.cc +++ b/src/Dict.cc @@ -426,7 +426,7 @@ void* Dictionary::Insert(DictEntry* new_entry, int copy_key) } } } - else + else // Create new chain. chain = ttbl[h] = new PList(DictEntry);