pathlib passed to tornado StaticFileHandler when it expects str

[dukrat]

tornado's StaticFileHandler expects str but it is being passed pathlib. This works most of the time, but when os.path.sep is not "/" (like on Windows) it causes this error:

ERROR    2020-05-05 21:29:34,510 [30532:HttpServer] tornado.application
  Uncaught exception GET /iris/settings (::1)
HTTPServerRequest(protocol='http', host='localhost:6680', method='GET', uri='/iris/settings', version='HTTP/1.1', remote_ip='::1')
Traceback (most recent call last):
  File "C:\msys64\mingw64\lib\python3.8\site-packages\tornado\web.py", line 1703, in _execute
    result = await result
  File "C:\msys64\mingw64\lib\python3.8\site-packages\tornado\web.py", line 2578, in get
    self.path = self.parse_url_path(path)
  File "C:\msys64\mingw64\lib\python3.8\site-packages\tornado\web.py", line 2967, in parse_url_path
    url_path = url_path.replace("/", os.path.sep)
TypeError: replace() takes 2 positional arguments but 3 were given
ERROR    2020-05-05 21:29:34,513 [30532:HttpServer] tornado.access

EDIT: This doesn't work, see #538 (comment)
I believe wrapping what is passed in str is sufficient to fix this and will not hurt anything. i.e. changing the end of __init__.py to

    return [
        (r"/http/([^/]*)", HttpHandler, {"core": core, "config": config}),
        (r"/ws/?", WebsocketHandler, {"core": core, "config": config}),
        (r"/assets/(.*)", StaticFileHandler, {"path": str(path / "assets")}),
        (r"/((.*)(?:css|js|json|map)$)", StaticFileHandler, {"path": str(path)}),
        (r"/(.*)", ReactRouterHandler, {"path": str(path / "index.html")}),
    ]

Let me know if you want a PR of that.

[kingosticks]

@jodal

[kingosticks]

@dukrat, could you try Mopidy-Local also? I'd imagine this suffers from the same problem.

[jodal]

@kingosticks Do you think there's anything Mopidy core can do differently here?

And yes, it seems Mopidy-Local is passing two pathlib objects to Tornado here: https://github.com/mopidy/mopidy-local/blob/master/mopidy_local/__init__.py#L53-L54

[dukrat]

Mopidy-Local is not affected by this for some reason. Tornado seems to handle the conversion in some (most?) situations. I think it is because self.path is being set directly during initialize of ReactRouterHandler.

Furthermore, the proposed fix causes another error but this seems to be fine
end of handlers.py

class ReactRouterHandler(tornado.web.StaticFileHandler):
    def initialize(self, path):
        self.path = str(path)

[jaedb]

@dukrat So ultimately is the only change required in handlers.py (see PR #540)?

[dukrat]

Yeah, I ran that and cache cleared and force refreshed a bunch without any errors. #540 resolves #538