sgf parser escape fix

katrain/core/engine.py

@@ -61,18 +61,18 @@ class KataGoEngine:
 
             exepath, exename = os.path.split(exe)
             if exepath and not os.path.isfile(exe):
-                self.katrain.log(i18n._('Kata exe not found').format(exe=exe), OUTPUT_ERROR)
+                self.katrain.log(i18n._("Kata exe not found").format(exe=exe), OUTPUT_ERROR)
                 return  # don't start
             elif not exepath and not any(
                 os.path.isfile(os.path.join(path, exe)) for path in os.environ.get("PATH", "").split(os.pathsep)
             ):
-                self.katrain.log(i18n._('Kata exe not found in path').format(exe),                    OUTPUT_ERROR   )
+                self.katrain.log(i18n._("Kata exe not found in path").format(exe), OUTPUT_ERROR)
                 return  # don't start
             elif not os.path.isfile(model):
-                self.katrain.log(i18n._('Kata model not found').format(model), OUTPUT_ERROR)
+                self.katrain.log(i18n._("Kata model not found").format(model), OUTPUT_ERROR)
                 return  # don't start
             elif not os.path.isfile(cfg):
-                self.katrain.log(i18n._('Kata config not found').format(config=cfg), OUTPUT_ERROR)
+                self.katrain.log(i18n._("Kata config not found").format(config=cfg), OUTPUT_ERROR)
                 return  # don't start
             self.command = f'"{exe}" analysis -model "{model}" -config "{cfg}" -analysis-threads {config["threads"]}'
         self.start()

katrain/core/game.py

@@ -320,7 +320,7 @@ class Game:
         engine = self.engines[cn.next_player]
         if mode == "extra":
             visits = cn.analysis_visits_requested + engine.config["max_visits"]
-            self.katrain.controls.set_status(i18n._('extra analysis').format(visits=visits))
+            self.katrain.controls.set_status(i18n._("extra analysis").format(visits=visits))
             cn.analyze(engine, visits=visits, priority=-1_000, time_limit=False)
             return
         elif mode == "sweep":
@@ -348,7 +348,7 @@ class Game:
                     if (x, y) not in stones
                 ]
             visits = engine.config["fast_visits"]
-            self.katrain.controls.set_status(i18n._('sweep analysis').format(visits=visits))
+            self.katrain.controls.set_status(i18n._("sweep analysis").format(visits=visits))
             priority = -1_000_000_000
         else:  # mode=='equalize':
             if not cn.analysis_ready:
@@ -357,7 +357,7 @@ class Game:
 
             analyze_moves = [Move.from_gtp(gtp, player=cn.next_player) for gtp, _ in cn.analysis["moves"].items()]
             visits = max(d["visits"] for d in cn.analysis["moves"].values())
