From 2645fafc38ecf13c64c3c7ebdb890af97c3486b1 Mon Sep 17 00:00:00 2001
From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Fri, 4 Aug 2017 13:28:56 -0400
Subject: [PATCH] Raise if manual revision id contains dashes or at signs

A :class:`.CommandError` is raised if the "--rev-id" passed to the
:func:`.revision` command contains dashes or at-signs, as this interferes
with the command notation used to locate revisions.

Change-Id: I60a794a5c80bf47b149998b8c5cb04ecbfd05bfa
Fixes: #441
---
 alembic/script/base.py          | 10 +++++++++-
 alembic/script/revision.py      | 13 +++++++++++++
 docs/build/unreleased/441.rst   |  7 +++++++
 tests/test_script_production.py | 29 +++++++++++++++++++++++++++++
 4 files changed, 58 insertions(+), 1 deletion(-)
 create mode 100644 docs/build/unreleased/441.rst

diff --git a/alembic/script/base.py b/alembic/script/base.py
index 64486851..d296a0f0 100644
--- a/alembic/script/base.py
+++ b/alembic/script/base.py
@@ -489,6 +489,11 @@ class ScriptDirectory(object):
         if head is None:
             head = "head"
 
+        try:
+            Script.verify_rev_id(revid)
+        except revision.RevisionError as err:
+            compat.raise_from_cause(util.CommandError(err.args[0]))
+
         with self._catch_revision_errors(multiple_heads=(
             "Multiple heads are present; please specify the head "
             "revision on which the new revision should be based, "
@@ -562,7 +567,10 @@ class ScriptDirectory(object):
             message=message if message is not None else ("empty message"),
             **kw
         )
-        script = Script._from_path(self, path)
+        try:
+            script = Script._from_path(self, path)
+        except revision.RevisionError as err:
+            compat.raise_from_cause(util.CommandError(err.args[0]))
         if branch_labels and not script.branch_labels:
             raise util.CommandError(
                 "Version %s specified branch_labels %s, however the "
diff --git a/alembic/script/revision.py b/alembic/script/revision.py
index 608e16bc..9145f551 100644
--- a/alembic/script/revision.py
+++ b/alembic/script/revision.py
@@ -6,6 +6,7 @@ from sqlalchemy import util as sqlautil
 from ..util import compat
 
 _relative_destination = re.compile(r'(?:(.+?)@)?(\w+)?((?:\+|-)\d+)')
+_revision_illegal_chars = ['@', '-']
 
 
 class RevisionError(Exception):
@@ -821,9 +822,21 @@ class Revision(object):
     """Optional string/tuple of symbolic names to apply to this
     revision's branch"""
 
+    @classmethod
+    def verify_rev_id(cls, revision):
+        illegal_chars = set(revision).intersection(_revision_illegal_chars)
+        if illegal_chars:
+            raise RevisionError(
+                "Character(s) '%s' not allowed in revision identifier '%s'" % (
+                    ", ".join(sorted(illegal_chars)),
+                    revision
+                )
+            )
+
     def __init__(
             self, revision, down_revision,
             dependencies=None, branch_labels=None):
+        self.verify_rev_id(revision)
         self.revision = revision
         self.down_revision = tuple_rev_as_scalar(down_revision)
         self.dependencies = tuple_rev_as_scalar(dependencies)
diff --git a/docs/build/unreleased/441.rst b/docs/build/unreleased/441.rst
new file mode 100644
index 00000000..6c46e354
--- /dev/null
+++ b/docs/build/unreleased/441.rst
@@ -0,0 +1,7 @@
+.. change::
+    :tags: bug, commands
+    :tickets: 441
+
+    A :class:`.CommandError` is raised if the "--rev-id" passed to the
+    :func:`.revision` command contains dashes or at-signs, as this interferes
+    with the command notation used to locate revisions.
\ No newline at end of file
diff --git a/tests/test_script_production.py b/tests/test_script_production.py
index 3364d566..c5a99b86 100644
--- a/tests/test_script_production.py
+++ b/tests/test_script_production.py
@@ -255,6 +255,35 @@ class RevisionCommandTest(TestBase):
             self.cfg, message="some message", head=self.b
         )
 
+    def test_illegal_revision_chars(self):
+        assert_raises_message(
+            util.CommandError,
+            r"Character\(s\) '-' not allowed in "
+            "revision identifier 'no-dashes'",
+            command.revision,
+            self.cfg, message="some message", rev_id="no-dashes"
+        )
+
+        assert not os.path.exists(
+            os.path.join(
+                self.env.dir, "versions", "no-dashes_some_message.py"))
+
+        assert_raises_message(
+            util.CommandError,
+            r"Character\(s\) '@' not allowed in "
+            "revision identifier 'no@atsigns'",
+            command.revision,
+            self.cfg, message="some message", rev_id="no@atsigns"
+        )
+
+        assert_raises_message(
+            util.CommandError,
+            r"Character\(s\) '-, @' not allowed in revision "
+            "identifier 'no@atsigns-ordashes'",
+            command.revision,
+            self.cfg, message="some message", rev_id="no@atsigns-ordashes"
+        )
+
     def test_create_script_branches(self):
         rev = command.revision(
             self.cfg, message="some message", branch_label="foobar")
-- 
2.47.2