Merge pull request #1105 from twisted/9135-typecheck-url-path

[9135] typecheck url path of Resource.putChild()

src/twisted/web/newsfragments/9135.removal

@@ -0,0 +1 @@
+Passing a path argument to twisted.web.resource.Resource.putChild which is not of type bytes is now deprecated.  In the future, passing a non-bytes argument to putChild will return an error.

src/twisted/web/resource.py

@@ -214,8 +214,23 @@ def putChild(self, path, child):
         intended to have the root of a folder, e.g. /foo/, you want
         path to be ''.
 
+        @param path: A single path component.
+        @type path: L{bytes}
+
+        @param child: The child resource to register.
+        @type child: L{IResource}
+
         @see: L{IResource.putChild}
         """
+        if not isinstance(path, bytes):
+            warnings.warn(
+                'Path segment must be bytes; '
+                'passing {0} has never worked, and '
+                'will raise an exception in the future.'
+                .format(type(path)),
+                category=DeprecationWarning,
+                stacklevel=2)
+
         self.children[path] = child
         child.server = self.server
 

src/twisted/web/test/test_resource.py

@@ -6,6 +6,7 @@
 """
 
 from twisted.trial.unittest import TestCase
+from twisted.python.compat import _PY3
 
 from twisted.web.error import UnsupportedMethod
 from twisted.web.resource import (
@@ -175,6 +176,32 @@ def test_dynamicChildren(self):
         self.assertIdentical(child.request, request)
 
 
+    def test_staticChildPathType(self):
+        """
+        Test that passing the wrong type to putChild results in a warning,
+        and a failure in Python 3
+        """
+        resource = Resource()
+        child = Resource()
+        sibling = Resource()
+        resource.putChild(u"foo", child)
+        warnings = self.flushWarnings([self.test_staticChildPathType])
+        self.assertEqual(len(warnings), 1)
+        self.assertIn("Path segment must be bytes",
+                      warnings[0]['message'])
+        if _PY3:
+            # We expect an error here because u"foo" != b"foo" on Py3k
+            self.assertIsInstance(
+                resource.getChildWithDefault(b"foo", DummyRequest([])),
+                ErrorPage)
+
+        resource.putChild(None, sibling)
+        warnings = self.flushWarnings([self.test_staticChildPathType])
+        self.assertEqual(len(warnings), 1)
+        self.assertIn("Path segment must be bytes",
+                      warnings[0]['message'])
+
+
     def test_defaultHEAD(self):
         """
         When not otherwise overridden, L{Resource.render} treats a I{HEAD}