PICARD-2960: Prevent dir separator replacement being set to dir separator (2.x backport)

picard/__init__.py

@@ -40,7 +40,7 @@
 PICARD_DISPLAY_NAME = "MusicBrainz Picard"
 PICARD_APP_ID = "org.musicbrainz.Picard"
 PICARD_DESKTOP_NAME = PICARD_APP_ID + ".desktop"
-PICARD_VERSION = Version(2, 12, 2, 'final', 0)
+PICARD_VERSION = Version(2, 12, 3, 'dev', 1)
 
 
 # optional build version

picard/config_upgrade.py

@@ -26,7 +26,7 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
 
-
+import os
 import re
 
 from PyQt5 import QtWidgets
@@ -46,6 +46,8 @@
 from picard.const.sys import IS_FROZEN
 from picard.util import unique_numbered_title
 
+from picard.ui.options.renaming_compat import DEFAULT_REPLACEMENT
+
 
 # TO ADD AN UPGRADE HOOK:
 # ----------------------
@@ -478,6 +480,15 @@ def upgrade_to_v2_10_1_dev_2(config):
     config.persist['splitters_OptionsDialog'] = b''
 
 
+def upgrade_to_v2_12_3_dev_1(config):
+    """Ensure "replace_dir_separator" contains no directory separator"""
+    replace_dir_separator = config.setting['replace_dir_separator']
+    replace_dir_separator = replace_dir_separator.replace(os.sep, DEFAULT_REPLACEMENT)
+    if os.altsep:
+        replace_dir_separator = replace_dir_separator.replace(os.altsep, DEFAULT_REPLACEMENT)
+    config.setting['replace_dir_separator'] = replace_dir_separator
+
+
 def rename_option(config, old_opt, new_opt, option_type, default):
     _s = config.setting
     if old_opt in _s:
@@ -524,4 +535,5 @@ def upgrade_config(config):
     cfg.register_upgrade_hook(upgrade_to_v2_8_0_dev_2)
     cfg.register_upgrade_hook(upgrade_to_v2_9_0_alpha_2)
     cfg.register_upgrade_hook(upgrade_to_v2_10_1_dev_2)
+    cfg.register_upgrade_hook(upgrade_to_v2_12_3_dev_1)
     cfg.run_upgrade_hooks(log.debug)

picard/ui/options/renaming_compat.py

@@ -4,7 +4,7 @@
 #
 # Copyright (C) 2006-2008, 2011 Lukáš Lalinský
 # Copyright (C) 2008-2009 Nikolai Prokoschenko
-# Copyright (C) 2009-2010, 2014-2015, 2018-2022 Philipp Wolfer
+# Copyright (C) 2009-2010, 2014-2015, 2018-2024 Philipp Wolfer
 # Copyright (C) 2011-2013 Michael Wiencek
 # Copyright (C) 2011-2013 Wieland Hoffmann
 # Copyright (C) 2013 Calvin Walton
@@ -32,7 +32,7 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
 
-
+import os
 import re
 
 from PyQt5 import (
@@ -52,6 +52,7 @@
 
 from picard.ui import PicardDialog
 from picard.ui.options import (
+    OptionsCheckError,
     OptionsPage,
     register_options_page,
 )
@@ -100,6 +101,7 @@ def __init__(self, parent=None):
         self.ui.windows_long_paths.toggled.connect(self.on_options_changed)
         self.ui.replace_spaces_with_underscores.toggled.connect(self.on_options_changed)
         self.ui.replace_dir_separator.textChanged.connect(self.on_options_changed)
+        self.ui.replace_dir_separator.setValidator(NoDirectorySeparatorValidator())
         self.ui.btn_windows_compatibility_change.clicked.connect(self.open_win_compat_dialog)
 
     def load(self):
@@ -126,6 +128,14 @@ def save(self):
         for key, value in options.items():
             config.setting[key] = value
 
+    def check(self):
+        (valid_state, _text, _pos) = self.ui.replace_dir_separator.validator().validate(self.ui.replace_dir_separator.text(), 0)
+        if valid_state != QtGui.QValidator.State.Acceptable:
+            raise OptionsCheckError(
+                _("Invalid directory separator replacement"),
+                _("The replacement for directory separators must not be itself a directory separator.")
+            )
+
     def toggle_windows_long_paths(self, state):
         if state and not system_supports_long_paths():
             dialog = QtWidgets.QMessageBox(
@@ -160,6 +170,15 @@ def open_win_compat_dialog(self):
             self.on_options_changed()
 
 
+class NoDirectorySeparatorValidator(QtGui.QValidator):
+    def validate(self, text: str, pos):
+        if os.sep in text or (os.altsep and os.altsep in text):
+            state = QtGui.QValidator.State.Invalid
+        else:
+            state = QtGui.QValidator.State.Acceptable
+        return state, text, pos
+
+
 class WinCompatReplacementValidator(QtGui.QValidator):
     _re_valid_win_replacement = re.compile(r'^[^"*:<>?|/\\\s]?$')
 

test/test_config_upgrade.py

@@ -21,6 +21,7 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
 
+import os
 
 from PyQt5.QtCore import QByteArray
 
@@ -58,13 +59,16 @@
     upgrade_to_v2_6_0_beta_3,
     upgrade_to_v2_6_0_dev_1,
     upgrade_to_v2_8_0_dev_2,
+    upgrade_to_v2_12_3_dev_1,
 )
 from picard.const import (
     DEFAULT_FILE_NAMING_FORMAT,
     DEFAULT_SCRIPT_NAME,
 )
 from picard.util import unique_numbered_title
 
+from picard.ui.options.renaming_compat import DEFAULT_REPLACEMENT
+
 
 class TestPicardConfigUpgrades(TestPicardConfigCommon):
 
@@ -364,3 +368,14 @@ def test_upgrade_to_v2_8_0_dev_2(self):
         self.assertEqual(expected, self.config.setting['toolbar_layout'])
         upgrade_to_v2_8_0_dev_2(self.config)
         self.assertEqual(expected, self.config.setting['toolbar_layout'])
+
+    def test_upgrade_to_v2_12_3_dev_1(self):
+        TextOption('setting', 'replace_dir_separator', DEFAULT_REPLACEMENT)
+        self.config.setting['replace_dir_separator'] = os.sep
+        upgrade_to_v2_12_3_dev_1(self.config)
+        self.assertEqual(DEFAULT_REPLACEMENT, self.config.setting['replace_dir_separator'])
+
+        if os.altsep:
+            self.config.setting['replace_dir_separator'] = os.altsep
+            upgrade_to_v2_12_3_dev_1(self.config)
+            self.assertEqual(DEFAULT_REPLACEMENT, self.config.setting['replace_dir_separator'])