From a572fd33c7fefe9bdcb1600c12cdcbb1eb8922b0 Mon Sep 17 00:00:00 2001
From: Joseph Schorr <joseph.schorr@coreos.com>
Date: Sun, 8 Jul 2018 15:20:02 +0300
Subject: [PATCH] Ensure that we limit the length of repository names

Until now, they'd simply be truncated by the database. Now, we properly check their lengths.

Fixes https://jira.coreos.com/browse/QUAY-963
---
 endpoints/api/test/test_repository.py | 21 +++++++++++++++++++++
 test/registry/protocol_v1.py          |  3 ++-
 test/registry/protocol_v2.py          |  1 +
 test/registry/protocols.py            |  1 +
 test/registry/registry_tests.py       | 20 +++++++++++++++-----
 util/names.py                         |  2 +-
 6 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/endpoints/api/test/test_repository.py b/endpoints/api/test/test_repository.py
index d98153a5c..2981d7e22 100644
--- a/endpoints/api/test/test_repository.py
+++ b/endpoints/api/test/test_repository.py
@@ -96,3 +96,24 @@ def test_list_starred_repos(client):
     repos = {r['namespace'] + '/' + r['name'] for r in response['repositories']}
     assert 'devtable/simple' in repos
     assert 'public/publicrepo' not in repos
+
+
+@pytest.mark.parametrize('repo_name, expected_status', [
+  pytest.param('x' * 255, 201, id='Maximum allowed length'),
+  pytest.param('x' * 256, 400, id='Over allowed length'),
+  pytest.param('a|b', 400, id='Invalid name'),
+])
+def test_create_repository(repo_name, expected_status, client):
+  with client_with_identity('devtable', client) as cl:
+    body = {
+      'namespace': 'devtable',
+      'repository': repo_name,
+      'visibility': 'public',
+      'description': 'foo',
+    }
+
+    result = conduct_api_call(client, RepositoryList, 'post', None, body,
+                              expected_code=expected_status).json
+    if expected_status == 201:
+      assert result['name'] == repo_name
+      assert model.repository.get_repository('devtable', repo_name).name == repo_name
diff --git a/test/registry/protocol_v1.py b/test/registry/protocol_v1.py
index 4404024a1..d5b776c54 100644
--- a/test/registry/protocol_v1.py
+++ b/test/registry/protocol_v1.py
@@ -21,7 +21,8 @@ class V1Protocol(RegistryProtocol):
       Failures.UNAUTHENTICATED: 401,
       Failures.UNAUTHORIZED: 403,
       Failures.APP_REPOSITORY: 405,
-      Failures.INVALID_REPOSITORY: 404,
+      Failures.SLASH_REPOSITORY: 404,
+      Failures.INVALID_REPOSITORY: 400,
       Failures.DISALLOWED_LIBRARY_NAMESPACE: 400,
       Failures.NAMESPACE_DISABLED: 400,
     },
diff --git a/test/registry/protocol_v2.py b/test/registry/protocol_v2.py
index 6f5fb0049..045a3ba3c 100644
--- a/test/registry/protocol_v2.py
+++ b/test/registry/protocol_v2.py
@@ -28,6 +28,7 @@ class V2Protocol(RegistryProtocol):
       Failures.APP_REPOSITORY: 405,
       Failures.ANONYMOUS_NOT_ALLOWED: 401,
       Failures.INVALID_REPOSITORY: 400,
+      Failures.SLASH_REPOSITORY: 400,
       Failures.NAMESPACE_DISABLED: 400,
     },
     V2ProtocolSteps.MOUNT_BLOB: {
diff --git a/test/registry/protocols.py b/test/registry/protocols.py
index 833a4dc4f..2cd073441 100644
--- a/test/registry/protocols.py
+++ b/test/registry/protocols.py
@@ -41,6 +41,7 @@ class Failures(Enum):
   UNAUTHORIZED = 'unauthorized'
   INVALID_REGISTRY = 'invalid-registry'
   INVALID_REPOSITORY = 'invalid-repository'
+  SLASH_REPOSITORY = 'slash-repository'
   APP_REPOSITORY = 'app-repository'
   UNKNOWN_TAG = 'unknown-tag'
   ANONYMOUS_NOT_ALLOWED = 'anonymous-not-allowed'
diff --git a/test/registry/registry_tests.py b/test/registry/registry_tests.py
index 9a7e6a41e..15c4d2531 100644
--- a/test/registry/registry_tests.py
+++ b/test/registry/registry_tests.py
@@ -362,15 +362,25 @@ def test_image_replication(pusher, basic_images, liveserver_session, app_reloade
       assert r.text == 'OK'
 
 
-def test_push_reponame_with_slashes(pusher, basic_images, liveserver_session, app_reloader):
-  """ Test: Attempt to add a repository name with slashes. This should fail as we do not
-      support it.
+@pytest.mark.parametrize('repo_name, expected_failure', [
+  ('something', None),
+  ('some/slash', Failures.SLASH_REPOSITORY),
+  pytest.param('x' * 255, None, id='Valid long name'),
+  pytest.param('x' * 256, Failures.INVALID_REPOSITORY, id='Name too long'),
+])
+def test_push_reponame(repo_name, expected_failure, pusher, puller, basic_images,
+                       liveserver_session, app_reloader):
+  """ Test: Attempt to add a repository with various names.
   """
   credentials = ('devtable', 'password')
 
-  pusher.push(liveserver_session, 'devtable', 'some/slash', 'latest', basic_images,
+  pusher.push(liveserver_session, 'devtable', repo_name, 'latest', basic_images,
               credentials=credentials,
-              expected_failure=Failures.INVALID_REPOSITORY)
+              expected_failure=expected_failure)
+
+  if expected_failure is None:
+    puller.pull(liveserver_session, 'devtable', repo_name, 'latest', basic_images,
+                credentials=credentials)
 
 
 @pytest.mark.parametrize('tag_name, expected_failure', [
diff --git a/util/names.py b/util/names.py
index e8eb72645..b02fef9c1 100644
--- a/util/names.py
+++ b/util/names.py
@@ -5,7 +5,7 @@ import anunidecode # Don't listen to pylint's lies. This import is required for
 
 from uuid import uuid4
 
-REPOSITORY_NAME_REGEX = re.compile(r'^[\.a-zA-Z0-9_-]+$')
+REPOSITORY_NAME_REGEX = re.compile(r'^[\.a-zA-Z0-9_-]{1,255}$')
 
 VALID_TAG_PATTERN = r'[\w][\w.-]{0,127}'
 FULL_TAG_PATTERN = r'^[\w][\w.-]{0,127}$'