9438 py2 bytes directory listing #1008
Conversation
This prevents the template from being promoted to unicode via interpolation of unicode paths.
src/twisted/web/static.py
Outdated
| Return a resource that generates an HTML listing of the | ||
| directory this path represents. | ||
|
|
||
| @return: A L{resource.Resource} that renders the directory to |
There was a problem hiding this comment.
Given that DirectoryLister is public, this could refer directly to that.
There was a problem hiding this comment.
Why is it public though. 😭
| path = self.path | ||
| names = self.listNames() | ||
| else: | ||
| # DirectoryLister works in terms of native strings, so on |
There was a problem hiding this comment.
Do we not need to do the inverse (.asTextMode()) on PY3 then?
There was a problem hiding this comment.
No, but for a bad reason: File and DirectoryLister are tricky.
getChild now decodes all descendant paths to text, so that by the time the empty path is reached and directoryListing called, the path given to DirectoryLister is guaranteed to be text, unless of course the path being listed is the File's own path.
Given:
from twisted.web import server, static
from twisted.internet import defer, endpoints, task
@task.react
def main(reactor):
site = server.Site(static.File(b"/path/to/some/directory/"))
listener = endpoints.serverFromString(reactor, "tcp:8080")
d = listener.listen(site)
@d.addCallback
def _(_):
return defer.Deferred()
return dAssuming that /path/to/some/directory/child exists, accessing http://localhost:8080/child/ or any other descendant creates a DirectoryLister with a text path. Accessing http://localhost:8080/, however, creates one with a bytes path.
Surprise! On Python 3, this is fine. That's DirectoryLister itself decodes bytes descendants to text on Python 3.
So the actual admitted by DirectoryLister is:
DirectoryLister parameter |
Python 3 | Python 2 |
|---|---|---|
| pathname | str or bytes |
str |
| dirs | str or bytes |
str |
| contentTypes | dict of str |
dict of str |
| contentEncodings | dict of str |
dict of str |
| defaultType | str |
str |
Which is not exactly what I said in the commit message, but does mean this part of the PR is correct (I think.)
There was a problem hiding this comment.
Every day I'm a little more sympathetic towards web2's abandonment of this API :).
src/twisted/web/test/test_static.py
Outdated
| self.assertIsInstance(response, bytes) | ||
| if sys.getfilesystemencoding().lower() not in ('utf-8', 'mcbs'): | ||
| test_emptyChildUnicodeParent.skip = ( | ||
| "Cannot write unicode filenames with file system encoding of" |
There was a problem hiding this comment.
ITYM "non-ASCII"? Given that the u'' child is the one we care about, I'm pretty sure that will survive most boneheaded filesystem encodings, and this skip could be removed?
There was a problem hiding this comment.
This is a safeguard I carried over from another test, because who knows what sys.getfilesystemencoding, which FilePath uses, is going to return. But as you point out, it should be impossible that SynchronousTestCase.mkstemp().decode(sys.getfilesystemencoding()) + u'' could return an unencodable path, so this skip is unnecessary.
It should be impossible for SynchronousTestCase.mkstemp().decode(sys.getfilesystemencoding()) + u'' to return an unencodable path, so the skip is unnecessary.
|
Is there a plan to release new twisted version with this fix? |
Contributor Checklist: