message parser fixes and testbench enhancements

- improved testbench to contain samples for totally malformed messages
  which miss parts of the message content
- bugfix: some malformed messages could lead to a missing LF inside files
  or some other missing parts of the template content.
- bugfix: if a message ended immediately with a hostname, the hostname
  was mistakenly interpreted as TAG, and localhost be used as hostname

5 files changed, 64 insertions, 11 deletions

diff --git a/ChangeLog b/ChangeLog
index d84a0d1c..99f45851 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+- improved testbench to contain samples for totally malformed messages
+  which miss parts of the message content
+- bugfix: some malformed messages could lead to a missing LF inside files
+  or some other missing parts of the template content.
+- bugfix: if a message ended immediately with a hostname, the hostname
+  was mistakenly interpreted as TAG, and localhost be used as hostname
 - bugfix: message without MSG part could case a segfault
   [backported from v5 commit 98d1ed504ec001728955a5bcd7916f64cd85f39f]
   This actually was a "recent" regression, but I did not realize that it
diff --git a/runtime/datetime.c b/runtime/datetime.c
index 6160bd7c..7b0d8d11 100644
--- a/runtime/datetime.c
+++ b/runtime/datetime.c
@@ -291,11 +291,11 @@ ParseTIMESTAMP3339(struct syslogTime *pTime, uchar** ppszTS, int *pLenStr)
 	}
 
 	/* OK, we actually have a 3339 timestamp, so let's indicated this */
