From ec2a1bd42a5f2c9e92fb0f153c305e55bf1e90ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Fri, 4 Jan 2019 16:40:02 +0100 Subject: [PATCH] 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 182fb11c80935f2ef1661beaf1151969a24e833f. 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. --- pelican/paginator.py | 12 +++++++++++- pelican/tests/test_paginator.py | 5 +++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/pelican/paginator.py b/pelican/paginator.py index ceabf43b..fe63863e 100644 --- a/pelican/paginator.py +++ b/pelican/paginator.py @@ -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')) diff --git a/pelican/tests/test_paginator.py b/pelican/tests/test_paginator.py index e06075fe..4e3ef035 100644 --- a/pelican/tests/test_paginator.py +++ b/pelican/tests/test_paginator.py @@ -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/')