Handle paths with trailing /

[stsewd]

No description provided.

[ericholscher]

This seems hacky -- why are we doing this here?

[stsewd]

the slash gets captured in

readthedocs.org/readthedocs/proxito/urls.py

Line 94 in dd66c22

r'^_proxito_404_(?P<proxito_path>.*)$',

and it can end up with a slash in

readthedocs.org/readthedocs/proxito/views/utils.py

Line 61 in dd66c22

filename = f'{lang_slug}/{version_slug}/{filename}'

[ericholscher]

We should attempt to fix that closer to where it's happening then. Perhaps os.path.join was the proper logic after all?

We are calling this function a lot of places, and I don't want to put the logic here because it might cause other unintended issues.

[stsewd]

os.path.join depends on the running OS to add / or \, it could also happen to add an extra / if it thinks it's a directory or the root. Changed to just strip the file before concatenate it.

[humitos]

We have been hitting these kind of issues a lot when manipulating URLs, do you think that using a 3rd party library and follow the same pattern everywhere (*) in our code could help us?

3rd party package example: https://github.com/homm/yurl/

(*) do not use os.path.join, nor "...".format() nor "..." + "..." etc, that we have mixtures of all of them --and use a package like this intead

[stsewd]

Looks like doesn't work like we want

>>> print(yurl.URL('foo/bar') + yurl.URL('bar'))
foo/bar
>>> print(yurl.URL('foo') + yurl.URL('bar'))
bar
>>> print(yurl.URL('foo/') + yurl.URL('bar'))
foo/bar
>>> print(yurl.URL('foo/') + yurl.URL('/bar'))
/bar

I think we could just write a little function to handle it.

[humitos]

Yeah. I haven't tried that particular one, but there are many similar to this one.

[ericholscher]

This looks like a better fix, at least for the current bug 👍