From b1d44c1c871acee8687b93ba34d6354f1a56d63f Mon Sep 17 00:00:00 2001
From: Andrew Jorgensen <andrew@jorgensenfamily.us>
Date: Fri, 9 Nov 2018 10:33:36 -0800
Subject: [PATCH] Replace %s rather than fallback to defaults

3a0add4b6e66fe08e9f5710f98235491c09e4f81 caused existing configs to fall
back to defaults. But since we know exactly how to fix the user config
so that the behavior doesn't change, we should do so, while still
warning that use of %s is deprecated.

Also fixes a bug where we tried to look for %s in None.
---
 pelican/settings.py            | 40 +++++++++++++++++++++++++++++-----
 pelican/tests/test_settings.py |  9 ++++++++
 2 files changed, 43 insertions(+), 6 deletions(-)

diff --git a/pelican/settings.py b/pelican/settings.py
index f681a182..b963007a 100644
--- a/pelican/settings.py
+++ b/pelican/settings.py
@@ -245,6 +245,22 @@ def get_jinja_environment(settings):
     return settings
 
 
+def _printf_s_to_format_field(printf_string, format_field):
+    """Tries to replace %s with {format_field} in the provided printf_string.
+    Raises ValueError in case of failure.
+    """
+    TEST_STRING = 'PELICAN_PRINTF_S_DEPRECATION'
+    expected = printf_string % TEST_STRING
+
+    result = printf_string.replace('{', '{{').replace('}', '}}') \
+        % '{{{}}}'.format(format_field)
+    if result.format(**{format_field: TEST_STRING}) != expected:
+        raise ValueError('Failed to safely replace %s with {{{}}}'.format(
+            format_field))
+
+    return result
+
+
 def handle_deprecated_settings(settings):
     """Converts deprecated settings and issues warnings. Issues an exception
     if both old and new setting is specified.
@@ -394,10 +410,16 @@ def handle_deprecated_settings(settings):
     for key in ['TRANSLATION_FEED_ATOM',
                 'TRANSLATION_FEED_RSS'
                 ]:
-        if key in settings and '%s' in settings[key]:
+        if settings.get(key) and '%s' in settings[key]:
             logger.warning('%%s usage in %s is deprecated, use {lang} '
-                           'instead. Falling back to default.', key)
-            settings[key] = DEFAULT_CONFIG[key]
+                           'instead.', key)
+            try:
+                settings[key] = _printf_s_to_format_field(
+                    settings[key], 'lang')
+            except ValueError:
+                logger.warning('Failed to convert %%s to {lang} for %s. '
+                               'Falling back to default.', key)
+                settings[key] = DEFAULT_CONFIG[key]
     for key in ['AUTHOR_FEED_ATOM',
                 'AUTHOR_FEED_RSS',
                 'CATEGORY_FEED_ATOM',
@@ -405,10 +427,16 @@ def handle_deprecated_settings(settings):
                 'TAG_FEED_ATOM',
                 'TAG_FEED_RSS',
                 ]:
-        if key in settings and '%s' in settings[key]:
+        if settings.get(key) and '%s' in settings[key]:
             logger.warning('%%s usage in %s is deprecated, use {slug} '
-                           'instead. Falling back to default.', key)
-            settings[key] = DEFAULT_CONFIG[key]
+                           'instead.', key)
+            try:
+                settings[key] = _printf_s_to_format_field(
+                    settings[key], 'slug')
+            except ValueError:
+                logger.warning('Failed to convert %%s to {slug} for %s. '
+                               'Falling back to default.', key)
+                settings[key] = DEFAULT_CONFIG[key]
 
     return settings
 
diff --git a/pelican/tests/test_settings.py b/pelican/tests/test_settings.py
index a92cf09c..21759d40 100644
--- a/pelican/tests/test_settings.py
+++ b/pelican/tests/test_settings.py
@@ -9,6 +9,7 @@ from sys import platform
 
 
 from pelican.settings import (DEFAULT_CONFIG, DEFAULT_THEME,
+                              _printf_s_to_format_field,
                               configure_settings, handle_deprecated_settings,
                               read_settings)
 from pelican.tests.support import unittest
@@ -168,6 +169,14 @@ class TestSettingsConfiguration(unittest.TestCase):
 
         self.assertRaises(Exception, configure_settings, settings)
 
+    def test__printf_s_to_format_field(self):
+        for s in ('%s', '{%s}', '{%s'):
+            option = 'foo/{}/bar.baz'.format(s)
+            result = _printf_s_to_format_field(option, 'slug')
+            expected = option % 'qux'
+            found = result.format(slug='qux')
+            self.assertEqual(expected, found)
+
     def test_deprecated_extra_templates_paths(self):
         settings = self.settings
         settings['EXTRA_TEMPLATES_PATHS'] = ['/foo/bar', '/ha']