#8985: Raise TypeError for Resource.putChild(str) (#11718)

src/twisted/web/distrib.py

@@ -10,9 +10,8 @@
 """
 
 import copy
-
-# System Imports
 import os
+import sys
 
 try:
     import pwd
@@ -24,8 +23,6 @@
 from twisted.internet import address, reactor
 from twisted.logger import Logger
 from twisted.persisted import styles
-
-# Twisted Imports
 from twisted.spread import pb
 from twisted.spread.banana import SIZE_LIMIT
 from twisted.web import http, resource, server, static, util
@@ -355,18 +352,20 @@ def render_GET(self, request):
         return htmlDoc.encode("utf-8")
 
     def getChild(self, name, request):
-        if name == "":
+        if name == b"":
             return self
 
-        td = ".twistd"
+        td = b".twistd"
 
-        if name[-len(td) :] == td:
+        if name.endswith(td):
             username = name[: -len(td)]
             sub = 1
         else:
             username = name
             sub = 0
         try:
+            # Decode using the filesystem encoding to reverse a transformation
+            # done in the pwd module.
             (
                 pw_name,
                 pw_passwd,
@@ -375,7 +374,7 @@ def getChild(self, name, request):
                 pw_gecos,
                 pw_dir,
                 pw_shell,
-            ) = self._pwd.getpwnam(username)
+            ) = self._pwd.getpwnam(username.decode(sys.getfilesystemencoding()))
         except KeyError:
             return resource.NoResource()
         if sub:

src/twisted/web/newsfragments/8985.removal

@@ -0,0 +1 @@
+twisted.web.resource.Resource.putChild now raises TypeError when the path argument is not bytes, rather than issuing a deprecation warning.

src/twisted/web/resource.py

@@ -59,16 +59,18 @@ def getChildWithDefault(name, request):
         @type request: L{twisted.web.server.Request}
         """
 
-    def putChild(path, child):
+    def putChild(path: bytes, child: "IResource") -> None:
         """
-        Put a child IResource implementor at the given path.
+        Put a child L{IResource} implementor at the given path.
 
         @param path: A single path component, to be interpreted relative to the
             path this resource is found at, at which to put the given child.
             For example, if resource A can be found at I{http://example.com/foo}
             then a call like C{A.putChild(b"bar", B)} will make resource B
             available at I{http://example.com/foo/bar}.
-        @type path: C{bytes}
+
+            The path component is I{not} URL-encoded -- pass C{b'foo bar'}
+            rather than C{b'foo%20bar'}.
         """
 
     def render(request):
@@ -103,7 +105,7 @@ class Resource:
     """
     Define a web-accessible resource.
 
-    This serves 2 main purposes; one is to provide a standard representation
+    This serves two main purposes: one is to provide a standard representation
     for what HTTP specification calls an 'entity', and the other is to provide
     an abstract directory structure for URL retrieval.
     """
@@ -199,12 +201,17 @@ def getChildWithDefault(self, path, request):
         return self.getChild(path, request)
 
     def getChildForRequest(self, request):
+        """
+        Deprecated in favor of L{getChildForRequest}.
+
+        @see: L{twisted.web.resource.getChildForRequest}.
+        """
         warnings.warn(
             "Please use module level getChildForRequest.", DeprecationWarning, 2
         )
         return getChildForRequest(self, request)
 
-    def putChild(self, path, child):
+    def putChild(self, path: bytes, child: IResource) -> None:
         """
         Register a static child.
 
@@ -213,24 +220,18 @@ def putChild(self, path, child):
         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 {} has never worked, and "
-                "will raise an exception in the future.".format(type(path)),
-                category=DeprecationWarning,
-                stacklevel=2,
-            )
+            raise TypeError(f"Path segment must be bytes, but {path!r} is {type(path)}")
 
         self.children[path] = child
-        child.server = self.server
+        # IResource is incomplete and doesn't mention this server attribute, see
+        # https://github.com/twisted/twisted/issues/11717
+        child.server = self.server  # type: ignore[attr-defined]
 
     def render(self, request):
         """

src/twisted/web/test/requesthelper.py

@@ -5,11 +5,12 @@
 Helpers related to HTTP requests, used by tests.
 """
 
+from __future__ import annotations
 
 __all__ = ["DummyChannel", "DummyRequest"]
 
 from io import BytesIO
-from typing import Optional
+from typing import Dict, List, Optional
 
 from zope.interface import implementer, verify
 
@@ -206,6 +207,11 @@ class DummyRequest:
     uri = b"http://dummy/"
     method = b"GET"
     client: Optional[IAddress] = None
+    sitepath: List[bytes]
+    written: List[bytes]
+    prepath: List[bytes]
+    args: Dict[bytes, List[bytes]]
+    _finishedDeferreds: List[Deferred[None]]
 
     def registerProducer(self, prod, s):
         """
@@ -225,7 +231,12 @@ def registerProducer(self, prod, s):
     def unregisterProducer(self):
         self.go = 0
 
-    def __init__(self, postpath, session=None, client=None):
+    def __init__(
+        self,
+        postpath: list[bytes],
+        session: Optional[Session] = None,
+        client: Optional[IAddress] = None,
+    ) -> None:
         self.sitepath = []
         self.written = []
         self.finished = 0

src/twisted/web/test/test_distrib.py

@@ -29,7 +29,7 @@
 from twisted.web import client, distrib, resource, server, static
 from twisted.web.http_headers import Headers
 from twisted.web.test._util import _render
-from twisted.web.test.test_web import DummyChannel, DummyRequest
+from twisted.web.test.requesthelper import DummyChannel, DummyRequest
 
 
 class MySite(server.Site):
@@ -395,36 +395,32 @@ def test_interface(self):
         """
         self.assertTrue(verifyObject(resource.IResource, self.directory))
 
-    def _404Test(self, name):
+    async def _404Test(self, name: bytes) -> None:
         """
         Verify that requesting the C{name} child of C{self.directory} results
         in a 404 response.
         """
         request = DummyRequest([name])
         result = self.directory.getChild(name, request)
         d = _render(result, request)
+        await d
+        self.assertEqual(request.responseCode, 404)
 
-        def cbRendered(ignored):
-            self.assertEqual(request.responseCode, 404)
-
-        d.addCallback(cbRendered)
-        return d
-
-    def test_getInvalidUser(self):
+    async def test_getInvalidUser(self):
         """
         L{UserDirectory.getChild} returns a resource which renders a 404
         response when passed a string which does not correspond to any known
         user.
         """
-        return self._404Test("carol")
+        await self._404Test(b"carol")
 
-    def test_getUserWithoutResource(self):
+    async def test_getUserWithoutResource(self):
         """
         L{UserDirectory.getChild} returns a resource which renders a 404
         response when passed a string which corresponds to a known user who has
         neither a user directory nor a user distrib socket.
         """
-        return self._404Test("alice")
+        await self._404Test(b"alice")
 
     def test_getPublicHTMLChild(self):
         """
@@ -436,7 +432,7 @@ def test_getPublicHTMLChild(self):
         public_html = home.child("public_html")
         public_html.makedirs()
         request = DummyRequest(["bob"])
-        result = self.directory.getChild("bob", request)
+        result = self.directory.getChild(b"bob", request)
         self.assertIsInstance(result, static.File)
         self.assertEqual(result.path, public_html.path)
 
@@ -450,7 +446,7 @@ def test_getDistribChild(self):
         home.makedirs()
         web = home.child(".twistd-web-pb")
         request = DummyRequest(["bob"])
-        result = self.directory.getChild("bob.twistd", request)
+        result = self.directory.getChild(b"bob.twistd", request)
         self.assertIsInstance(result, distrib.ResourceSubscription)
         self.assertEqual(result.host, "unix")
         self.assertEqual(abspath(result.port), web.path)

src/twisted/web/test/test_resource.py

@@ -182,19 +182,8 @@ def test_staticChildPathType(self):
         resource = Resource()
         child = Resource()
         sibling = Resource()
-        resource.putChild("foo", child)
-        warnings = self.flushWarnings([self.test_staticChildPathType])
-        self.assertEqual(len(warnings), 1)
-        self.assertIn("Path segment must be bytes", warnings[0]["message"])
-        # We expect an error here because "foo" != b"foo" on Python 3+
-        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"])
+        self.assertRaises(TypeError, resource.putChild, "foo", child)
+        self.assertRaises(TypeError, resource.putChild, None, sibling)
 
     def test_defaultHEAD(self):
         """

src/twisted/web/test/test_template.py

@@ -761,7 +761,7 @@ class DummyRenderRequest(DummyRequest):  # type: ignore[misc]
     """
 
     def __init__(self) -> None:
-        super().__init__([""])
+        super().__init__([b""])
         self.site = FakeSite()