From 96bf21c1da7ca3d832662c036e47a32b2d706981 Mon Sep 17 00:00:00 2001
From: Markus Unterwaditzer <markus@unterwaditzer.net>
Date: Tue, 6 Dec 2016 15:58:16 +0100
Subject: [PATCH] Dont repair unsafe UIDs by default, fix #527

---
 CHANGELOG.rst              |  6 ++++++
 tests/test_repair.py       |  4 ++--
 vdirsyncer/cli/__init__.py | 15 ++++++++++-----
 vdirsyncer/cli/tasks.py    |  4 ++--
 vdirsyncer/repair.py       | 10 +++++++---
 5 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 6defbe8..5c8c5d0 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -9,6 +9,12 @@ Package maintainers and users who have to manually update their installation
 may want to subscribe to `GitHub's tag feed
 <https://github.com/pimutils/vdirsyncer/tags.atom>`_.
 
+Version 0.13.1
+==============
+
+- ``vdirsyncer repair`` no longer changes "unsafe" UIDs by default, an extra
+  option has to be specified. See :gh:`527`.
+
 Version 0.14.0
 ==============
 
diff --git a/tests/test_repair.py b/tests/test_repair.py
index 6d8f837..6a0d97a 100644
--- a/tests/test_repair.py
+++ b/tests/test_repair.py
@@ -28,7 +28,7 @@ def test_repair_uids(uid):
     uid1, uid2 = [s.get(href)[0].uid for href, etag in s.list()]
     assert uid1 == uid2
 
-    repair_storage(s)
+    repair_storage(s, repair_unsafe_uid=False)
 
     uid1, uid2 = [s.get(href)[0].uid for href, etag in s.list()]
     assert uid1 != uid2
@@ -44,7 +44,7 @@ def test_repair_unsafe_uids(uid):
     assert s.get(href)[0].uid == uid
     assert not href_safe(uid)
 
-    repair_storage(s)
+    repair_storage(s, repair_unsafe_uid=True)
 
     new_href = list(s.list())[0][0]
     assert href_safe(new_href)
diff --git a/vdirsyncer/cli/__init__.py b/vdirsyncer/cli/__init__.py
index 3537037..47e661e 100644
--- a/vdirsyncer/cli/__init__.py
+++ b/vdirsyncer/cli/__init__.py
@@ -207,16 +207,20 @@ def discover(ctx, pairs, max_workers, list):
 
 @app.command()
 @click.argument('collection')
+@click.option('--repair-unsafe-uid/--no-repair-unsafe-uid', default=False,
+              help=('Some characters in item UIDs and URLs may cause problems '
+                    'with buggy software. Adding this option will reassign '
+                    'new UIDs to those items.'))
 @pass_context
 @catch_errors
-def repair(ctx, collection):
+def repair(ctx, collection, repair_unsafe_uid):
     '''
     Repair a given collection.
 
     Runs a few checks on the collection and applies some fixes to individual
     items that may improve general stability, also with other CalDAV/CardDAV
     clients. In particular, if you encounter URL-encoding-related issues with
-    other clients, this command might help.
+    other clients, this command with --repair-unsafe-uid might help.
 
     Example: `vdirsyncer repair calendars_local/foo` repairs the `foo`
     collection of the `calendars_local` storage.
@@ -224,7 +228,8 @@ def repair(ctx, collection):
     from .tasks import repair_collection
 
     cli_logger.warning('This operation will take a very long time.')
-    cli_logger.warning('It\'s recommended to turn off other client\'s '
-                       'synchronization features.')
+    cli_logger.warning('It\'s recommended to make a backup and '
+                       'turn off other client\'s synchronization features.')
     click.confirm('Do you want to continue?', abort=True)
-    repair_collection(ctx.config, collection)
+    repair_collection(ctx.config, collection,
+                      repair_unsafe_uid=repair_unsafe_uid)
diff --git a/vdirsyncer/cli/tasks.py b/vdirsyncer/cli/tasks.py
index 7d4cd12..4cde106 100644
--- a/vdirsyncer/cli/tasks.py
+++ b/vdirsyncer/cli/tasks.py
@@ -93,7 +93,7 @@ def discover_collections(wq, pair, **kwargs):
                     .format(pair.name, json.dumps(collections)))
 
 
-def repair_collection(config, collection):
+def repair_collection(config, collection, repair_unsafe_uid):
     from ..repair import repair_storage
 
     storage_name, collection = collection, None
@@ -120,7 +120,7 @@ def repair_collection(config, collection):
 
     cli_logger.info('Repairing {}/{}'.format(storage_name, collection))
     cli_logger.warning('Make sure no other program is talking to the server.')
-    repair_storage(storage)
+    repair_storage(storage, repair_unsafe_uid=repair_unsafe_uid)
 
 
 def metasync_collection(wq, collection, general):
diff --git a/vdirsyncer/repair.py b/vdirsyncer/repair.py
index 33240b1..4470143 100644
--- a/vdirsyncer/repair.py
+++ b/vdirsyncer/repair.py
@@ -8,7 +8,7 @@ from .utils import generate_href, href_safe
 logger = logging.getLogger(__name__)
 
 
-def repair_storage(storage):
+def repair_storage(storage, repair_unsafe_uid):
     seen_uids = set()
     all_hrefs = list(storage.list())
     for i, (href, _) in enumerate(all_hrefs):
@@ -30,8 +30,12 @@ def repair_storage(storage):
             logger.warning('Duplicate UID, assigning random one.')
             new_item = item.with_uid(generate_href())
         elif not href_safe(item.uid) or not href_safe(basename(href)):
-            logger.warning('UID or href is unsafe, assigning random UID.')
-            new_item = item.with_uid(generate_href(item.uid))
+            if not repair_unsafe_uid:
+                logger.warning('UID or href may cause problems, add '
+                               '--repair-unsafe-hrefs to repair.')
+            else:
+                logger.warning('UID or href is unsafe, assigning random UID.')
+                new_item = item.with_uid(generate_href(item.uid))
 
         if not new_item.uid:
             logger.error('Item {!r} is malformed beyond repair. '