From 44a17adf95036cdff503145589153ddf1df58d2f Mon Sep 17 00:00:00 2001 From: "maruel@chromium.org" Date: Mon, 8 Jun 2009 14:14:35 +0000 Subject: [PATCH] Add InputApi.ReadFile() and presubmit_canned_checks.CheckChangeHasNoCR(). ReadFile limits to reading files inside the repository. CheckChangeHasNoCR reads the text files looking for \r and fails if found. TEST=none BUG=none Review URL: http://codereview.chromium.org/118370 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@17857 0039d316-1c4b-4281-b951-d872f2087c98 --- presubmit_canned_checks.py | 12 +++++++ presubmit_support.py | 13 ++++++- tests/presubmit_unittest.py | 67 +++++++++++++++++++++++++++++++++---- 3 files changed, 85 insertions(+), 7 deletions(-) diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index 7897effd7..b8cbd94d3 100755 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -68,6 +68,18 @@ def CheckDoNotSubmit(input_api, output_api): ) +def CheckChangeHasNoCR(input_api, output_api): + """Checks that there are no \r, \r\n (CR or CRLF) characters in any of the + text files to be submitted. + """ + outputs = [] + for f in input_api.AffectedTextFiles(): + if '\r' in input_api.ReadFile(f, 'rb'): + outputs.append(output_api.PresubmitPromptWarning( + "Found a CR character in %s" % f.LocalPath())) + return outputs + + def CheckChangeHasNoTabs(input_api, output_api): """Checks that there are no tab characters in any of the text files to be submitted. diff --git a/presubmit_support.py b/presubmit_support.py index 86398cb8f..0dbe30abf 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -6,7 +6,7 @@ """Enables directory-specific presubmit checks to run at upload and/or commit. """ -__version__ = '1.3' +__version__ = '1.3.1' # TODO(joi) Add caching where appropriate/needed. The API is designed to allow # caching (between all different invocations of presubmit scripts for a given @@ -271,6 +271,17 @@ class InputApi(object): filter(lambda x: x.IsTextFile(), self.AffectedFiles(include_deletes=False))) + def ReadFile(self, file, mode='r'): + """Reads an arbitrary file. + + Deny reading anything outside the repository. + """ + if isinstance(file, AffectedFile): + file = file.AbsoluteLocalPath() + if not file.startswith(self.change.RepositoryRoot()): + raise IOError('Access outside the repository root is denied.') + return gcl.ReadFile(file, mode) + @staticmethod def _RightHandSideLinesImpl(affected_files): """Implements RightHandSideLines for InputApi and GclChange.""" diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 8135e5f03..cc5825f42 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -94,6 +94,12 @@ def CheckChangeOnUpload(input_api, output_api): if not x.startswith('_')] self.assertEqual(actual_members, sorted(members)) + def MakeBasicChange(self, name, description, root=None): + ci = presubmit.gcl.ChangeInfo(name, 0, 0, description, None) + if root is None: + root = self.fake_root_dir + return presubmit.GclChange(ci, root) + class PresubmitUnittest(PresubmitTestsBase): """General presubmit_support.py tests (excluding InputApi and OutputApi).""" @@ -519,7 +525,7 @@ class InputApiUnittest(PresubmitTestsBase): members = [ 'AbsoluteLocalPaths', 'AffectedFiles', 'AffectedTextFiles', 'DepotToLocalPath', 'LocalPaths', 'LocalToDepotPath', - 'PresubmitLocalPath', 'RightHandSideLines', 'ServerPaths', + 'PresubmitLocalPath', 'RightHandSideLines', 'ReadFile', 'ServerPaths', 'basename', 'cPickle', 'cStringIO', 'canned_checks', 'change', 'is_committing', 'marshal', 'os_path', 'pickle', 'platform', 're', 'subprocess', 'tempfile', 'traceback', 'unittest', 'urllib2', @@ -672,6 +678,34 @@ class InputApiUnittest(PresubmitTestsBase): api = presubmit.InputApi(change, 'foo/PRESUBMIT.py', True) api.AffectedTextFiles(include_deletes=False) + def testReadFileStringDenied(self): + self.mox.ReplayAll() + input_api = presubmit.InputApi(None, './p', False) + input_api.change = self.MakeBasicChange('foo', 'Foo\n', '/AA') + self.assertRaises(IOError, input_api.ReadFile, 'boo', 'x') + + def testReadFileStringAccepted(self): + presubmit.gcl.ReadFile('/AA/boo', 'x').AndReturn(None) + self.mox.ReplayAll() + input_api = presubmit.InputApi(None, './p', False) + input_api.change = self.MakeBasicChange('foo', 'Foo\n', '/AA') + input_api.ReadFile('/AA/boo', 'x') + + def testReadFileAffectedFileDenied(self): + file = presubmit.AffectedFile('boo', 'M') + self.mox.ReplayAll() + input_api = presubmit.InputApi(None, './p', False) + input_api.change = self.MakeBasicChange('foo', 'Foo\n', '/AA') + self.assertRaises(IOError, input_api.ReadFile, 'boo', 'x') + + def testReadFileAffectedFileAccepted(self): + file = presubmit.AffectedFile('/AA/boo', 'M') + presubmit.gcl.ReadFile('/AA/boo', 'x').AndReturn(None) + self.mox.ReplayAll() + input_api = presubmit.InputApi(None, './p', False) + input_api.change = self.MakeBasicChange('foo', 'Foo\n', '/AA') + input_api.ReadFile('/AA/boo', 'x') + class OuputApiUnittest(PresubmitTestsBase): """Tests presubmit.OutputApi.""" @@ -836,14 +870,10 @@ class CannedChecksUnittest(PresubmitTestsBase): input_api.unittest = unittest return input_api - def MakeBasicChange(self, name, description): - ci = presubmit.gcl.ChangeInfo(name, 0, 0, description, None) - return presubmit.GclChange(ci, self.fake_root_dir) - def testMembersChanged(self): self.mox.ReplayAll() members = [ - 'CheckChangeHasBugField', 'CheckChangeHasNoTabs', + 'CheckChangeHasBugField', 'CheckChangeHasNoCR', 'CheckChangeHasNoTabs', 'CheckChangeHasQaField', 'CheckChangeHasTestedField', 'CheckChangeHasTestField', 'CheckDoNotSubmit', 'CheckDoNotSubmitInDescription', 'CheckDoNotSubmitInFiles', @@ -922,6 +952,31 @@ class CannedChecksUnittest(PresubmitTestsBase): 'DO NOTSUBMIT', 'DO NOT ' + 'SUBMIT', presubmit.OutputApi.PresubmitError) + def testCheckChangeHasNoCR(self): + input_api1 = self.MockInputApi() + self.mox.StubOutWithMock(input_api1, 'ReadFile') + input_api1.change = self.MakeBasicChange('foo', 'Foo\n') + affected_file1 = self.mox.CreateMock(presubmit.SvnAffectedFile) + input_api1.AffectedTextFiles().AndReturn([affected_file1]) + input_api1.ReadFile(affected_file1, 'rb').AndReturn("Hey!\nHo!\n") + input_api2 = self.MockInputApi() + self.mox.StubOutWithMock(input_api2, 'ReadFile') + input_api2.change = self.MakeBasicChange('foo', 'Foo\n') + affected_file2 = self.mox.CreateMock(presubmit.SvnAffectedFile) + input_api2.AffectedTextFiles().AndReturn([affected_file2]) + input_api2.ReadFile(affected_file2, 'rb').AndReturn("Hey!\r\nHo!\r\n") + affected_file2.LocalPath().AndReturn('bar.cc') + self.mox.ReplayAll() + + results = presubmit_canned_checks.CheckChangeHasNoCR( + input_api1, presubmit.OutputApi) + self.assertEquals(results, []) + results2 = presubmit_canned_checks.CheckChangeHasNoCR( + input_api2, presubmit.OutputApi) + self.assertEquals(len(results2), 1) + self.assertEquals(results2[0].__class__, + presubmit.OutputApi.PresubmitPromptWarning) + def testCannedCheckChangeHasNoTabs(self): self.TestContent(presubmit_canned_checks.CheckChangeHasNoTabs, 'blah blah', 'blah\tblah',