project_loader: handle invalid unicode chars (#1941)

Note that the exception is currently raised with valid and invalid unicode chbaracters due to the upstream bug pyyaml#25. But we'll want to handle the error cleanly even if the upstream issue is fixed.

This branch adds a patch to the PyYAML used by the Snapcraft snap to handle the unicode code points erroneously flagged as invalid, such as the hankey emoji. A Snapcraft snap built from this PR will successfully validate a summary or description making use of the hankey emoji.

New test cases:

    tests.unit.project_loader.test_config.test_invalid_yaml_invalid_unicode_chars
    tests.integration.general.test_global_properties

Note: The integration level test for the PyYAML work-around is skipped unless testing with a snap or Debian package (ie. SNAPCRAFT_FROM_SNAP=1 is set or SNAPCRAFT_FROM_DEB=1 is set), both of which are patched. I verified with a snap from the branch, and running the tests in a virtual environment respectively.

LP: #1737571

patches/pyyaml-support-high-codepoints.diff

@@ -0,0 +1,27 @@
+diff --git a/yaml/emitter.py b/yaml/emitter.py
+index 34cb145..1f8ed92 100644
+--- a/yaml/emitter.py
++++ b/yaml/emitter.py
+@@ -698,7 +698,8 @@ class Emitter:
+                 line_breaks = True
+             if not (ch == '\n' or '\x20' <= ch <= '\x7E'):
+                 if (ch == '\x85' or '\xA0' <= ch <= '\uD7FF'
+-                        or '\uE000' <= ch <= '\uFFFD') and ch != '\uFEFF':
++                        or '\uE000' <= ch <= '\uFFFD'
++                        or '\U00010000' <= ch < '\U0010ffff') and ch != '\uFEFF':
+                     unicode_characters = True
+                     if not self.allow_unicode:
+                         special_characters = True
+diff --git a/yaml/reader.py b/yaml/reader.py
+index f70e920..5764f2d 100644
+--- a/yaml/reader.py
++++ b/yaml/reader.py
+@@ -134,7 +134,7 @@ class Reader(object):
+                 self.encoding = 'utf-8'
+         self.update(1)
+
+-    NON_PRINTABLE = re.compile('[^\x09\x0A\x0D\x20-\x7E\x85\xA0-\uD7FF\uE000-\uFFFD]')
++    NON_PRINTABLE = re.compile('[^\x09\x0A\x0D\x20-\x7E\x85\xA0-\uD7FF\uE000-\uFFFD\U00010000-\U0010ffff]')
+     def check_printable(self, data):
+         match = self.NON_PRINTABLE.search(data)
+         if match:

snap/snapcraft.yaml

@@ -16,11 +16,12 @@ apps:
     completer: snapcraft-completion
 
 parts:
-  ctypes:
-    source: ctypes_override
+  patches:
+    source: patches
     plugin: dump
     prime:
         - -ctypes_init.diff
+        - -pyyaml-support-high-codepoints.diff
   bash-completion:
     source: debian
     plugin: dump
@@ -48,6 +49,7 @@ parts:
         TRIPLET_PATH="$SNAPCRAFT_PART_INSTALL/usr/lib/$(gcc -print-multiarch)"
         LIBSODIUM=$(readlink -n $TRIPLET_PATH/libsodium.so.18)
         ln -s $LIBSODIUM $TRIPLET_PATH/libsodium.so
+        patch -d $SNAPCRAFT_PART_INSTALL/lib/python3.6/site-packages -p1 < $SNAPCRAFT_STAGE/pyyaml-support-high-codepoints.diff
     after: [python, apt]
   patchelf:
     source: https://github.com/NixOS/patchelf
@@ -65,7 +67,7 @@ parts:
         - -usr/include
     install: |
         patch $SNAPCRAFT_PART_INSTALL/usr/lib/python3.6/ctypes/__init__.py $SNAPCRAFT_STAGE/ctypes_init.diff
-    after: [ctypes]
+    after: [patches]
   apt:
     source: https://github.com/Debian/apt
     source-type: git

snapcraft/internal/project_loader/_config.py

@@ -22,6 +22,8 @@
 
 import jsonschema
 import yaml
+import yaml.reader
+
 
 import snapcraft
 from snapcraft.internal import common, deprecations, remote_parts, states
@@ -268,6 +270,10 @@ def _snapcraft_yaml_load(yaml_file):
     except yaml.scanner.ScannerError as e:
         raise errors.YamlValidationError('{} on line {} of {}'.format(
             e.problem, e.problem_mark.line + 1, yaml_file)) from e
+    except yaml.reader.ReaderError as e:
+        raise errors.YamlValidationError(
+            'Invalid character {!r} at position {} of {}: {}'.format(
+                chr(e.character), e.position + 1, yaml_file, e.reason)) from e
 
 
 def _ensure_confinement_default(yaml_data, schema):

tests/integration/general/test_global_properties.py

@@ -0,0 +1,46 @@
+# -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*-
+#
+# Copyright (C) 2018 Canonical Ltd
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 3 as
+# published by the Free Software Foundation.
+#
+# This program 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 this program.  If not, see <http://www.gnu.org/licenses/>.
+import os
+
+import testscenarios
+
+from tests import integration, fixture_setup
+
+
+class UnicodePropertyTestCase(testscenarios.WithScenarios,
+                              integration.TestCase):
+
+    scenarios = [
+        ('summary',
+         dict(name='foo', summary='bar💩', description='baz')),
+        ('description',
+         dict(name='foo', summary='bar', description='baz💩')),
+        ]
+
+    def test_invalid_unicode_workaround(self):
+        if not (os.getenv('SNAPCRAFT_FROM_SNAP', False) or
+                os.getenv('SNAPCRAFT_FROM_DEB', False)):
+            self.skipTest('The yaml unicode patch is applied to the snap '
+                          'and python3-yaml package, but not PyYAML in PyPI')
+
+        snapcraft_yaml = fixture_setup.SnapcraftYaml(
+            self.path, name=self.name,
+            summary=self.summary, description=self.description)
+        snapcraft_yaml.update_part('my-part', {
+            'plugin': 'nil',
+        })
+        self.useFixture(snapcraft_yaml)
+        self.run_snapcraft('pull')

tests/unit/project_loader/test_config.py

@@ -531,6 +531,23 @@ def test_invalid_yaml_missing_icon(self):
         self.assertThat(raised.message,
                         Equals("Specified icon 'icon.png' does not exist"))
 
+    def test_invalid_yaml_invalid_unicode_chars(self):
+        fake_logger = fixtures.FakeLogger(level=logging.ERROR)
+        self.useFixture(fake_logger)
+
+        self.make_snapcraft_yaml("""name: foobar
+version: "1"
+summary: test\uffff
+description: nothing
+""")
+        raised = self.assertRaises(
+            errors.YamlValidationError,
+            _config.Config)
+
+        self.assertThat(raised.message, Equals(
+            "Invalid character '\\uffff' at position 40 "
+            "of snap/snapcraft.yaml: special characters are not allowed"))
+
     def test_invalid_yaml_invalid_name_chars(self):
         fake_logger = fixtures.FakeLogger(level=logging.ERROR)
         self.useFixture(fake_logger)