aio-libs / aiohttp-debugtoolbar

aiohttp_debugtoolbar is library for debugtoolbar support for aiohttp
Apache License 2.0
194 stars 31 forks source link

yarl update broke static file urls in templates #478

Closed 14droplets closed 1 year ago

14droplets commented 1 year ago

With yarl version 1.9.2 static files urls with url_for(filename="") as used, for example, here: https://github.com/aio-libs/aiohttp-debugtoolbar/blob/6570cb394bc232b8b758878893349534f0a7a218/aiohttp_debugtoolbar/tbtools/tbtools.py#L210 are no longer works correctly. Produced URL no longer contains terminating slash to be concatenated with filename in template code like it is done here: https://github.com/aio-libs/aiohttp-debugtoolbar/blob/6570cb394bc232b8b758878893349534f0a7a218/aiohttp_debugtoolbar/templates/toolbar.jinja2#L10

I traced issue to this line in yarl: https://github.com/aio-libs/yarl/blob/1f94e1a7833a27203d63f8618d01d4d4fa2d3520/yarl/_url.py#L719. With segment as empty string - main loop breaks immediately.

Issue is observed with yarl versions 1.9.2 and 1.9.1 and disappears with yarl version 1.8.2.

I can rewrite templates using url_for instead of {{ static_path }}css/debugger.css like syntax and provide a pull request. Is it acceptable fix? Should I do it?

Dreamsorcerer commented 1 year ago

aiohttp-jinja2 has static(): https://aiohttp-jinja2.readthedocs.io/en/stable/#default-globals

So, if you can update to that, that would be perfect.

14droplets commented 1 year ago

Thank you, interesting feature. But using that would require to mess with app['static_root_url'] which may be used by user. I will try to do something like this but autonomous

Dreamsorcerer commented 1 year ago

Oh, yes, we can't use that until #71 is done. Will need to stick with a hack for now.

14droplets commented 1 year ago

Hacky hack goes brrr. As hacky as I could, but without ANY additional pollution to globals