diff --git a/PRESUBMIT.py b/PRESUBMIT.py index df891a6c3e..c8ac1656a5 100755 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -381,8 +381,8 @@ def CheckNoPackageBoundaryViolations(input_api, gn_files, output_api): cwd = input_api.PresubmitLocalPath() script_path = os.path.join('tools_webrtc', 'presubmit_checks_lib', 'check_package_boundaries.py') - command = [sys.executable, script_path] - command += [gn_file.LocalPath() for gn_file in gn_files] + command = [sys.executable, script_path, cwd] + command += [os.path.join(cwd, gn_file.LocalPath()) for gn_file in gn_files] returncode, _, stderr = _RunCommand(command, cwd) if returncode: return [output_api.PresubmitError( @@ -392,7 +392,8 @@ def CheckNoPackageBoundaryViolations(input_api, gn_files, output_api): def CheckGnChanges(input_api, output_api): source_file_filter = lambda x: input_api.FilterSourceFile( - x, white_list=(r'.+\.(gn|gni)$',)) + x, white_list=(r'.+\.(gn|gni)$',), + black_list=r'.*/presubmit_checks_lib/testdata/.*') gn_files = [] for f in input_api.AffectedSourceFiles(source_file_filter): diff --git a/tools_webrtc/presubmit_checks_lib/check_package_boundaries.py b/tools_webrtc/presubmit_checks_lib/check_package_boundaries.py index ec1521b56a..aabcd813e4 100644 --- a/tools_webrtc/presubmit_checks_lib/check_package_boundaries.py +++ b/tools_webrtc/presubmit_checks_lib/check_package_boundaries.py @@ -57,7 +57,7 @@ def _BuildSubpackagesPattern(packages, query): query += os.path.sep length = len(query) pattern = r'(?P\d+)\$\s*"(?P(?P' - pattern += '|'.join(package[length:].replace(os.path.sep, '/') + pattern += '|'.join(re.escape(package[length:].replace(os.path.sep, '/')) for package in packages if package.startswith(query)) pattern += r')/[\w\./]*)"' return re.compile(pattern) @@ -100,10 +100,13 @@ def _CheckBuildFile(build_file_path, packages, logger): def CheckPackageBoundaries(root_dir, logger, build_files=None): packages = [root for root, _, files in os.walk(root_dir) if 'BUILD.gn' in files] - default_build_files = [os.path.join(package, 'BUILD.gn') - for package in packages] - build_files = build_files or default_build_files + if build_files is not None: + for build_file_path in build_files: + assert build_file_path.startswith(root_dir) + else: + build_files = [os.path.join(package, 'BUILD.gn') for package in packages] + return any([_CheckBuildFile(build_file_path, packages, logger) for build_file_path in build_files]) diff --git a/tools_webrtc/presubmit_checks_lib/check_package_boundaries_test.py b/tools_webrtc/presubmit_checks_lib/check_package_boundaries_test.py index 1487a230e8..1acdb10891 100755 --- a/tools_webrtc/presubmit_checks_lib/check_package_boundaries_test.py +++ b/tools_webrtc/presubmit_checks_lib/check_package_boundaries_test.py @@ -63,6 +63,17 @@ class UnitTest(unittest.TestCase): def testAllBuildFiles(self): self.RunTest(os.path.join(TESTDATA_DIR, 'all_build_files'), True) + def testSanitizeFilename(self): + # The `dangerous_filename` test case contains a directory with '++' in its + # name. If it's not properly escaped, a regex error would be raised. + self.RunTest(os.path.join(TESTDATA_DIR, 'dangerous_filename'), True) + + def testRelativeFilename(self): + test_dir = os.path.join(TESTDATA_DIR, 'all_build_files') + logger = Logger(test_dir) + with self.assertRaises(AssertionError): + CheckPackageBoundaries(test_dir, logger, ["BUILD.gn"]) + if __name__ == '__main__': unittest.main() diff --git a/tools_webrtc/presubmit_checks_lib/testdata/dangerous_filename/BUILD.gn b/tools_webrtc/presubmit_checks_lib/testdata/dangerous_filename/BUILD.gn new file mode 100644 index 0000000000..e15d9721f5 --- /dev/null +++ b/tools_webrtc/presubmit_checks_lib/testdata/dangerous_filename/BUILD.gn @@ -0,0 +1,15 @@ +# Copyright (c) 2017 The WebRTC project authors. All Rights Reserved. +# +# Use of this source code is governed by a BSD-style license +# that can be found in the LICENSE file in the root of the source +# tree. An additional intellectual property rights grant can be found +# in the file PATENTS. All contributing project authors may +# be found in the AUTHORS file in the root of the source tree. + +# "libc++" is considered a "dangerous filename" because it's an invalid regex. + +target("dummy_target") { + sources = [ + "libc++/dummy_subpackage_file.h", + ] +} diff --git a/tools_webrtc/presubmit_checks_lib/testdata/dangerous_filename/expected.pyl b/tools_webrtc/presubmit_checks_lib/testdata/dangerous_filename/expected.pyl new file mode 100644 index 0000000000..5447ab041d --- /dev/null +++ b/tools_webrtc/presubmit_checks_lib/testdata/dangerous_filename/expected.pyl @@ -0,0 +1,5 @@ +[["BUILD.gn", + "13", + "dummy_target", + "libc++/dummy_subpackage_file.h", + "libc++"]] diff --git a/tools_webrtc/presubmit_checks_lib/testdata/dangerous_filename/libc++/BUILD.gn b/tools_webrtc/presubmit_checks_lib/testdata/dangerous_filename/libc++/BUILD.gn new file mode 100644 index 0000000000..63b960492a --- /dev/null +++ b/tools_webrtc/presubmit_checks_lib/testdata/dangerous_filename/libc++/BUILD.gn @@ -0,0 +1,13 @@ +# Copyright (c) 2017 The WebRTC project authors. All Rights Reserved. +# +# Use of this source code is governed by a BSD-style license +# that can be found in the LICENSE file in the root of the source +# tree. An additional intellectual property rights grant can be found +# in the file PATENTS. All contributing project authors may +# be found in the AUTHORS file in the root of the source tree. + +group("dummy_subpackage") { + sources = [ + "dummy_subpackage.h", + ] +}