-            self.katrain.controls.set_status(i18n._('equalizing analysis').format(visits=visits))
+            self.katrain.controls.set_status(i18n._("equalizing analysis").format(visits=visits))
             priority = -1_000
         for move in analyze_moves:
             cn.analyze(

katrain/core/sgf_parser.py

@@ -91,13 +91,21 @@ class SGFNode:
         """For hooking into in a subclass and overriding branch order."""
         return children
 
+    @staticmethod
+    def _escape_value(value):
+        return re.sub(r"([\]\\])", r"\\\1", value) if isinstance(value, str) else value  # escape \ and ]
+
+    @staticmethod
+    def _unescape_value(value):
+        return re.sub(r"\\([\]\\])", r"\1", value) if isinstance(value, str) else value  # unescape \ and ]
+
     def sgf(self, **xargs) -> str:
         """Generates an SGF, calling sgf_properties on each node with the given xargs, so it can filter relevant properties if needed."""
 
         def node_sgf_str(node):
             return ";" + "".join(
                 [
-                    prop + "".join(f"[{v}]" for v in values)
+                    prop + "".join(f"[{self._escape_value(v)}]" for v in values)
                     for prop, values in node.sgf_properties(**xargs).items()
                     if values
                 ]
@@ -127,12 +135,9 @@ class SGFNode:
 
     def set_property(self, property: str, value: Any):
         """Add some values to the property. If not a list, it will be made into a single-value list."""
-        if isinstance(value, list):
+        if not isinstance(value, list):
+            value = [value]
         self.properties[property] = value
-        else:
-            if isinstance(value, str):
-                value = re.sub(r"(?<!\\)(\])", r"\\\1", value)  # escape unescaped ] - TODO: this could be \\[ or something and still break / unescape as well
-            self.properties[property] = [value]
 
     def get_property(self, property, default=None) -> Any:
         """Get the first value of the property, typically when exactly one is expected."""
@@ -278,9 +283,10 @@ class SGFNode:
 
 
 class SGF:
-    """Class used for SGF Nodes, can change this to something that inherits from SGFNode"""
 
-    _NODE_CLASS = SGFNode
+    _NODE_CLASS = SGFNode  # Class used for SGF Nodes, can change this to something that inherits from SGFNode
+    # https://xkcd.com/1171/
+    SGFPROP_PAT = re.compile(r"\s*(?:\(|\)|;|(?:(\w+)((\s*\[([^\]\\]*(\\.[^\]\\]*)*)\])+)))", flags=re.DOTALL)
 
     @classmethod
     def parse(cls, input_str) -> SGFNode:
@@ -311,8 +317,8 @@ class SGF:
         self._parse_branch(self.root)
 
     def _parse_branch(self, current_move: SGFNode):
-        while self.ix < len(self.contents):  # https://xkcd.com/1171/
-            match = re.match(r"\s*(?:\(|\)|;|(?:(\w+)((?:\[.*?(?<!\\)\]\s*)+)))", self.contents[self.ix :], re.DOTALL)
+        while self.ix < len(self.contents):
+            match = re.match(self.SGFPROP_PAT, self.contents[self.ix :])
             if not match:
                 break
             self.ix += len(match[0])
@@ -327,7 +333,7 @@ class SGF:
             else:
                 property, value = match[1], match[2].strip()[1:-1]
                 values = re.split(r"\]\s*\[", value)
-                current_move.add_list_property(property, values)
+                current_move.add_list_property(property, [SGFNode._unescape_value(v) for v in values])
         if self.ix < len(self.contents):
             raise ParseError(f"Parse Error: unexpected character at {self.contents[self.ix:self.ix+25]}")
         raise ParseError("Parse Error: expected ')' at end of input.")

tests/test_parser.py

@@ -1,6 +1,6 @@
 import os
 
-from katrain.core.sgf_parser import SGF
+from katrain.core.sgf_parser import SGF, SGFNode
 
 
 def test_simple():
@@ -33,6 +33,20 @@ or \\]
     assert input_sgf == root.sgf()
 
 
+def test_backslash_escape():
+    nasty_string = "[]]\\"
+    nasty_strings = ["[\\]\\]\\\\", "[", "]", "\\", "\\[", "\\]", "\\\\[", "\\\\]", "]]]\\]]\\]]["]
+    assert "[\\]\\]\\\\" == SGFNode._escape_value(nasty_string)
+    for x in nasty_strings:
+        assert x == SGFNode._unescape_value(SGFNode._escape_value(x))
+
+    c2 = ["]", "\\"]
+    node = SGFNode(properties={"C1": nasty_string})
+    node.set_property("C2", c2)
+    assert "(;C1[[\\]\\]\\\\]C2[\\]][\\\\])" == node.sgf()
+    assert {"C1": [nasty_string], "C2": c2} == SGF.parse(node.sgf()).properties
+
+
 def test_alphago():
     file = os.path.join(os.path.dirname(__file__), "data/LS vs AG - G4 - English.sgf")
     SGF.parse_file(file)