Skip to content

C
Chromium_Depot_tools
Commit eb5edbcf authored by maruel@chromium.org's avatar maruel@chromium.org
Browse files
Options

Add UpgradeToHttps() to reliably and forcibly upgrade all urls to https. 

Enable it for git-cl and gcl.

R=nsylvain@chromium.org
BUG=107838
TEST=New connections go through https://


Review URL: http://codereview.chromium.org/9214004

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@117857 0039d316-1c4b-4281-b951-d872f2087c98
parent 99ac1c58
Hide whitespace changes
Inline Side-by-side
Showing with 120 additions and 68 deletions
gcl.py
View file @ eb5edbcf
......@@ -294,10 +294,8 @@ class ChangeInfo(object):
self.patch = None
self._local_root = local_root
self.needs_upload = needs_upload
self.rietveld = rietveld_url
if not self.rietveld:
# Set the default value.
self.rietveld = GetCodeReviewSetting('CODE_REVIEW_SERVER')
self.rietveld = gclient_utils.UpgradeToHttps(
rietveld_url or GetCodeReviewSetting('CODE_REVIEW_SERVER'))
self._rpc_server = None
def _get_description(self):
......@@ -1039,7 +1037,7 @@ def CMDcommit(change_info, args):
if change_info.issue:
revision = re.compile(".*?\nCommitted revision (\d+)",
re.DOTALL).match(output).group(1)
viewvc_url = GetCodeReviewSetting("VIEW_VC")
viewvc_url = gclient_utils.UpgradeToHttps(GetCodeReviewSetting('VIEW_VC'))
change_info.description += '\n'
if viewvc_url:
change_info.description += "\nCommitted: " + viewvc_url + revision
......
gclient_utils.py
View file @ eb5edbcf
......@@ -14,6 +14,7 @@ import sys
import tempfile
import threading
import time
import urlparse
import subprocess2
......@@ -734,6 +735,30 @@ def RunEditor(content, git):
os.remove(filename)
def UpgradeToHttps(url):
"""Upgrades random urls to https://.
Do not touch unknown urls like ssh:// or git://.
Do not touch http:// urls with a port number,
Fixes invalid GAE url.
"""
if not url:
return url
if not re.match(r'[a-z\-]+\://.*', url):
# Make sure it is a valid uri. Otherwise, urlparse() will consider it a
# relative url and will use http:///foo. Note that it defaults to http://
# for compatibility with naked url like "localhost:8080".
url = 'http://%s' % url
parsed = list(urlparse.urlparse(url))
# Do not automatically upgrade http to https if a port number is provided.
if parsed[0] == 'http' and not re.match(r'^.+?\:\d+$', parsed[1]):
parsed[0] = 'https'
# Until GAE supports SNI, manually convert the url.
if parsed[1] == 'codereview.chromium.org':
parsed[1] = 'chromiumcodereview.appspot.com'
return urlparse.urlunparse(parsed)
def ParseCodereviewSettingsContent(content):
"""Process a codereview.settings file properly."""
lines = (l for l in content.splitlines() if not l.strip().startswith("#"))
......@@ -742,5 +767,9 @@ def ParseCodereviewSettingsContent(content):
except ValueError:
raise Error(
'Failed to process settings, please fix. Content:\n\n%s' % content)
# TODO(maruel): Post-process
def fix_url(key):
if keyvals.get(key):
keyvals[key] = UpgradeToHttps(keyvals[key])
fix_url('CODE_REVIEW_SERVER')
fix_url('VIEW_VC')
return keyvals
git_cl.py
View file @ eb5edbcf
......@@ -43,7 +43,7 @@ import subprocess2
import watchlists
DEFAULT_SERVER = 'http://codereview.appspot.com'
DEFAULT_SERVER = 'https://codereview.appspot.com'
POSTUPSTREAM_HOOK_PATTERN = '.git/hooks/post-cl-%s'
DESCRIPTION_BACKUP_FILE = '~/.git_cl_description_backup'
......@@ -94,15 +94,6 @@ def ask_for_data(prompt):
sys.exit(1)
def FixUrl(server):
"""Fix a server url to defaults protocol to http:// if none is specified."""
if not server:
return server
if not re.match(r'[a-z]+\://.*', server):
return 'http://' + server
return server
def MatchSvnGlob(url, base_url, glob_spec, allow_wildcards):
"""Return the corresponding git ref if |base_url| together with |glob_spec|
matches the full |url|.
......@@ -168,15 +159,15 @@ class Settings(object):
def GetDefaultServerUrl(self, error_ok=False):
if not self.default_server:
self.LazyUpdateIfNeeded()
self.default_server = FixUrl(self._GetConfig('rietveld.server',
error_ok=True))
self.default_server = gclient_utils.UpgradeToHttps(
self._GetConfig('rietveld.server', error_ok=True))
if error_ok:
return self.default_server
if not self.default_server:
error_message = ('Could not find settings file. You must configure '
'your review setup by running "git cl config".')
self.default_server = FixUrl(self._GetConfig(
'rietveld.server', error_message=error_message))
self.default_server = gclient_utils.UpgradeToHttps(
self._GetConfig('rietveld.server', error_message=error_message))
return self.default_server
def GetRoot(self):
......@@ -266,7 +257,8 @@ class Settings(object):
def GetViewVCUrl(self):
if not self.viewvc_url:
self.viewvc_url = self._GetConfig('rietveld.viewvc-url', error_ok=True)
self.viewvc_url = gclient_utils.UpgradeToHttps(
self._GetConfig('rietveld.viewvc-url', error_ok=True))
return self.viewvc_url
def GetDefaultCCList(self):
......@@ -426,7 +418,7 @@ or verify this branch is set up to track another (via the --track argument to
issue = RunGit(['config', self._IssueSetting()], error_ok=True).strip()
if issue:
self.issue = issue
self.rietveld_server = FixUrl(RunGit(
self.rietveld_server = gclient_utils.UpgradeToHttps(RunGit(
['config', self._RietveldServer()], error_ok=True).strip())
else:
self.issue = None
......@@ -625,23 +617,28 @@ def GetCodereviewSettingsInteractively():
newserver = ask_for_data(prompt + ':')
if not server and not newserver:
newserver = DEFAULT_SERVER
if newserver and newserver != server:
RunGit(['config', 'rietveld.server', newserver])
if newserver:
newserver = gclient_utils.UpgradeToHttps(newserver)
if newserver != server:
RunGit(['config', 'rietveld.server', newserver])
def SetProperty(initial, caption, name):
def SetProperty(initial, caption, name, is_url):
prompt = caption
if initial:
prompt += ' ("x" to clear) [%s]' % initial
new_val = ask_for_data(prompt + ':')
if new_val == 'x':
RunGit(['config', '--unset-all', 'rietveld.' + name], error_ok=True)
elif new_val and new_val != initial:
RunGit(['config', 'rietveld.' + name, new_val])
elif new_val:
if is_url:
new_val = gclient_utils.UpgradeToHttps(new_val)
if new_val != initial:
RunGit(['config', 'rietveld.' + name, new_val])
SetProperty(settings.GetDefaultCCList(), 'CC list', 'cc')
SetProperty(settings.GetDefaultCCList(), 'CC list', 'cc', False)
SetProperty(settings.GetTreeStatusUrl(error_ok=True), 'Tree status URL',
'tree-status-url')
SetProperty(settings.GetViewVCUrl(), 'ViewVC URL', 'viewvc-url')
'tree-status-url', False)
SetProperty(settings.GetViewVCUrl(), 'ViewVC URL', 'viewvc-url', True)
# TODO: configure a default branch to diff against, rather than this
# svn-based hackery.
......@@ -1238,8 +1235,8 @@ def CMDpatch(parser, args):
issue = issue_arg
patch_data = Changelist().GetPatchSetDiff(issue)
else:
# Assume it's a URL to the patch. Default to http.
issue_url = FixUrl(issue_arg)
# Assume it's a URL to the patch. Default to https.
issue_url = gclient_utils.UpgradeToHttps(issue_arg)
match = re.match(r'.*?/issue(\d+)_\d+.diff', issue_url)
if not match:
DieWithError('Must pass an issue ID or full URL for '
......
tests/gcl_unittest.py
View file @ eb5edbcf
#!/usr/bin/env python
# Copyright (c) 2011 The Chromium Authors. All rights reserved.
# Copyright (c) 2012 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
......@@ -52,7 +52,7 @@ class GclTestsBase(SuperMoxTestBase):
change_info.GetFileNames = lambda : [f[1] for f in change_info.files]
change_info.GetLocalRoot = lambda : 'proout'
change_info.patch = None
change_info.rietveld = 'my_server'
change_info.rietveld = 'https://my_server'
change_info.reviewers = None
change_info._closed = False
change_info._deleted = False
......@@ -259,7 +259,7 @@ class ChangeInfoUnittest(GclTestsBase):
gcl.GetChangelistInfoFile('').AndReturn('foo')
values = {
'description': '', 'patchset': 2, 'issue': 1,
'files': [], 'needs_upload': False, 'rietveld': 'foo'}
'files': [], 'needs_upload': False, 'rietveld': 'https://foo'}
gcl.gclient_utils.FileWrite(
'foo', gcl.json.dumps(values, sort_keys=True, indent=2))
self.mox.ReplayAll()
......@@ -272,7 +272,7 @@ class ChangeInfoUnittest(GclTestsBase):
gcl.GetChangelistInfoFile('n').AndReturn('foo')
values = {
'description': 'des', 'patchset': 0, 'issue': 0,
'files': [], 'needs_upload': True, 'rietveld': 'foo'}
'files': [], 'needs_upload': True, 'rietveld': 'https://foo'}
gcl.gclient_utils.FileWrite(
'foo', gcl.json.dumps(values, sort_keys=True, indent=2))
self.mox.ReplayAll()
......@@ -302,7 +302,7 @@ class CMDuploadUnittest(GclTestsBase):
change_info.description = 'deescription\n\nR=foo@bar.com',
change_info.files = [('A', 'aa'), ('M', 'bb')]
change_info.patch = None
change_info.rietveld = 'my_server'
change_info.rietveld = 'https://my_server'
files = [item[1] for item in change_info.files]
output = presubmit_support.PresubmitOutput()
gcl.DoPresubmitChecks(change_info, False, True).AndReturn(output)
......@@ -312,7 +312,7 @@ class CMDuploadUnittest(GclTestsBase):
gcl.os.chdir('proout')
change_info.GetFileNames().AndReturn(files)
gcl.GenerateDiff(files)
gcl.upload.RealMain(['upload.py', '-y', '--server=my_server',
gcl.upload.RealMain(['upload.py', '-y', '--server=https://my_server',
'-r', 'georges@example.com',
'--message=\'\'', '--issue=1'],
change_info.patch).AndReturn(("1",
......@@ -356,9 +356,10 @@ class CMDuploadUnittest(GclTestsBase):
gcl.os.getcwd().AndReturn('somewhere')
gcl.os.chdir(change_info.GetLocalRoot())
gcl.GenerateDiff(change_info.GetFileNames())
gcl.upload.RealMain(['upload.py', '-y', '--server=my_server', '--server=a',
"--description_file=descfile",
"--message=deescription"], change_info.patch).AndReturn(("1", "2"))
gcl.upload.RealMain(
[ 'upload.py', '-y', '--server=https://my_server', '--server=a',
'--description_file=descfile', '--message=deescription'],
change_info.patch).AndReturn(("1", "2"))
gcl.os.remove('descfile')
change_info.SendToRietveld("/lint/issue%s_%s" % ('1', '2'), timeout=1)
gcl.os.chdir('somewhere')
......@@ -397,7 +398,7 @@ class CMDuploadUnittest(GclTestsBase):
gcl.os.getcwd().AndReturn('somewhere')
gcl.os.chdir(change_info.GetLocalRoot())
gcl.GenerateDiff(change_info.GetFileNames())
gcl.upload.RealMain(['upload.py', '-y', '--server=my_server',
gcl.upload.RealMain(['upload.py', '-y', '--server=https://my_server',
"--description_file=descfile",
"--message=deescription"], change_info.patch).AndReturn(("1", "2"))
gcl.os.remove('descfile')
......@@ -442,7 +443,7 @@ class CMDuploadUnittest(GclTestsBase):
change_info.description = 'deescription\n\nR=georges@example.com',
change_info.files = [('A', 'aa'), ('M', 'bb')]
change_info.patch = None
change_info.rietveld = 'my_server'
change_info.rietveld = 'https://my_server'
change_info.reviewers = ['georges@example.com']
files = [item[1] for item in change_info.files]
output = presubmit_support.PresubmitOutput()
......@@ -454,7 +455,7 @@ class CMDuploadUnittest(GclTestsBase):
change_info.GetLocalRoot().AndReturn('proout')
gcl.os.chdir('proout')
gcl.GenerateDiff(files)
gcl.upload.RealMain(['upload.py', '-y', '--server=my_server',
gcl.upload.RealMain(['upload.py', '-y', '--server=https://my_server',
'--reviewers=georges@example.com',
'--message=\'\'', '--issue=1'],
change_info.patch).AndReturn(("1", "2"))
......@@ -483,7 +484,7 @@ class CMDuploadUnittest(GclTestsBase):
gcl.os.getcwd().AndReturn('somewhere')
gcl.os.chdir('proout')
gcl.GenerateDiff(change_info.GetFileNames())
gcl.upload.RealMain(['upload.py', '-y', '--server=my_server',
gcl.upload.RealMain(['upload.py', '-y', '--server=https://my_server',
'--reviewers=foo@example.com,bar@example.com',
'--message=\'\'', '--issue=1'],
change_info.patch).AndReturn(("1", "2"))
......@@ -558,8 +559,8 @@ class CMDCommitUnittest(GclTestsBase):
def testPresubmitSucceeds(self):
change_info = self.mockLoad()
self.mockPresubmit(change_info, fail=False)
self.mockCommit(change_info, 'deescription\nReview URL: http://my_server/1',
'')
self.mockCommit(
change_info, 'deescription\nReview URL: https://my_server/1', '')
self.mox.ReplayAll()
retval = gcl.CMDcommit(['naame'])
......@@ -573,15 +574,16 @@ class CMDCommitUnittest(GclTestsBase):
def testPresubmitSucceedsWithCommittedMessage(self):
change_info = self.mockLoad()
self.mockPresubmit(change_info, fail=False)
self.mockCommit(change_info, 'deescription\nReview URL: http://my_server/1',
'\nCommitted revision 12345')
self.mockCommit(
change_info, 'deescription\nReview URL: https://my_server/1',
'\nCommitted revision 12345')
self.mox.ReplayAll()
retval = gcl.CMDcommit(['naame'])
self.assertEquals(retval, 0)
self.assertEquals(change_info.description,
'deescription\n\nCommitted: http://view/12345')
'deescription\n\nCommitted: https://view/12345')
# pylint: disable=W0212
self.assertTrue(change_info._deleted)
self.assertTrue(change_info._closed)
......
tests/gclient_utils_test.py
View file @ eb5edbcf
......@@ -35,10 +35,11 @@ class GclientUtilsUnittest(GclientUtilBase):
'MakeDateRevision', 'MakeFileAutoFlush', 'MakeFileAnnotated',
'PathDifference', 'ParseCodereviewSettingsContent',
'PrintableObject', 'RemoveDirectory', 'RunEditor',
'SplitUrlRevision', 'SyntaxErrorToError', 'Wrapper', 'WorkItem',
'SplitUrlRevision', 'SyntaxErrorToError',
'UpgradeToHttps', 'Wrapper', 'WorkItem',
'errno', 'lockedmethod', 'logging', 'os', 'Queue', 're', 'rmtree',
'safe_makedirs', 'stat', 'subprocess2', 'sys', 'tempfile', 'threading',
'time',
'time', 'urlparse',
]
# If this test fails, you should add the relevant test.
self.compareMembers(gclient_utils, members)
......@@ -171,20 +172,45 @@ class GClientUtilsTest(trial_dir.TestCase):
os.chmod(l2, 0)
os.chmod(l1, 0)
def testUpgradeToHttps(self):
values = [
['', ''],
[None, None],
['foo', 'https://foo'],
['http://foo', 'https://foo'],
['foo/', 'https://foo/'],
['ssh-svn://foo', 'ssh-svn://foo'],
['ssh-svn://foo/bar/', 'ssh-svn://foo/bar/'],
['codereview.chromium.org', 'https://chromiumcodereview.appspot.com'],
['codereview.chromium.org/', 'https://chromiumcodereview.appspot.com/'],
['http://foo:8080', 'http://foo:8080'],
['http://foo:8080/bar', 'http://foo:8080/bar'],
['foo:8080', 'http://foo:8080'],
['foo:', 'https://foo:'],
]
for content, expected in values:
self.assertEquals(
expected, gclient_utils.UpgradeToHttps(content))
def testParseCodereviewSettingsContent(self):
expected = {
'Foo': 'bar:baz',
'Second': 'value',
}
content = (
'# bleh\n'
'\t# foo : bar\n'
'Foo:bar:baz\n'
' Second : value \n\r'
'#inconsistency'
)
self.assertEquals(
expected, gclient_utils.ParseCodereviewSettingsContent(content))
values = [
['# bleh\n', {}],
['\t# foo : bar\n', {}],
['Foo:bar', {'Foo': 'bar'}],
['Foo:bar:baz\n', {'Foo': 'bar:baz'}],
[' Foo : bar ', {'Foo': 'bar'}],
[' Foo : bar \n', {'Foo': 'bar'}],
['a:b\n\rc:d\re:f', {'a': 'b', 'c': 'd', 'e': 'f'}],
['an_url:http://value/', {'an_url': 'http://value/'}],
[
'CODE_REVIEW_SERVER : http://r/s',
{'CODE_REVIEW_SERVER': 'https://r/s'}
],
['VIEW_VC:http://r/s', {'VIEW_VC': 'https://r/s'}],
]
for content, expected in values:
self.assertEquals(
expected, gclient_utils.ParseCodereviewSettingsContent(content))
if __name__ == '__main__':
......
tests/git_cl_test.py
View file @ eb5edbcf
#!/usr/bin/env python
# Copyright (c) 2011 The Chromium Authors. All rights reserved.
# Copyright (c) 2012 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
......@@ -105,7 +105,7 @@ class TestGitCl(TestCase):
(['git', 'svn', 'info'], ''),
(['git', 'config', 'branch.master.rietveldissue', '1'], ''),
(['git', 'config', 'branch.master.rietveldserver',
'http://codereview.example.com'], ''),
'https://codereview.example.com'], ''),
(['git', 'config', 'branch.master.rietveldpatchset', '2'], ''),
]
......@@ -115,7 +115,7 @@ class TestGitCl(TestCase):
msg = description.split('\n', 1)[0]
return [
'upload', '--assume_yes', '--server',
'http://codereview.example.com',
'https://codereview.example.com',
'--message', msg,
'--description', description
] + args + [
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment