You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by jo...@apache.org on 2014/01/30 22:38:57 UTC
[2/2] git commit: [#7118] Added validation to confirm GitHub project
exists and is readable
[#7118] Added validation to confirm GitHub project exists and is readable
Signed-off-by: Cory Johns <cj...@slashdotmedia.com>
Project: http://git-wip-us.apache.org/repos/asf/incubator-allura/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-allura/commit/ca119b44
Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/ca119b44
Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/ca119b44
Branch: refs/heads/cj/7118
Commit: ca119b44ebc8711bda04a6cc907de53a2ba85c43
Parents: a5119f4
Author: Cory Johns <cj...@slashdotmedia.com>
Authored: Thu Jan 30 21:14:26 2014 +0000
Committer: Cory Johns <cj...@slashdotmedia.com>
Committed: Thu Jan 30 21:38:37 2014 +0000
----------------------------------------------------------------------
.../forgeimporters/github/__init__.py | 26 ++++++++++++++++++++
ForgeImporters/forgeimporters/github/code.py | 8 ++++--
ForgeImporters/forgeimporters/github/project.py | 15 ++++++-----
.../forgeimporters/github/tests/test_code.py | 18 ++++++++++----
.../forgeimporters/github/tests/test_tracker.py | 9 +++++--
.../forgeimporters/github/tests/test_wiki.py | 11 +++++++--
ForgeImporters/forgeimporters/github/tracker.py | 14 ++++++++---
ForgeImporters/forgeimporters/github/wiki.py | 8 ++++--
.../forgeimporters/google/__init__.py | 8 +++---
.../tests/github/test_extractor.py | 7 ++++++
10 files changed, 96 insertions(+), 28 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/ca119b44/ForgeImporters/forgeimporters/github/__init__.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/github/__init__.py b/ForgeImporters/forgeimporters/github/__init__.py
index 2d4e85c..d511e8b 100644
--- a/ForgeImporters/forgeimporters/github/__init__.py
+++ b/ForgeImporters/forgeimporters/github/__init__.py
@@ -26,12 +26,34 @@ from tg import config, session, redirect, request, expose
from tg.decorators import without_trailing_slash
from pylons import tmpl_context as c
from requests_oauthlib import OAuth2Session
+import requests
+from formencode import validators as fev
from forgeimporters import base
log = logging.getLogger(__name__)
+class GitHubProjectNameValidator(fev.FancyValidator):
+ not_empty = True
+ messages = {
+ 'invalid': 'Valid symbols are: letters, numbers, dashes, '
+ 'underscores and periods',
+ 'unavailable': 'This project is unavailable for import',
+ }
+
+ def _to_python(self, value, state=None):
+ user_name = state.full_dict.get('user_name', '').strip()
+ project_name = value.strip()
+ full_project_name = '%s/%s' % (user_name, project_name)
+ if not re.match(r'^[a-z0-9-_.]+$', project_name):
+ raise fev.Invalid(self.message('invalid', state), value, state)
+
+ if not GitHubProjectExtractor(full_project_name).check_readable():
+ raise fev.Invalid(self.message('unavailable', state), value, state)
+ return project_name
+
+
class GitHubProjectExtractor(base.ProjectExtractor):
PAGE_MAP = {
'project_info': 'https://api.github.com/repos/{project_name}',
@@ -83,6 +105,10 @@ class GitHubProjectExtractor(base.ProjectExtractor):
return self.urlopen(url, **kw)
return resp
+ def check_readable(self):
+ resp = requests.head(self.get_page_url('project_info'))
+ return resp.status_code == 200
+
def get_next_page_url(self, link):
if not link:
return
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/ca119b44/ForgeImporters/forgeimporters/github/code.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/github/code.py b/ForgeImporters/forgeimporters/github/code.py
index ede36b4..710e868 100644
--- a/ForgeImporters/forgeimporters/github/code.py
+++ b/ForgeImporters/forgeimporters/github/code.py
@@ -39,12 +39,16 @@ from forgeimporters.base import (
ToolImporter,
ToolImportForm,
)
-from forgeimporters.github import GitHubProjectExtractor, GitHubOAuthMixin
+from forgeimporters.github import (
+ GitHubProjectExtractor,
+ GitHubOAuthMixin,
+ GitHubProjectNameValidator,
+)
class GitHubRepoImportForm(ToolImportForm):
- gh_project_name = fev.UnicodeString(not_empty=True)
gh_user_name = fev.UnicodeString(not_empty=True)
+ gh_project_name = GitHubProjectNameValidator()
class GitHubRepoImportController(BaseController, GitHubOAuthMixin):
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/ca119b44/ForgeImporters/forgeimporters/github/project.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/github/project.py b/ForgeImporters/forgeimporters/github/project.py
index fc7247b..003adae 100644
--- a/ForgeImporters/forgeimporters/github/project.py
+++ b/ForgeImporters/forgeimporters/github/project.py
@@ -24,20 +24,19 @@ from tg.decorators import with_trailing_slash
from allura.lib.decorators import require_post
-from .. import base
-from . import tasks
-from . import GitHubOAuthMixin
+from forgeimporters import base
+from forgeimporters.github import (
+ tasks,
+ GitHubOAuthMixin,
+ GitHubProjectNameValidator,
+)
log = logging.getLogger(__name__)
class GitHubProjectForm(base.ProjectImportForm):
- project_name = fev.Regex(r'^[a-zA-Z0-9-_.]+$',
- not_empty=True,
- messages={
- 'invalid': 'Valid symbols are: letters, numbers, dashes, underscores and periods',
- })
+ project_name = GitHubProjectNameValidator()
class GitHubProjectImporter(base.ProjectImporter, GitHubOAuthMixin):
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/ca119b44/ForgeImporters/forgeimporters/github/tests/test_code.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/github/tests/test_code.py b/ForgeImporters/forgeimporters/github/tests/test_code.py
index 8fa6a2f..bb8ed25 100644
--- a/ForgeImporters/forgeimporters/github/tests/test_code.py
+++ b/ForgeImporters/forgeimporters/github/tests/test_code.py
@@ -75,8 +75,10 @@ class TestGitHubImportController(TestController, TestCase):
self.assertIsNotNone(r.html.find(attrs=dict(name="mount_point")))
@with_git
+ @patch('forgeimporters.github.requests')
@patch('forgeimporters.base.import_tool')
- def test_create(self, import_tool):
+ def test_create(self, import_tool, requests):
+ requests.head.return_value.status_code = 200
params = dict(
gh_user_name='spooky',
gh_project_name='poop',
@@ -84,11 +86,13 @@ class TestGitHubImportController(TestController, TestCase):
mount_point='mymount',
)
r = self.app.post(
- '/p/{}/admin/ext/import/github-repo/create'.format(test_project_with_repo),
+ '/p/{}/admin/ext/import/github-repo/create'.format(
+ test_project_with_repo),
params,
status=302)
self.assertEqual(
- r.location, 'http://localhost/p/{}/admin/'.format(test_project_with_repo))
+ r.location, 'http://localhost/p/{}/admin/'.format(
+ test_project_with_repo))
self.assertEqual(
u'mymount', import_tool.post.call_args[1]['mount_point'])
self.assertEqual(
@@ -96,10 +100,13 @@ class TestGitHubImportController(TestController, TestCase):
self.assertEqual(
u'poop', import_tool.post.call_args[1]['project_name'])
self.assertEqual(u'spooky', import_tool.post.call_args[1]['user_name'])
+ self.assertEqual(requests.head.call_count, 1)
@with_git
+ @patch('forgeimporters.github.requests')
@patch('forgeimporters.base.import_tool')
- def test_create_limit(self, import_tool):
+ def test_create_limit(self, import_tool, requests):
+ requests.head.return_value.status_code = 200
project = M.Project.query.get(shortname=test_project_with_repo)
project.set_tool_data('GitHubRepoImporter', pending=1)
ThreadLocalORMSession.flush_all()
@@ -110,7 +117,8 @@ class TestGitHubImportController(TestController, TestCase):
mount_point='mymount',
)
r = self.app.post(
- '/p/{}/admin/ext/import/github-repo/create'.format(test_project_with_repo),
+ '/p/{}/admin/ext/import/github-repo/create'.format(
+ test_project_with_repo),
params,
status=302).follow()
self.assertIn('Please wait and try again', r)
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/ca119b44/ForgeImporters/forgeimporters/github/tests/test_tracker.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/github/tests/test_tracker.py b/ForgeImporters/forgeimporters/github/tests/test_tracker.py
index 72e1148..ba09ece 100644
--- a/ForgeImporters/forgeimporters/github/tests/test_tracker.py
+++ b/ForgeImporters/forgeimporters/github/tests/test_tracker.py
@@ -45,8 +45,10 @@ class TestGitHubTrackerImportController(TestController, TestCase):
self.assertIsNotNone(r.html.find(attrs=dict(name='mount_point')))
@with_tracker
+ @patch('forgeimporters.github.requests')
@patch('forgeimporters.base.import_tool')
- def test_create(self, import_tool):
+ def test_create(self, import_tool, requests):
+ requests.head.return_value.status_code = 200
params = dict(
gh_user_name='spooky',
gh_project_name='mulder',
@@ -62,10 +64,13 @@ class TestGitHubTrackerImportController(TestController, TestCase):
self.assertEqual(
u'mulder', import_tool.post.call_args[1]['project_name'])
self.assertEqual(u'spooky', import_tool.post.call_args[1]['user_name'])
+ self.assertEqual(requests.head.call_count, 1)
@with_tracker
+ @patch('forgeimporters.github.requests')
@patch('forgeimporters.base.import_tool')
- def test_create_limit(self, import_tool):
+ def test_create_limit(self, import_tool, requests):
+ requests.head.return_value.status_code = 200
p = M.Project.query.get(shortname=test_project_with_tracker)
p.set_tool_data('GitHubTrackerImporter', pending=1)
ThreadLocalORMSession.flush_all()
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/ca119b44/ForgeImporters/forgeimporters/github/tests/test_wiki.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/github/tests/test_wiki.py b/ForgeImporters/forgeimporters/github/tests/test_wiki.py
index f231f49..50e7278 100644
--- a/ForgeImporters/forgeimporters/github/tests/test_wiki.py
+++ b/ForgeImporters/forgeimporters/github/tests/test_wiki.py
@@ -538,8 +538,10 @@ class TestGitHubWikiImportController(TestController, TestCase):
r.html.find(attrs=dict(name='tool_option', value='import_history')))
@with_wiki
+ @patch('forgeimporters.github.requests')
@patch('forgeimporters.base.import_tool')
- def test_create(self, import_tool):
+ def test_create(self, import_tool, requests):
+ requests.head.return_value.status_code = 200
params = dict(
gh_user_name='spooky',
gh_project_name='mulder',
@@ -555,8 +557,10 @@ class TestGitHubWikiImportController(TestController, TestCase):
self.assertEqual(u'mulder', args['project_name'])
self.assertEqual(u'spooky', args['user_name'])
self.assertEqual(u'import_history', args['tool_option'])
+ self.assertEqual(requests.head.call_count, 1)
# without history
+ requests.head.reset_mock()
params.pop('tool_option')
r = self.app.post(self.url + 'create', params, status=302)
self.assertEqual(r.location, 'http://localhost/p/%s/admin/' %
@@ -567,10 +571,13 @@ class TestGitHubWikiImportController(TestController, TestCase):
self.assertEqual(u'mulder', args['project_name'])
self.assertEqual(u'spooky', args['user_name'])
self.assertEqual(u'', args['tool_option'])
+ self.assertEqual(requests.head.call_count, 1)
@with_wiki
+ @patch('forgeimporters.github.requests')
@patch('forgeimporters.base.import_tool')
- def test_create_limit(self, import_tool):
+ def test_create_limit(self, import_tool, requests):
+ requests.head.return_value.status_code = 200
p = M.Project.query.get(shortname=test_project_with_wiki)
p.set_tool_data('GitHubWikiImporter', pending=1)
ThreadLocalORMSession.flush_all()
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/ca119b44/ForgeImporters/forgeimporters/github/tracker.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/github/tracker.py b/ForgeImporters/forgeimporters/github/tracker.py
index 3af1517..fff88f6 100644
--- a/ForgeImporters/forgeimporters/github/tracker.py
+++ b/ForgeImporters/forgeimporters/github/tracker.py
@@ -46,11 +46,17 @@ from ming.orm import session, ThreadLocalORMSession
from pylons import tmpl_context as c
from pylons import app_globals as g
-from . import GitHubProjectExtractor, GitHubOAuthMixin
-from ..base import ToolImporter
from forgetracker.tracker_main import ForgeTrackerApp
from forgetracker import model as TM
-from forgeimporters.base import ToolImportForm
+from forgeimporters.base import (
+ ToolImportForm,
+ ToolImporter,
+)
+from forgeimporters.github import (
+ GitHubProjectExtractor,
+ GitHubOAuthMixin,
+ GitHubProjectNameValidator,
+)
from forgeimporters.github.utils import GitHubMarkdownConverter
@@ -58,7 +64,7 @@ log = logging.getLogger(__name__)
class GitHubTrackerImportForm(ToolImportForm):
- gh_project_name = fev.UnicodeString(not_empty=True)
+ gh_project_name = GitHubProjectNameValidator()
gh_user_name = fev.UnicodeString(not_empty=True)
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/ca119b44/ForgeImporters/forgeimporters/github/wiki.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/github/wiki.py b/ForgeImporters/forgeimporters/github/wiki.py
index 0bd95da..99333f7 100644
--- a/ForgeImporters/forgeimporters/github/wiki.py
+++ b/ForgeImporters/forgeimporters/github/wiki.py
@@ -52,7 +52,11 @@ from forgeimporters.base import (
ToolImporter,
ToolImportForm,
)
-from forgeimporters.github import GitHubProjectExtractor, GitHubOAuthMixin
+from forgeimporters.github import (
+ GitHubProjectExtractor,
+ GitHubOAuthMixin,
+ GitHubProjectNameValidator,
+)
from forgeimporters.github.utils import GitHubMarkdownConverter
from forgewiki import model as WM
from forgewiki.converters import mediawiki2markdown
@@ -71,7 +75,7 @@ except ImportError:
class GitHubWikiImportForm(ToolImportForm):
- gh_project_name = fev.UnicodeString(not_empty=True)
+ gh_project_name = GitHubProjectNameValidator()
gh_user_name = fev.UnicodeString(not_empty=True)
tool_option = fev.UnicodeString(if_missing=u'')
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/ca119b44/ForgeImporters/forgeimporters/google/__init__.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/google/__init__.py b/ForgeImporters/forgeimporters/google/__init__.py
index 51560f9..aebc596 100644
--- a/ForgeImporters/forgeimporters/google/__init__.py
+++ b/ForgeImporters/forgeimporters/google/__init__.py
@@ -118,7 +118,9 @@ def csv_parser(page):
class GoogleCodeProjectNameValidator(fev.FancyValidator):
not_empty = True
messages = {
- 'invalid': 'Please enter a project URL, or a project name containing only letters, numbers, and dashes.',
+ 'invalid': 'Please enter a project URL, or a project name containing '
+ 'only letters, numbers, and dashes.',
+ 'unavailable': 'This project is unavailable for import',
}
def _to_python(self, value, state=None):
@@ -128,10 +130,10 @@ class GoogleCodeProjectNameValidator(fev.FancyValidator):
else:
project_name = os.path.basename(url.path.strip('/'))
if not re.match(r'^[a-z0-9][a-z0-9-]{,61}$', project_name):
- raise fev.Invalid(self.message('invalid'))
+ raise fev.Invalid(self.message('invalid', state), value, state)
if not GoogleCodeProjectExtractor(project_name).check_readable():
- raise fev.Invalid('The project "%s" is not avalible for import' % value, value, state)
+ raise fev.Invalid(self.message('unavailable', state), value, state)
return project_name
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/ca119b44/ForgeImporters/forgeimporters/tests/github/test_extractor.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/tests/github/test_extractor.py b/ForgeImporters/forgeimporters/tests/github/test_extractor.py
index a8ae0a5..8b81f88 100644
--- a/ForgeImporters/forgeimporters/tests/github/test_extractor.py
+++ b/ForgeImporters/forgeimporters/tests/github/test_extractor.py
@@ -213,3 +213,10 @@ class TestGitHubProjectExtractor(TestCase):
'Rate limit exceeded (10 requests/hour). '
'Sleeping until 2013-10-25 09:32:02 UTC'
)
+
+ @patch.object(github.requests, 'head')
+ def test_check_readable(self, head):
+ head.return_value.status_code = 200
+ assert github.GitHubProjectExtractor('my-project').check_readable()
+ head.return_value.status_code = 404
+ assert not github.GitHubProjectExtractor('my-project').check_readable()