-	if(lenStr > 0 && *pszTS == ' ') {
+	if(lenStr > 0) {
+		if(*pszTS != ' ') /* if it is not a space, it can not be a "good" time - 2010-02-22 rgerhards */
+			ABORT_FINALIZE(RS_RET_INVLD_TIME);
+		++pszTS; /* just skip past it */
 		--lenStr;
-		++pszTS;
-	} else {
-		ABORT_FINALIZE(RS_RET_INVLD_TIME);
 	}
 
 	/* we had success, so update parse pointer and caller-provided timestamp */
@@ -510,6 +510,7 @@ ParseTIMESTAMP3164(struct syslogTime *pTime, uchar** ppszTS, int *pLenStr)
 
 	if(lenStr == 0 || *pszTS++ != ' ')
 		ABORT_FINALIZE(RS_RET_INVLD_TIME);
+	--lenStr;
 
 	/* we accept a slightly malformed timestamp when receiving. This is
 	 * we accept one-digit days
@@ -565,7 +566,13 @@ ParseTIMESTAMP3164(struct syslogTime *pTime, uchar** ppszTS, int *pLenStr)
 	 * invalid format, it occurs frequently enough (e.g. with Cisco devices)
 	 * to permit it as a valid case. -- rgerhards, 2008-09-12
 	 */
-	if(lenStr == 0 || *pszTS++ == ':') {
+	if(lenStr > 0 && *pszTS == ':') {
+		++pszTS; /* just skip past it */
+		--lenStr;
+	}
+	if(lenStr > 0) {
+		if(*pszTS != ' ') /* if it is not a space, it can not be a "good" time - 2010-02-22 rgerhards */
+			ABORT_FINALIZE(RS_RET_INVLD_TIME);
 		++pszTS; /* just skip past it */
 		--lenStr;
 	}
diff --git a/runtime/rsyslog.h b/runtime/rsyslog.h
index 27bea6bc..0f489a7f 100644
--- a/runtime/rsyslog.h
+++ b/runtime/rsyslog.h
@@ -35,7 +35,7 @@
  * value to the fixed size of the message object.
  */
 #define CONF_TAG_MAXSIZE		512	/* a value that is deemed far too large for any valid TAG */
-#define CONF_TAG_HOSTNAME		512	/* a value that is deemed far too large for any valid HOSTNAME */
+#define CONF_HOSTNAME_MAXSIZE		512	/* a value that is deemed far too large for any valid HOSTNAME */
 #define CONF_RAWMSG_BUFSIZE		101
 #define CONF_TAG_BUFSIZE		32
 #define CONF_HOSTNAME_BUFSIZE		32
diff --git a/tests/testsuites/weird.parse1 b/tests/testsuites/weird.parse1
index bc898fd4..e8b90c74 100644
--- a/tests/testsuites/weird.parse1
+++ b/tests/testsuites/weird.parse1
@@ -2,4 +2,36 @@
 # some other deliberately generated. The main point is that they
 # should not cause an abort...
 <14>Aug 30 23:00:05 X4711 AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
-14,user,info,Aug 30 23:00:05,X4711,AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA,AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+14,user,info,Aug 30 23:00:05,X4711,AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA,AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA,
+# important: the following line has a SP at the end of the line!
+<14>Aug 30 23:00:05 X4711 
+14,user,info,Aug 30 23:00:05,X4711,,,
+# and this one NOT
+<14>Aug 30 23:00:05 X4711
+14,user,info,Aug 30 23:00:05,X4711,,,
+# there is a SP at the end of the line
+<14>Aug 30 23:00:05 
+14,user,info,Aug 30 23:00:05,localhost,,,
+# and here is no SP at the end of the line
+<14>Aug 30 23:00:05
+14,user,info,Aug 30 23:00:05,localhost,,,
+# unfortunately, I can not test missing dates with this test suite, because
+# we would have the current date in the response, which we can not check against
+#
+# and now the same tests with RFC3339 data - this can make a difference
+# as a different date parser is involved.
+# 
+<14>2010-08-30T23:00:05Z X4711 AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+14,user,info,Aug 30 23:00:05,X4711,AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA,AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA,
+# important: the following line has a SP at the end of the line!
+<14>2010-08-30T23:00:05Z X4711 
+14,user,info,Aug 30 23:00:05,X4711,,,
+# and this one NOT
+<14>2010-08-30T23:00:05Z X4711
+14,user,info,Aug 30 23:00:05,X4711,,,
+# there is a SP at the end of the line
+<14>2010-08-30T23:00:05Z 
+14,user,info,Aug 30 23:00:05,localhost,,,
+# and here is no SP at the end of the line
+<14>2010-08-30T23:00:05Z
+14,user,info,Aug 30 23:00:05,localhost,,,
diff --git a/tools/syslogd.c b/tools/syslogd.c
index 3e6d51d3..caab1691 100644
--- a/tools/syslogd.c
+++ b/tools/syslogd.c
@@ -1192,12 +1192,12 @@ int parseLegacySyslogMsg(msg_t *pMsg, int flags)
 	int bTAGCharDetected;
 	int i;	/* general index for parsing */
 	uchar bufParseTAG[CONF_TAG_MAXSIZE];
-	uchar bufParseHOSTNAME[CONF_TAG_HOSTNAME];
+	uchar bufParseHOSTNAME[CONF_HOSTNAME_MAXSIZE];
 	BEGINfunc
 
 	assert(pMsg != NULL);
 	assert(pMsg->pszRawMsg != NULL);
-	lenMsg = pMsg->iLenRawMsg - (pMsg->offAfterPRI + 1);
+	lenMsg = pMsg->iLenRawMsg - pMsg->offAfterPRI; /* note: offAfterPRI is already the number of PRI chars (do not add one!) */
 	p2parse = pMsg->pszRawMsg + pMsg->offAfterPRI; /* point to start of text, after PRI */
 
 	/* Check to see if msg contains a timestamp. We start by assuming
@@ -1254,12 +1254,20 @@ int parseLegacySyslogMsg(msg_t *pMsg, int flags)
 		if(lenMsg > 0 && flags & PARSE_HOSTNAME) {
 			i = 0;
 			while(i < lenMsg && (isalnum(p2parse[i]) || p2parse[i] == '.' || p2parse[i] == '.'
-				|| p2parse[i] == '_' || p2parse[i] == '-') && i < CONF_TAG_MAXSIZE) {
+				|| p2parse[i] == '_' || p2parse[i] == '-') && i < (CONF_HOSTNAME_MAXSIZE - 1)) {
 				bufParseHOSTNAME[i] = p2parse[i];
 				++i;
 			}
 
-			if(i > 0 && p2parse[i] == ' ' && isalnum(p2parse[i-1])) {
+			if(i == lenMsg) {
+				/* we have a message that is empty immediately after the hostname,
+				 * but the hostname thus is valid! -- rgerhards, 2010-02-22
+				 */
+				p2parse += i;
+				lenMsg -= i;
+				bufParseHOSTNAME[i] = '\0';
+				MsgSetHOSTNAME(pMsg, bufParseHOSTNAME, i);
+			} else if(i > 0 && p2parse[i] == ' ' && isalnum(p2parse[i-1])) {
 				/* we got a hostname! */
 				p2parse += i + 1; /* "eat" it (including SP delimiter) */
 				lenMsg -= i + 1;