pagination: this lstrip() was way too hungry, eating absolute URLs.

Reverts back to how pagination worked for the {url} placeholder as I did
it in 182fb11c80. Absolute URLs with
one or two leading slashes were eaten by lstrip() and became relative,
which then caused broken links in my case.

Added extra comments to this piece of code (*and* the test) to make it
less likely that someone breaks this again in the future.
This commit is contained in:
Vladimír Vondruš 2019-01-04 16:40:02 +01:00
commit ec2a1bd42a
2 changed files with 14 additions and 3 deletions

View file

@ -146,7 +146,17 @@ class Page(object):
}
ret = prop_value.format(**context)
ret = ret.lstrip('/')
# Remove a single leading slash, if any. This is done for backwards
# compatibility reasons. If a leading slash is needed (for URLs
# relative to server root or absolute URLs without the scheme such as
# //blog.my.site/), it can be worked around by prefixing the pagination
# pattern by an additional slash (which then gets removed, preserving
# the other slashes). This also means the following code *can't* be
# changed to lstrip() because that would remove all leading slashes and
# thus make the workaround impossible. See
# test_custom_pagination_pattern() for a verification of this.
if ret[0] == '/':
ret = ret[1:]
return ret
url = property(functools.partial(_from_settings, key='URL'))

View file

@ -72,9 +72,10 @@ class TestPage(unittest.TestCase):
Article(**self.page_kwargs)]
paginator = Paginator('blog/index.html', '//blog.my.site/',
object_list, settings, 1)
# The URL *has to* stay absolute (with // in the front), so verify that
page1 = paginator.page(1)
self.assertEqual(page1.save_as, 'blog/index.html')
self.assertEqual(page1.url, 'blog.my.site/')
self.assertEqual(page1.url, '//blog.my.site/')
page2 = paginator.page(2)
self.assertEqual(page2.save_as, 'blog/2/index.html')
self.assertEqual(page2.url, 'blog.my.site/2/')
self.assertEqual(page2.url, '//blog.my.site/2/')