Merge pull request python-zeroconf#77 from stephenrauch/fix-instance-name-with-dot

stephenrauch · web-flow · commit 9035c6a246b6 · 2016-07-13T20:49:34.000-07:00
Allow dots in service instance name
diff --git a/README.rst b/README.rst
@@ -129,13 +129,14 @@ Changelog
 * Many exceptions will now log a warning the first time they are seen
 * Catch and log sendto() errors
 * Fix/Implement duplicate name change
+* Fix overly strict name validation introduced in 0.17.6
 * Greatly improve handling of oversized packets including:
 
   - Implement name compression per RFC1035
   - Limit size of generated packets to 9000 bytes as per RFC6762
   - Better handle over sized incoming packets
 
-* Increased test coverage to 94%
+* Increased test coverage to 95%
 
 0.17.6
 ------
diff --git a/test_zeroconf.py b/test_zeroconf.py
@@ -422,24 +422,43 @@ def test_bad_service_names(self):
             '_22._udp.local.',
             '_2-2._tcp.local.',
             '_1234567890-abcde._udp.local.',
-            '._x._udp.local.',
+            '\x00._x._udp.local.',
         )
         for name in bad_names_to_try:
             self.assertRaises(
                 r.BadTypeInNameException,
                 self.browser.get_service_info, name, 'x.' + name)
 
-    def test_bad_sub_types(self):
-        bad_names_to_try = (
-            '_sub._http._tcp.local.',
+    def test_good_instance_names(self):
+        good_names_to_try = (
+            '.._x._tcp.local.',
             'x.sub._http._tcp.local.',
+            '6d86f882b90facee9170ad3439d72a4d6ee9f511._zget._http._tcp.local.'
+        )
+        for name in good_names_to_try:
+            r.service_type_name(name)
+
+    def test_bad_types(self):
+        bad_names_to_try = (
+            '._x._tcp.local.',
             'a' * 64 + '._sub._http._tcp.local.',
             'a' * 62 + u'â._sub._http._tcp.local.',
         )
         for name in bad_names_to_try:
             self.assertRaises(
                 r.BadTypeInNameException, r.service_type_name, name)
 
+    def test_bad_sub_types(self):
+        bad_names_to_try = (
+            '_sub._http._tcp.local.',
+            '._sub._http._tcp.local.',
+            '\x7f._sub._http._tcp.local.',
+            '\x1f._sub._http._tcp.local.',
+        )
+        for name in bad_names_to_try:
+            self.assertRaises(
+                r.BadTypeInNameException, r.service_type_name, name)
+
     def test_good_service_names(self):
         good_names_to_try = (
             '_x._tcp.local.',
diff --git a/zeroconf.py b/zeroconf.py
@@ -158,6 +158,7 @@ def emit(self, record):
 
 _HAS_A_TO_Z = re.compile(r'[A-Za-z]')
 _HAS_ONLY_A_TO_Z_NUM_HYPHEN = re.compile(r'^[A-Za-z0-9\-]+$')
+_HAS_ASCII_CONTROL_CHARS = re.compile(r'[\x00-\x1f\x7f]')
 
 
 @enum.unique
@@ -212,6 +213,14 @@ def service_type_name(type_):
 
     The instance name <Instance> and sub type <sub> may be up to 63 bytes.
 
+    The portion of the Service Instance Name is a user-
+    friendly name consisting of arbitrary Net-Unicode text [RFC5198]. It
+    MUST NOT contain ASCII control characters (byte values 0x00-0x1F and
+    0x7F) [RFC20] but otherwise is allowed to contain any characters,
+    without restriction, including spaces, uppercase, lowercase,
+    punctuation -- including dots -- accented characters, non-Roman text,
+    and anything else that may be represented using Net-Unicode.
+
     :param type_: Type, SubType or service name to validate
     :return: fully qualified service name (eg: _http._tcp.local.)
     """
@@ -220,15 +229,15 @@ def service_type_name(type_):
             "Type '%s' must end with '._tcp.local.' or '._udp.local.'" %
             type_)
 
-    if type_.startswith('.'):
-        raise BadTypeInNameException(
-            "Type '%s' must not start with '.'" % type_)
-
     remaining = type_[:-len('._tcp.local.')].split('.')
     name = remaining.pop()
     if not name:
         raise BadTypeInNameException("No Service name found")
 
+    if len(remaining) == 1 and len(remaining[0]) == 0:
+        raise BadTypeInNameException(
+            "Type '%s' must not start with '.'" % type_)
+
     if name[0] != '_':
         raise BadTypeInNameException(
             "Service name (%s) must start with '_'" % name)
@@ -260,20 +269,23 @@ def service_type_name(type_):
 
     if remaining and remaining[-1] == '_sub':
         remaining.pop()
-        if len(remaining) == 0:
+        if len(remaining) == 0 or len(remaining[0]) == 0:
             raise BadTypeInNameException(
                 "_sub requires a subtype name")
 
     if len(remaining) > 1:
-        raise BadTypeInNameException(
-            "Unexpected characters '%s.' in '%s'" % (
-                '.'.join(remaining[1:]), type_))
+        remaining = ['.'.join(remaining)]
 
     if remaining:
         length = len(remaining[0].encode('utf-8'))
         if length > 63:
             raise BadTypeInNameException("Too long: '%s'" % remaining[0])
 
+        if _HAS_ASCII_CONTROL_CHARS.search(remaining[0]):
+            raise BadTypeInNameException(
+                "Ascii control character 0x00-0x1F and 0x7F illegal in '%s'" %
+                remaining[0])
+
     return '_' + name + type_[-len('._tcp.local.'):]
 
 
@@ -1846,13 +1858,13 @@ def unregister_all_services(self):
     def check_service(self, info, allow_name_change):
         """Checks the network for a unique service name, modifying the
         ServiceInfo passed in if it is not unique."""
-        service_name = service_type_name(info.name)
 
-        # This asserts breaks on the current subtype based tests
+        # This is kind of funky because of the subtype based tests
         # need to make subtypes a first class citizen
-        #          assert service_name == info.type
-        #  instead try:
-        assert service_name == '.'.join(info.type.split('.')[-4:])
+        service_name = service_type_name(info.name)
+        if not info.type.endswith(service_name):
+            raise BadTypeInNameException
+
         instance_name = info.name[:-len(service_name) - 1]
         next_instance_number = 2
 
