From bcbe749d8061255225978ddcf9381ada6d1cabf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kriszti=C3=A1n=20L=C3=B6ki?= Date: Mon, 26 Feb 2018 16:35:54 +0100 Subject: [PATCH 1/2] Print the mismatching characters to ease debugging --- src/StreamCore.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/StreamCore.cc b/src/StreamCore.cc index 1943bf1..4154fa6 100644 --- a/src/StreamCore.cc +++ b/src/StreamCore.cc @@ -1364,13 +1364,15 @@ normal_format: { int i = 0; while (commandIndex[i] >= ' ') i++; - error("%s: Input \"%s%s\" mismatch after %ld byte%s\n", + error("%s: Input \"%s%s\" mismatch after %ld byte%s: %c != %c\n", name(), consumedInput > 10 ? "..." : "", inputLine.expand(consumedInput > 10 ? consumedInput-10 : 0,20)(), consumedInput, - consumedInput==1 ? "" : "s"); + consumedInput==1 ? "" : "s", + command, + inputLine[consumedInput]); #ifndef NO_TEMPORARY error("%s: got \"%s\" where \"%s\" was expected\n", From da174b099e09086d33ef0d9d51908f0739ec9d23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kriszti=C3=A1n=20L=C3=B6ki?= Date: Tue, 27 Feb 2018 15:19:55 +0100 Subject: [PATCH 2/2] Do not encode formats during recursion Encoding a format involves replacing it with binary encoded information that contains a NULL byte. This null byte terminates the search for subsequent formats. This means that if the encoding is done when the recursion depth is greater than 0, no format will be recognized as such when the recursion depth decreases. A real life manifestation of this bug: wont_work { prefix = "@" "%3c" "ACK"; in $prefix "%s"; } The "%s" will be interpreted as literal '%' followed by 's'. What happens here is that the variable $prefix will be replaced (by recursively calling compileString()) with "@%3cACK" in the first step and in the second step the format conversion will be replaced with a binary encoding (sg like "<03>3c<00>..."). That NUL byte will cause the search for '%' (in the parent compileString()) to stop, leaving the "%s" intact. The fix is simple; defer the format encode until everything else is done. To preserve binary compatibility a new private function is introduced instead of changing the signature of the exising one. --- src/StreamProtocol.cc | 12 ++++++++++-- src/StreamProtocol.h | 2 ++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/StreamProtocol.cc b/src/StreamProtocol.cc index 6f97db8..f43f8c2 100644 --- a/src/StreamProtocol.cc +++ b/src/StreamProtocol.cc @@ -1058,6 +1058,14 @@ compileNumber(unsigned long& number, const char*& source, unsigned long max) bool StreamProtocolParser::Protocol:: compileString(StreamBuffer& buffer, const char*& source, FormatType formatType, Client* client, int quoted) +{ + return compileStringInternal(buffer, source, formatType, client, quoted, 0); +} + + +bool StreamProtocolParser::Protocol:: +compileStringInternal(StreamBuffer& buffer, const char*& source, + FormatType formatType, Client* client, int quoted, int recursionDepth) { bool escaped = false; int newline = 0; @@ -1085,7 +1093,7 @@ compileString(StreamBuffer& buffer, const char*& source, // compile all formats in this line // We do this here after all variables in this line // have been replaced and after string has been coded. - if (formatType != NoFormat) + if (recursionDepth == 0 && formatType != NoFormat) { int nformats=0; char c; @@ -1250,7 +1258,7 @@ compileString(StreamBuffer& buffer, const char*& source, source += strlen(source)+1+sizeof(int); p = value(); int saveline = line; - if (!compileString(buffer, p, formatType, client)) + if (!compileStringInternal(buffer, p, formatType, client, false, recursionDepth + 1)) return false; line = saveline; continue; diff --git a/src/StreamProtocol.h b/src/StreamProtocol.h index b3c1231..91f9183 100644 --- a/src/StreamProtocol.h +++ b/src/StreamProtocol.h @@ -59,6 +59,8 @@ public: bool compileCommands(StreamBuffer&, const char*& source, Client*); bool replaceVariable(StreamBuffer&, const char* varname); const Variable* getVariable(const char* name); + bool compileStringInternal(StreamBuffer& buffer, const char*& source, + FormatType formatType, Client*, int quoted, int recursionDepth); public: