#1654: Handle entity properties containing double quotation marks (#1688

)

* #1654: Escape and unescape double quotation marks on map load / save.

* #1654: Add issue generators for entity properties with double quotation marks.

* #1654: Replace some raw string literals that trip up VC++.

* #1654: Use std::function instead of template arguments.

* 1654: Add a hack to the map parser to allow paths with unescaped trailing backslashes.

* 1654: Add test for trailing escaped backslashes.

* 1654: Fix treatment of multiple escape chars in StringUtils::unescape.

common/src/IO/MapFileSerializer.cpp

@@ -244,7 +244,9 @@ namespace TrenchBroom {
         }
 
         void MapFileSerializer::doEntityAttribute(const Model::EntityAttribute& attribute) { 
-            std::fprintf(m_stream, "\"%s\" \"%s\"\n", attribute.name().c_str(), attribute.value().c_str());
+            std::fprintf(m_stream, "\"%s\" \"%s\"\n",
+                         escapeEntityAttribute( attribute.name()).c_str(),
+                         escapeEntityAttribute(attribute.value()).c_str());
             ++m_line;
         }
 

common/src/IO/MapStreamSerializer.cpp

@@ -183,7 +183,7 @@ namespace TrenchBroom {
         }
 
         void MapStreamSerializer::doEntityAttribute(const Model::EntityAttribute& attribute) {
-            m_stream << "\"" << attribute.name() << "\" \"" << attribute.value() << "\"\n";
+            m_stream << "\"" << escapeEntityAttribute(attribute.name()) << "\" \"" << escapeEntityAttribute(attribute.value()) << "\"\n";
         }
 
         void MapStreamSerializer::doBeginBrush(const Model::Brush* brush) {

common/src/IO/NodeSerializer.cpp

@@ -192,5 +192,9 @@ namespace TrenchBroom {
             attrs.push_back(Model::EntityAttribute(Model::AttributeNames::GroupId, m_groupIds.getId(group)));
             return attrs;
         }
+
+        String NodeSerializer::escapeEntityAttribute(const String& str) const {
+            return StringUtils::escape(str, "\"", '\\');
+        }
     }
 }

common/src/IO/NodeSerializer.h

@@ -112,6 +112,8 @@ namespace TrenchBroom {
         private:
             Model::EntityAttribute::List layerAttributes(const Model::Layer* layer);
             Model::EntityAttribute::List groupAttributes(const Model::Group* group);
+        protected:
+            String escapeEntityAttribute(const String& str) const;
         private:
             virtual void doBeginFile() = 0;
             virtual void doEndFile() = 0;

common/src/IO/StandardMapParser.cpp

@@ -31,11 +31,11 @@ namespace TrenchBroom {
         }
 
         QuakeMapTokenizer::QuakeMapTokenizer(const char* begin, const char* end) :
-        Tokenizer(begin, end, "", 0),
+        Tokenizer(begin, end, "\"", '\\'),
         m_skipEol(true) {}
 
         QuakeMapTokenizer::QuakeMapTokenizer(const String& str) :
-        Tokenizer(str, "", 0),
+        Tokenizer(str, "\"", '\\'),
         m_skipEol(true) {}
 
         void QuakeMapTokenizer::setSkipEol(bool skipEol) {
@@ -80,7 +80,7 @@ namespace TrenchBroom {
                     case '"': { // quoted string
                         advance();
                         c = curPos();
-                        const char* e = readQuotedString();
+                        const char* e = readQuotedString('"', "\n}");
                         return Token(QuakeMapToken::String, c, e, offset(c), startLine, startColumn);
                     }
                     case '\n':
@@ -263,13 +263,13 @@ namespace TrenchBroom {
         void StandardMapParser::parseEntityAttribute(Model::EntityAttribute::List& attributes, AttributeNames& names, ParserStatus& status) {
             Token token = m_tokenizer.nextToken();
             assert(token.type() == QuakeMapToken::String);
-            const String name = token.data();
+            const String name = m_tokenizer.unescapeString(token.data());
 
             const size_t line = token.line();
             const size_t column = token.column();
 
             expect(QuakeMapToken::String, token = m_tokenizer.nextToken());
-            const String value = token.data();
+            const String value = m_tokenizer.unescapeString(token.data());
 
             if (names.count(name) == 0) {
                 attributes.push_back(Model::EntityAttribute(name, value, NULL));

common/src/IO/Tokenizer.cpp

@@ -69,6 +69,14 @@ namespace TrenchBroom {
             return !eof() && m_escaped && m_escapableChars.find(curChar()) != String::npos;
         }
 
+        String TokenizerState::unescape(const String& str) {
+            return StringUtils::unescape(str, m_escapableChars, m_escapeChar);
+        }
+
+        void TokenizerState::resetEscaped() {
+            m_escaped = false;
+        }
+
         bool TokenizerState::eof() const {
             return eof(m_cur);
         }

common/src/IO/Tokenizer.h

@@ -78,6 +78,8 @@ namespace TrenchBroom {
             size_t column() const;
 
             bool escaped() const;
+            String unescape(const String& str);
+            void resetEscaped();
 
             bool eof() const;
             bool eof(const char* ptr) const;
@@ -173,6 +175,10 @@ namespace TrenchBroom {
                 return String(startPos, static_cast<size_t>(endPos - startPos));
             }
 
+            String unescapeString(const String& str) const {
+                return m_state->unescape(str);
+            }
+
             void reset() {
                 m_state->reset();
             }
@@ -316,10 +322,16 @@ namespace TrenchBroom {
                 return curPos();
             }
 
-            const char* readQuotedString(const char delim = '"') {
-                while (!eof() && (curChar() != delim || isEscaped()))
+            const char* readQuotedString(const char delim = '"', const String& hackDelims = "") {
+                while (!eof() && (curChar() != delim || isEscaped())) {
+                    // This is a hack to handle paths with trailing backslashes that get misinterpreted as escaped double quotation marks.
+                    if (!hackDelims.empty() && curChar() == '"' && isEscaped() && hackDelims.find(lookAhead()) != String::npos) {
+                        m_state->resetEscaped();
+                        break;
+                    }
                     advance();
-                m_state->errorIfEof();
+                }
+                errorIfEof();
                 const char* end = curPos();
                 advance();
                 return end;
@@ -377,6 +389,10 @@ namespace TrenchBroom {
                 e << "Unexpected character '" << c << "'";
                 throw e;
             }
+
+            void errorIfEof() const {
+                m_state->errorIfEof();
+            }
         protected:
             bool isAnyOf(const char c, const String& allow) const {
                 for (size_t i = 0; i < allow.size(); i++)

common/src/Model/AttributeNameWithDoubleQuotationMarksIssueGenerator.cpp

@@ -0,0 +1,79 @@
+/*
+ Copyright (C) 2010-2016 Kristian Duske
+ 
+ This file is part of TrenchBroom.
+ 
+ TrenchBroom is free software: you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation, either version 3 of the License, or
+ (at your option) any later version.
+ 
+ TrenchBroom is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ GNU General Public License for more details.
+ 
+ You should have received a copy of the GNU General Public License
+ along with TrenchBroom. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "AttributeNameWithDoubleQuotationMarksIssueGenerator.h"
+
+#include "StringUtils.h"
+#include "Assets/EntityDefinition.h"
+#include "Model/Brush.h"
+#include "Model/Entity.h"
+#include "Model/Issue.h"
+#include "Model/IssueQuickFix.h"
+#include "Model/MapFacade.h"
+#include "Model/PushSelection.h"
+#include "Model/RemoveEntityAttributesQuickFix.h"
+#include "Model/TransformEntityAttributesQuickFix.h"
+
+#include <cassert>
+
+namespace TrenchBroom {
+    namespace Model {
+        class AttributeNameWithDoubleQuotationMarksIssueGenerator::AttributeNameWithDoubleQuotationMarksIssue : public AttributeIssue {
+        public:
+            static const IssueType Type;
+        private:
+            const AttributeName m_attributeName;
+        public:
+            AttributeNameWithDoubleQuotationMarksIssue(AttributableNode* node, const AttributeName& attributeName) :
+            AttributeIssue(node),
+            m_attributeName(attributeName) {}
+
+            const AttributeName& attributeName() const override {
+                return m_attributeName;
+            }
+        private:
+            IssueType doGetType() const override {
+                return Type;
+            }
+
+            const String doGetDescription() const override {
+                return "The key of entity property '" + m_attributeName + "' contains double quotation marks. This may cause errors during compilation or in the game.";
+            }
+        };
+
+        const IssueType AttributeNameWithDoubleQuotationMarksIssueGenerator::AttributeNameWithDoubleQuotationMarksIssue::Type = Issue::freeType();
+
+        AttributeNameWithDoubleQuotationMarksIssueGenerator::AttributeNameWithDoubleQuotationMarksIssueGenerator() :
+        IssueGenerator(AttributeNameWithDoubleQuotationMarksIssue::Type, "Invalid entity property keys") {
+            addQuickFix(new RemoveEntityAttributesQuickFix(AttributeNameWithDoubleQuotationMarksIssue::Type));
+            addQuickFix(new TransformEntityAttributesQuickFix(AttributeNameWithDoubleQuotationMarksIssue::Type,
+                                                              "Replace \" with '",
+                                                              [] (const AttributeName& name)   { return StringUtils::replaceAll(name, "\"", "'"); },
+                                                              [] (const AttributeValue& value) { return value; }));
+        }
+
+        void AttributeNameWithDoubleQuotationMarksIssueGenerator::doGenerate(AttributableNode* node, IssueList& issues) const {
+            for (const EntityAttribute& attribute : node->attributes()) {
+                const AttributeName& attributeName = attribute.name();
+                if (attributeName.find('"') != String::npos)
+                    issues.push_back(new AttributeNameWithDoubleQuotationMarksIssue(node, attributeName));
+            }
+        }
+    }
+}

common/src/Model/AttributeNameWithDoubleQuotationMarksIssueGenerator.h

@@ -0,0 +1,38 @@
+/*
+ Copyright (C) 2010-2016 Kristian Duske
+ 
+ This file is part of TrenchBroom.
+ 
+ TrenchBroom is free software: you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation, either version 3 of the License, or
+ (at your option) any later version.
+ 
+ TrenchBroom is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ GNU General Public License for more details.
+ 
+ You should have received a copy of the GNU General Public License
+ along with TrenchBroom. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef AttributeNameWithDoubleQuotationMarksIssueGenerator_h
+#define AttributeNameWithDoubleQuotationMarksIssueGenerator_h
+
+#include "Model/IssueGenerator.h"
+#include "Model/ModelTypes.h"
+
+namespace TrenchBroom {
+    namespace Model {
+        class AttributeNameWithDoubleQuotationMarksIssueGenerator : public IssueGenerator {
+        private:
+            class AttributeNameWithDoubleQuotationMarksIssue;
+        public:
+            AttributeNameWithDoubleQuotationMarksIssueGenerator();
+        private:
+            void doGenerate(AttributableNode* node, IssueList& issues) const;
+        };
+    }
+}
+#endif /* AttributeNameWithDoubleQuotationMarksIssueGenerator_h */

common/src/Model/AttributeValueWithDoubleQuotationMarksIssueGenerator.cpp

@@ -0,0 +1,80 @@
+/*
+ Copyright (C) 2010-2016 Kristian Duske
+ 
+ This file is part of TrenchBroom.
+ 
+ TrenchBroom is free software: you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation, either version 3 of the License, or
+ (at your option) any later version.
+ 
+ TrenchBroom is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ GNU General Public License for more details.
+ 
+ You should have received a copy of the GNU General Public License
+ along with TrenchBroom. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "AttributeValueWithDoubleQuotationMarksIssueGenerator.h"
+
+#include "StringUtils.h"
+#include "Assets/EntityDefinition.h"
+#include "Model/Brush.h"
+#include "Model/Entity.h"
+#include "Model/Issue.h"
+#include "Model/IssueQuickFix.h"
+#include "Model/MapFacade.h"
+#include "Model/PushSelection.h"
+#include "Model/RemoveEntityAttributesQuickFix.h"
+#include "Model/TransformEntityAttributesQuickFix.h"
+
+#include <cassert>
+
+namespace TrenchBroom {
+    namespace Model {
+        class AttributeValueWithDoubleQuotationMarksIssueGenerator::AttributeValueWithDoubleQuotationMarksIssue : public AttributeIssue {
+        public:
+            static const IssueType Type;
+        private:
+            const AttributeName m_attributeName;
+        public:
+            AttributeValueWithDoubleQuotationMarksIssue(AttributableNode* node, const AttributeName& attributeName) :
+            AttributeIssue(node),
+            m_attributeName(attributeName) {}
+
+            const AttributeName& attributeName() const override {
+                return m_attributeName;
+            }
+        private:
+            IssueType doGetType() const override {
+                return Type;
+            }
+
+            const String doGetDescription() const override {
+                return "The value of entity property '" + m_attributeName + "' contains double quotation marks. This may cause errors during compilation or in the game.";
+            }
+        };
+
+        const IssueType AttributeValueWithDoubleQuotationMarksIssueGenerator::AttributeValueWithDoubleQuotationMarksIssue::Type = Issue::freeType();
+
+        AttributeValueWithDoubleQuotationMarksIssueGenerator::AttributeValueWithDoubleQuotationMarksIssueGenerator() :
+        IssueGenerator(AttributeValueWithDoubleQuotationMarksIssue::Type, "Invalid entity property keys") {
+            addQuickFix(new RemoveEntityAttributesQuickFix(AttributeValueWithDoubleQuotationMarksIssue::Type));
+            addQuickFix(new TransformEntityAttributesQuickFix(AttributeValueWithDoubleQuotationMarksIssue::Type,
+                                                              "Replace \" with '",
+                                                              [] (const AttributeName& name)   { return name; },
+                                                              [] (const AttributeValue& value) { return StringUtils::replaceAll(value, "\"", "'"); }));
+        }
+
+        void AttributeValueWithDoubleQuotationMarksIssueGenerator::doGenerate(AttributableNode* node, IssueList& issues) const {
+            for (const EntityAttribute& attribute : node->attributes()) {
+                const AttributeName& attributeName = attribute.name();
+                const AttributeValue& attributeValue = attribute.value();
+                if (attributeValue.find('"') != String::npos)
+                    issues.push_back(new AttributeValueWithDoubleQuotationMarksIssue(node, attributeName));
+            }
+        }
+    }
+}

common/src/Model/AttributeValueWithDoubleQuotationMarksIssueGenerator.h

@@ -0,0 +1,38 @@
+/*
+ Copyright (C) 2010-2016 Kristian Duske
+ 
+ This file is part of TrenchBroom.
+ 
+ TrenchBroom is free software: you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation, either version 3 of the License, or
+ (at your option) any later version.
+ 
+ TrenchBroom is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ GNU General Public License for more details.
+ 
+ You should have received a copy of the GNU General Public License
+ along with TrenchBroom. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef AttributeValueWithDoubleQuotationMarksIssueGenerator_h
+#define AttributeValueWithDoubleQuotationMarksIssueGenerator_h
+
+#include "Model/IssueGenerator.h"
+#include "Model/ModelTypes.h"
+
+namespace TrenchBroom {
+    namespace Model {
+        class AttributeValueWithDoubleQuotationMarksIssueGenerator : public IssueGenerator {
+        private:
+            class AttributeValueWithDoubleQuotationMarksIssue;
+        public:
+            AttributeValueWithDoubleQuotationMarksIssueGenerator();
+        private:
+            void doGenerate(AttributableNode* node, IssueList& issues) const;
+        };
+    }
+}
+#endif /* AttributeValueWithDoubleQuotationMarksIssueGenerator_h */

common/src/Model/Game.h

@@ -41,10 +41,6 @@ namespace TrenchBroom {
         class TextureManager;
     }
 
-    namespace IO {
-        class MapWriter;
-    }
-
     namespace Model {
         class BrushContentTypeBuilder;