From ebab05c8b396e137af660235e866042d5462efe3 Mon Sep 17 00:00:00 2001 From: Glenn Wurster Date: Thu, 15 Sep 2022 14:43:20 -0400 Subject: [PATCH] Fix errors found during additional testing This patch fixes a number of errors encountered during internal testing: 1. Processing of hunks within a patch file was incorrect. We were starting a new hunk every time we saw a --- or +++, but we should have been looking at the number of lines remaining in the current hunk (which started with a @@) to determine if we were at the end of the hunk. In the rare case where the patch is removing a line that starts with --, or adding a line that starts with ++, you'll have a --- or +++ within the hunk which we were treating as a new hunk even though it should not have been. Now, instead of looking for a +++ or --- on every line, we count the number of lines which should be in the hunk and treat those lines as part of the hunk instead of starting a new hunk. As part of implementing this feature, we renamed the _lineschanged array to the more descriptive _oldStart, _oldLength, _newStart, and _newLength variables. 2. We were not handling hunks which created or removed files correctly. In this case, either the source or target file will appear as /dev/null and we should be able to handle this. 3. On occasion, srcml was hanging which meant that apply.py also hung. Implement a timeout on the external program calls so that we can detect hangs and error out. 4. We do not handle binary patches, but we should be able to detect when they appear and raise an appropriate error. 5. Update the Makefile to automatically install srcml if it's not already installed. The http://gehry.sdml.cs.kent.edu site is unfortunately not available over HTTPS (it's a self-signed certificate and a default Apache testing page) and so for now we are downloading over HTTP. We install it into the env path. 6. Only ask questions about the possible files to apply the patch to if we are running over a TTY. This avoids the questions being asked if the output of apply.py is being piped to a file or another program. Unit tests are added for #1, #2, and #4 above. --- Makefile | 7 +- scripts/patch_apply/apply.py | 22 +- .../check_file_exists_elsewhere.py | 11 +- scripts/patch_apply/patchParser.py | 212 ++++++++++++------ scripts/patch_context/context_changes.py | 20 +- scripts/patch_context/slice_and_parse.py | 64 ++++-- scripts/patch_match/test_match.py | 2 +- tests/patches/applied/add-pluses.patch | 12 + tests/patches/applied/remove-negatives.patch | 13 ++ tests/patches/clean/add-file.patch | 65 ++++++ tests/patches/clean/add-pluses.patch | 13 ++ tests/patches/clean/one-line-after-hunk.patch | 51 +++++ tests/patches/clean/one-line-hunk.patch | 6 + tests/patches/clean/remove-file.patch | 65 ++++++ tests/patches/clean/remove-negatives.patch | 12 + tests/patches/git/binary-2.patch | 4 + tests/patches/git/binary.patch | 7 + tests/patches/negatives.c | 9 + tests/patches/one-line.cpp | 1 + tests/patches/pluses.c | 9 + tests/test_apply.py | 24 +- tests/test_basic.py | 30 ++- tests/test_changes.py | 2 +- tests/test_context.py | 4 +- tests/test_git.py | 8 + 25 files changed, 558 insertions(+), 115 deletions(-) create mode 100644 tests/patches/applied/add-pluses.patch create mode 100644 tests/patches/applied/remove-negatives.patch create mode 100644 tests/patches/clean/add-file.patch create mode 100644 tests/patches/clean/add-pluses.patch create mode 100644 tests/patches/clean/one-line-after-hunk.patch create mode 100644 tests/patches/clean/one-line-hunk.patch create mode 100644 tests/patches/clean/remove-file.patch create mode 100644 tests/patches/clean/remove-negatives.patch create mode 100644 tests/patches/git/binary-2.patch create mode 100644 tests/patches/git/binary.patch create mode 100644 tests/patches/negatives.c create mode 100644 tests/patches/one-line.cpp create mode 100644 tests/patches/pluses.c diff --git a/Makefile b/Makefile index 82a5265..7880794 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -build: python-packages links +build: python-packages links env/bin/srcml .PHONY: links links: env/bin/apply.py @@ -6,12 +6,17 @@ links: env/bin/apply.py env/bin/apply.py: env ln -s $$(realpath scripts/patch_apply/apply.py) env/bin +env/bin/srcml: + if [ -z "$$(which srcml)" ]; then cd env; curl http://gehry.sdml.cs.kent.edu/lmcrs/v1.0.0/srcml_1.0.0-1_ubuntu20.04.tar.gz | tar -zxv ; else ln -s "$$(which srcml)" /env/bin/srcml; fi + if [ -f "env/bin/srcml" ]; then mv env/bin/srcml env/bin/srcml-binary; echo '#!/bin/bash\nLD_LIBRARY_PATH=$(CURDIR)/env/lib exec srcml-binary $$*' >env/bin/srcml; chmod +x env/bin/srcml; fi; + .PHONY: python-packages python-packages: env @if [ ! -d env/lib/python*/site-packages/diff_match_patch ]; then echo "Installing diff_match_patch"; . env/bin/activate; python3 -m pip install diff_match_patch; fi @if [ ! -d env/lib/python*/site-packages/Levenshtein ]; then echo "Installing Levenshtein"; . env/bin/activate; python3 -m pip install Levenshtein; fi @if [ ! -d env/lib/python*/site-packages/pygments ]; then echo "Installing diff_match_patch"; . env/bin/activate; python3 -m pip install pygments; fi @if [ ! -x env/bin/pytest ]; then echo "Installing pytest"; . env/bin/activate; python3 -m pip install pytest; fi + @if [ ! -x env/bin/pytest-cov ]; then echo "Installing pytest-cov"; . env/bin/activate; python3 -m pip install pytest-cov; fi env: python3 -m venv env diff --git a/scripts/patch_apply/apply.py b/scripts/patch_apply/apply.py index 35dba29..09d1428 100755 --- a/scripts/patch_apply/apply.py +++ b/scripts/patch_apply/apply.py @@ -17,6 +17,10 @@ import scripts.patch_apply.check_file_exists_elsewhere as check_exist from scripts.enums import MatchStatus, natureOfChange, CONTEXT_DECISION, precheckStatus +def indent(text, amount, ch = ' '): + padding = amount * ch + return ''.join(padding + line for line in text.splitlines(True)) + def findGitPrefix(path): prefix='' resolved=False @@ -240,7 +244,7 @@ def apply(pathToPatch, **kwargs): error_message = patch_file.runResult if kwargs['verbose'] > 0: print("Patch failed while it was run with git apply with error:") - print(error_message) + print(indent(error_message, 4)) else: print("Patch failed to apply with git apply.") @@ -250,17 +254,21 @@ def apply(pathToPatch, **kwargs): does_not_apply = set() for line in error_message_lines: - print(line) split_line = [s.strip() for s in line.split(":")] if line[0:2] == " ": pass elif split_line[0] == "error": if split_line[1].startswith("corrupt patch"): line_num = re.findall(r'\d+', split_line[1]) - print("The patch is corrupted at line %s, stop processing..." % line_num[0]) + print("The patch is corrupted at line %s." % line_num[0]) return 1 - - if split_line[2] == "patch does not apply": + elif split_line[1].startswith("git diff header lacks filename information"): + print("The patch is corrupted at line %s." % line_num[0]) + return 1 + elif split_line[1].startswith("cannot apply binary patch"): + print("Binary patch detected.") + return 1 + elif split_line[2] == "patch does not apply": does_not_apply.add(split_line[1]) elif split_line[2] == "already exists": already_exists.add(split_line[1]) @@ -307,10 +315,10 @@ def apply(pathToPatch, **kwargs): gitFileName = os.path.join( findGitPrefix(fileName), fileName ) if see_patches: - print("\n" + ":".join([fileName, str(patch._lineschanged[0])])) + print("\n" + ":".join([fileName, str(patch._oldStart)])) print(patch) - subpatch_name = ":".join([fileName, str(patch._lineschanged[0])]) + subpatch_name = ":".join([fileName, str(patch._oldStart)]) if gitFileName in file_not_found: correct_loc = check_exist.checkFileExistsElsewhere(patch) diff --git a/scripts/patch_apply/check_file_exists_elsewhere.py b/scripts/patch_apply/check_file_exists_elsewhere.py index 11e5d91..cec6116 100644 --- a/scripts/patch_apply/check_file_exists_elsewhere.py +++ b/scripts/patch_apply/check_file_exists_elsewhere.py @@ -30,8 +30,8 @@ def checkFileExistsElsewhere(patch): if len(matched_file_locations) == 0: return None - else: - print("----------------------------------------------------------------------") + elif sys.stdout.isatty(): + print("-" * 70) print( "Here are the locations of files with the same filename as the following missing file: {}".format( toFind @@ -44,7 +44,7 @@ def checkFileExistsElsewhere(patch): "Select the file you would like to apply the patch to by entering the number next to it. Enter anything else to do nothing\n" ) - print("----------------------------------------------------------------------") + print("-" * 70) try: to_apply_file_index = int(to_apply_file_index) if 0 <= to_apply_file_index and to_apply_file_index < len( @@ -53,7 +53,10 @@ def checkFileExistsElsewhere(patch): return matched_file_locations[to_apply_file_index] except ValueError: return None - + else: + print(f"Possible files to apply the patch for {toFind} to:") + for i in matched_file_locations: + print(f" {i}") # Testing # patch_file = parse.PatchFile("../vulnerableforks/patches/CVE-2014-8172.patch") diff --git a/scripts/patch_apply/patchParser.py b/scripts/patch_apply/patchParser.py index b37d2a7..0529d5a 100644 --- a/scripts/patch_apply/patchParser.py +++ b/scripts/patch_apply/patchParser.py @@ -6,7 +6,7 @@ class Patch: - def __init__(self, filename): + def __init__(self): """ Constructor -------------------------- @@ -15,11 +15,17 @@ def __init__(self, filename): _lines is a list of tuples of the format- [(, ),(, ),...,] Nature of Change can be one of the enums defined in natureOfChange (ADDED, REMOVED, CONTEXT) - _lineschanged stores the lines changed info for a patch. ie- The data found between @@s + _oldStart, _oldLength, _newStart, and _newLength store the lines changed info for a patch. + ie- The data found between @@s """ - self._fileName = filename + self._fileName = None self._lines = [] - self._lineschanged = [-1, -1, -1, -1] + self._oldStart = -1 + self._oldLength = -1 + self._newStart = -1 + self._newLength = -1 + self._isNewFile = False + self._isFileRemoved = False def __str__(self): """ @@ -55,40 +61,46 @@ def getFileName(self): """ return self._fileName + def setFileName(self, filename): + self._fileName = filename + + def isNewFile(self): + return self._isNewFile + def setLinesChanged(self, rawData): """ Method used to add lines changed info for a patch """ match = re.fullmatch(r'@@ -([0-9]+),([0-9]+) *\+([0-9]+),([0-9]+) @@.*', rawData) if match is not None: - self._lineschanged[0] = int(match.group(1)) - self._lineschanged[1] = int(match.group(2)) - self._lineschanged[2] = int(match.group(3)) - self._lineschanged[3] = int(match.group(4)) + self._oldStart = int(match.group(1)) + self._oldLength = int(match.group(2)) + self._newStart = int(match.group(3)) + self._newLength = int(match.group(4)) return match = re.fullmatch(r'@@ -([0-9]+) *\+([0-9]+),([0-9]+) @@.*', rawData) if match is not None: - self._lineschanged[0] = int(match.group(1)) - self._lineschanged[1] = 1 - self._lineschanged[2] = int(match.group(2)) - self._lineschanged[3] = int(match.group(3)) + self._oldStart = int(match.group(1)) + self._oldLength = 1 + self._newStart = int(match.group(2)) + self._newLength = int(match.group(3)) return match = re.fullmatch(r'@@ -([0-9]+),([0-9]+) *\+([0-9]+) @@.*', rawData) if match is not None: - self._lineschanged[0] = int(match.group(1)) - self._lineschanged[1] = int(match.group(2)) - self._lineschanged[2] = int(match.group(3)) - self._lineschanged[3] = 1 + self._oldStart = int(match.group(1)) + self._oldLength = int(match.group(2)) + self._newStart = int(match.group(3)) + self._newLength = 1 return match = re.fullmatch(r'@@ -([0-9]+) *\+([0-9]+) @@.*', rawData) if match is not None: - self._lineschanged[0] = int(match.group(1)) - self._lineschanged[1] = 1 - self._lineschanged[2] = int(match.group(2)) - self._lineschanged[3] = 1 + self._oldStart = int(match.group(1)) + self._oldLength = 1 + self._newStart = int(match.group(2)) + self._newLength = 1 return raise ValueError( "Don't know how to handle context line %s" % rawData) @@ -100,7 +112,7 @@ def getLinesChanged(self): Eg: For @@ -20,7 +20,6 @@, this method returns [-20, 7, 20, 6] """ - return self._lineschanged + return (self._oldStart, self._oldLength, self._newStart, self._newLength) def _to_raw(self, string): """ Private helper method to return raw string""" @@ -118,10 +130,12 @@ def canApply(self, applyTo=None): applyTo = os.path.join( os.getcwd(), self.getFileName()) if not os.path.isfile(applyTo): + if self.isNewFile(): + return True return False orgPatch = [] - with open(applyTo, "r", encoding="utf-8") as srcfile: + with open(applyTo, "r", encoding="utf-8", errors='ignore') as srcfile: while True: line = srcfile.readline() if not line: @@ -309,8 +323,7 @@ def Apply(self, applyTo, dry_run=False): ite2 += 1 ite3 += 1 - # if not dry_run: - if False: # test + if not dry_run: writeobj = open(applyTo, "w") for i in orgPatch: i = i.replace("\n", "\\n") @@ -374,14 +387,48 @@ def getPatch(self): """ with open(self.pathToFile) as fileObj: - file = fileObj.read().rstrip() - file = file.split("\n") + file = fileObj.read().split("\n") patchObj = None + oldPatchObj = None + + hunkRemaining = [0, 0] for line in file: - # print( "Line: %s" % line) - # print("________________") + + if hunkRemaining[0] > 0 or hunkRemaining[1] > 0: + if line[0:1] == "-": + contextline = line[1:] + patchObj.addLines(natureOfChange.REMOVED, contextline) + hunkRemaining[0] -= 1 + + elif line[0:1] == "+": + contextline = line[1:] + patchObj.addLines(natureOfChange.ADDED, contextline) + hunkRemaining[1] -= 1 + + elif line[0:1] == ' ': + contextline = line[1:] + patchObj.addLines(natureOfChange.CONTEXT, contextline) + hunkRemaining[0] -= 1 + hunkRemaining[1] -= 1 + + # It's a corrupt patch if we have too many removed or + # added lines for the line count in the hunk header. + assert( hunkRemaining[0] >= 0 and hunkRemaining[1] >= 0) + + if hunkRemaining[0] == 0 and hunkRemaining[1] == 0: + assert( patchObj.getFileName() != None ) + self.patches.append(patchObj) + oldPatchObj = patchObj + patchObj = None + + continue + + # If it's an empty line (including the last newline in the + # file), don't immediately start a new patch. + if line == "": + continue # This is a HACK. In general, we should probably not be # modifying the file name in the patch. In this case @@ -389,58 +436,91 @@ def getPatch(self): # being modified. It also turns out that the unit tests # do this (not that we want to have specific code just for # the unit tests). - if line[0:6] == "+++ b/": - if patchObj is not None: - assert( len(patchObj.getLines()) > 0) - self.patches.append(patchObj) + if line.startswith('+++ b/'): + filename = line.split()[1][2:] if patchObj is None: - filename = line.split()[1][2:] - patchObj = Patch(filename) + patchObj = Patch() + + if patchObj.getFileName() is not None: + # We don't support patches where the two file + # names are different, except for patches that + # either add new files (original file was + # /dev/null) or delete files (new file is + # /dev/null). + assert( patchObj.getFileName() == filename ) + else: + patchObj.setFileName(filename) - elif line[0:3] == '+++': - if patchObj is not None: - assert( len(patchObj.getLines()) > 0) - self.patches.append(patchObj) + elif line.startswith('+++ /dev/null'): + if patchObj is None: + patchObj = Patch() + + patchObj._isFileRemoved = True + pass + + elif line.startswith('+++ '): + filename = line.split()[1] if patchObj is None: - filename = "./" + line.split()[1] - patchObj = Patch(filename) + patchObj = Patch() - elif line[0:3] == '---': - if patchObj is not None: - self.patches.append(patchObj) - patchObj = None + if patchObj.getFileName() is not None: + assert( patchObj.getFileName() == filename ) + else: + patchObj.setFileName(filename) - elif line[0:3] == "@@ ": - contextline = line[2:].split(" @@ ")[-1] - assert( patchObj is not None ) - if len(patchObj.getLines()) != 0: - filename = patchObj.getFileName() - self.patches.append(patchObj) - patchObj = Patch(filename) - patchObj.setLinesChanged(line) + elif line.startswith('--- a/'): + filename = line.split()[1][2:] + + if patchObj is None: + patchObj = Patch() + + if patchObj.getFileName() is not None: + # We don't support patches where the two file + # names are different, except for patches that + # either add new files (original file was + # /dev/null) or delete files (new file is + # /dev/null). + assert( patchObj.getFileName() == filename ) + else: + patchObj.setFileName(filename) + + elif line.startswith('--- /dev/null'): + if patchObj is None: + patchObj = Patch() + + patchObj._isNewFile = True + + elif line.startswith('--- '): + filename = line.split()[1] + + if patchObj is None: + patchObj = Patch() + + if patchObj.getFileName() is not None: + assert( patchObj.getFileName() == filename ) else: - patchObj.setLinesChanged(line) + patchObj.setFileName(filename) + + elif line.startswith('@@ '): + contextline = line[2:].split(" @@ ")[-1] + + if patchObj is None: + patchObj = Patch() + + if patchObj.getFileName() is None: + patchObj.setFileName(oldPatchObj.getFileName()) + + patchObj.setLinesChanged(line) + hunkRemaining = [patchObj._oldLength, patchObj._newLength] patchObj.addLines( natureOfChange.CONTEXT, contextline, ) - elif line[0:1] == "-" and patchObj is not None: - contextline = line[1:] - patchObj.addLines(natureOfChange.REMOVED, contextline) - - elif line[0:1] == "+" and patchObj is not None: - contextline = line[1:] - patchObj.addLines(natureOfChange.ADDED, contextline) - - elif line[0:1] == ' ' and patchObj is not None: - contextline = line[1:] - patchObj.addLines(natureOfChange.CONTEXT, contextline) - else: # A patch can have lots of lines in it that are added # by tools like GIT. These lines have no particular @@ -448,4 +528,4 @@ def getPatch(self): # hunks. pass - self.patches.append(patchObj) + assert(patchObj is None) diff --git a/scripts/patch_context/context_changes.py b/scripts/patch_context/context_changes.py index 7df23fc..f72cac1 100644 --- a/scripts/patch_context/context_changes.py +++ b/scripts/patch_context/context_changes.py @@ -41,12 +41,20 @@ def context_changes(sub_patch, expand=False): file_path = os.path.join( os.getcwd(), sub_patch.getFileName() ) if not os.path.exists(file_path): - return ContextResult( - CONTEXT_DECISION.DONT_RUN.value, - "The file %s does not exist" % (file_path), - None, - False, - ) + if not sub_patch.isNewFile(): + return ContextResult( + CONTEXT_DECISION.DONT_RUN.value, + "The file %s does not exist" % (file_path), + None, + False, + ) + else: + return ContextResult( + CONTEXT_DECISION.RUN.value, + "No context related issues found.", + None, + False, + ) file_slice = slice.SliceParser(file_path) file_slice_parsed = file_slice.slice_parse() diff --git a/scripts/patch_context/slice_and_parse.py b/scripts/patch_context/slice_and_parse.py index ad4e12f..a924cc3 100644 --- a/scripts/patch_context/slice_and_parse.py +++ b/scripts/patch_context/slice_and_parse.py @@ -1,4 +1,4 @@ -import os, subprocess, sys +import os, subprocess, threading, sys import tempfile as tfile import re from enum import Enum @@ -9,35 +9,61 @@ else: src_slice_path += "/srcSliceBuilds/ubuntu/srcslice-ubuntu" +class RunWithTimeout(object): + def __init__(self, cmd): + self.cmd = cmd + self.process = None + self.out = None + self.err = None + + def run(self, timeout): + def target(): + self.process = subprocess.Popen( args=self.cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE ) + self.out, self.err = self.process.communicate() + + thread = threading.Thread(target=target) + thread.start() + + for i in range(0, timeout, 5): + thread.join(5) + if not thread.is_alive(): + break + + print( f"Waited {i} seconds for {self.cmd} to exit." ) + + if thread.is_alive(): + print( f"Timeout waiting for {self.cmd} to exit." ) + self.process.terminate() + thread.join() + self.err += f"Timeout waiting for {self.cmd} to exit.".encode('ascii') + class SliceParser: def __init__(self, file): self.file = file def slice_parse(self): - p = subprocess.Popen( - ["srcml", f"{self.file}", "--position"], stdout=subprocess.PIPE - ) - out, err = p.communicate() - if err: + srcml = RunWithTimeout(["srcml", f"{self.file}", "--position"]) + srcml.run(timeout=120) + + if srcml.err: return None fd, path = tfile.mkstemp(suffix=".xml", prefix="temp") try: with os.fdopen(fd, "w") as tmpo: - tmpo.write(str(out, "utf-8")) - - p = subprocess.Popen( - [ - src_slice_path, - f"{path}", - ], - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - ) - out, err = p.communicate() - - str_out = out.decode("utf-8") + tmpo.write(str(srcml.out, "utf-8")) + + srcslice = RunWithTimeout([src_slice_path, f"{path}"]) + srcslice.run(timeout=120) + + # Remove the "Time is: ...." line from the error output. + srcslice.err = re.sub(b'Time is: [0-9.]*\n', b'', srcslice.err) + + if srcslice.err: + return None + + str_out = srcslice.out.decode("utf-8") slice_dict = {} for line in str_out.splitlines(): diff --git a/scripts/patch_match/test_match.py b/scripts/patch_match/test_match.py index 5ae12c8..e9deb57 100644 --- a/scripts/patch_match/test_match.py +++ b/scripts/patch_match/test_match.py @@ -357,7 +357,7 @@ def compare_nearby(patch_idx, patch_lines, file_idx, file_lines): def find_diffs(patch_obj, file_name, retry_obj=None, match_distance=3000): dmp.Match_Distance = match_distance function_for_patch, patch_lines = patch_obj._lines[0][1], patch_obj._lines[1:] - line_number = patch_obj._lineschanged[2] + line_number = patch_obj._newStart search_lines_with_type = get_file_without_patch(patch_lines) search_lines_without_type = [line[1] for line in search_lines_with_type] diff --git a/tests/patches/applied/add-pluses.patch b/tests/patches/applied/add-pluses.patch new file mode 100644 index 0000000..61b6e5d --- /dev/null +++ b/tests/patches/applied/add-pluses.patch @@ -0,0 +1,12 @@ +--- a/patches/pluses.c 2022-07-08 12:10:09.838919861 -0400 ++++ b/patches/pluses.c 2022-07-08 12:08:21.758457580 -0400 +@@ -1,5 +1,9 @@ + #include + ++/* ++++ This is a line with two plus signs at the front. ++*/ ++ + int main( int argc, char ** argv ) { + printf( "Hello world.\n" ); + } diff --git a/tests/patches/applied/remove-negatives.patch b/tests/patches/applied/remove-negatives.patch new file mode 100644 index 0000000..cea9a43 --- /dev/null +++ b/tests/patches/applied/remove-negatives.patch @@ -0,0 +1,13 @@ +--- a/patches/pluses.c 2022-07-08 12:10:51.635102787 -0400 ++++ b/patches/pluses.c 2022-07-08 12:08:21.758457580 -0400 +@@ -1,10 +1,6 @@ + #include + + /* +--- This is a line with two negative signs at the front. +-*/ +- +-/* + ++ This is a line with two plus signs at the front. + */ + diff --git a/tests/patches/clean/add-file.patch b/tests/patches/clean/add-file.patch new file mode 100644 index 0000000..df31b81 --- /dev/null +++ b/tests/patches/clean/add-file.patch @@ -0,0 +1,65 @@ +--- /dev/null 2022-07-07 08:45:46.573671159 -0400 ++++ patches/switch-new.cpp 2022-04-22 13:51:48.544311829 -0400 +@@ -0,0 +1,62 @@ ++#include ++#include ++ ++enum option_t { ++ OPT_ONE, ++ OPT_TWO, ++ OPT_THREE, ++ OPT_FOUR, ++ OPT_FIVE, ++ OPT_SIX, ++}; ++ ++int print_switch( option_t option ) { ++ ++ if( option > 6 ) { ++ printf( "Bad index %d.", option ); ++ return -1; ++ } ++ ++ switch( option ) { ++ ++ case OPT_ONE: ++ printf( "Found option 1.\n" ); ++ break; ++ ++ case OPT_TWO: ++ printf( "Found option 2.\n" ); ++ break; ++ ++ case OPT_THREE: ++ printf( "Found option 3.\n" ); ++ break; ++ ++ case OPT_FOUR: ++ printf( "Found option 4.\n" ); ++ break; ++ ++ case OPT_FIVE: ++ printf( "Found option 5.\n" ); ++ break; ++ ++ case OPT_SIX: ++ printf( "Found option 6.\n" ); ++ break; ++ ++ default: ++ printf( "Unknown option: %d\n", option ); ++ break; ++ } ++ return 0; ++} ++ ++int main( int argc, char ** argv ) { ++ if( argc < 2 ) { ++ fprintf( stderr, "Missing argument.\n" ); ++ return 1; ++ } ++ ++ option_t option = (option_t)atoi( argv[1] ); ++ print_switch( option ); ++ return 0; ++} diff --git a/tests/patches/clean/add-pluses.patch b/tests/patches/clean/add-pluses.patch new file mode 100644 index 0000000..814d854 --- /dev/null +++ b/tests/patches/clean/add-pluses.patch @@ -0,0 +1,13 @@ +--- a/patches/test.cpp 2022-04-22 13:51:48.544311829 -0400 ++++ b/patches/test.cpp 2022-07-07 17:53:47.927302814 -0400 +@@ -1,6 +1,10 @@ + #include + #include + ++/* ++++ This is a line with two plus signs at the beginning. ++*/ ++ + typedef struct element_t { + size_t length; + size_t max_length; diff --git a/tests/patches/clean/one-line-after-hunk.patch b/tests/patches/clean/one-line-after-hunk.patch new file mode 100644 index 0000000..124dfaf --- /dev/null +++ b/tests/patches/clean/one-line-after-hunk.patch @@ -0,0 +1,51 @@ +--- a/patches/test.cpp 2021-04-16 14:10:28.767487311 -0400 ++++ b/patches/test.cpp 2021-09-07 10:52:01.472170449 -0400 +@@ -1,48 +1 @@ + #include +-#include +- +-typedef struct element_t { +- size_t length; +- size_t max_length; +- char buffer[80]; +-} element_t; +- +-void copy_to_string( element_t &dst, const char * src ) { +- dst.length = strncpy( dst.buffer, src, dst.max_length ); +- if( dst.length >= dst.max_length ) +- dst.length = dst.max_length - 1; +- dst.buffer[dst.length] = '\0'; +- return; +-} +- +-std::string MakeString( const char * str ) { +- char localString[256]; +- memset( localString, 0, 256 ); +- snprintf( localString, 256, "%s", str ); +- +- printf( "Location of the string: %p\n", localString ); +- +- return localString; +-} +- +-int main( int argc, char ** argv ) { +- if( argc < 1 ) { +- printf( "This isn't going to work. I need an argument!\n" ); +- return 1; +- } +- +- std::string value = MakeString( argv[1] ); +- +- printf( "String: %s\n", value.c_str() ); +- printf( "Location of the string: %p\n", value.c_str() ); +- +- // Now, copy it into another buffer. +- element_t element; +- element.max_length = sizeof(element.buffer); +- +- copy_to_string( element, value.c_str() ); +- +- printf( "String: %s\n", element.buffer ); +- +- return 0; +-} diff --git a/tests/patches/clean/one-line-hunk.patch b/tests/patches/clean/one-line-hunk.patch new file mode 100644 index 0000000..7e7cc87 --- /dev/null +++ b/tests/patches/clean/one-line-hunk.patch @@ -0,0 +1,6 @@ +--- a/patches/one-line.cpp 2021-09-07 10:46:59.703099221 -0400 ++++ b/patches/one-line.cpp 2021-09-07 10:48:11.279353303 -0400 +@@ -1,1 +1,3 @@ ++#ifndef ONE_LINE + void main(int argc, char **argv) { return 0; } ++#endif /* ONE_LINE */ diff --git a/tests/patches/clean/remove-file.patch b/tests/patches/clean/remove-file.patch new file mode 100644 index 0000000..a1c699e --- /dev/null +++ b/tests/patches/clean/remove-file.patch @@ -0,0 +1,65 @@ +--- patches/switch.cpp 2022-04-22 13:51:48.544311829 -0400 ++++ /dev/null 2022-07-07 08:45:46.573671159 -0400 +@@ -1,62 +0,0 @@ +-#include +-#include +- +-enum option_t { +- OPT_ONE, +- OPT_TWO, +- OPT_THREE, +- OPT_FOUR, +- OPT_FIVE, +- OPT_SIX, +-}; +- +-int print_switch( option_t option ) { +- +- if( option > 6 ) { +- printf( "Bad index %d.", option ); +- return -1; +- } +- +- switch( option ) { +- +- case OPT_ONE: +- printf( "Found option 1.\n" ); +- break; +- +- case OPT_TWO: +- printf( "Found option 2.\n" ); +- break; +- +- case OPT_THREE: +- printf( "Found option 3.\n" ); +- break; +- +- case OPT_FOUR: +- printf( "Found option 4.\n" ); +- break; +- +- case OPT_FIVE: +- printf( "Found option 5.\n" ); +- break; +- +- case OPT_SIX: +- printf( "Found option 6.\n" ); +- break; +- +- default: +- printf( "Unknown option: %d\n", option ); +- break; +- } +- return 0; +-} +- +-int main( int argc, char ** argv ) { +- if( argc < 2 ) { +- fprintf( stderr, "Missing argument.\n" ); +- return 1; +- } +- +- option_t option = (option_t)atoi( argv[1] ); +- print_switch( option ); +- return 0; +-} diff --git a/tests/patches/clean/remove-negatives.patch b/tests/patches/clean/remove-negatives.patch new file mode 100644 index 0000000..79da84d --- /dev/null +++ b/tests/patches/clean/remove-negatives.patch @@ -0,0 +1,12 @@ +--- a/patches/negatives.c 2022-07-07 17:55:34.667754758 -0400 ++++ b/patches/negatives.c 2022-07-07 17:56:47.488063079 -0400 +@@ -1,9 +1,5 @@ + #include + +-/* +--- This is a line with two negatives at the front. +-*/ +- + int main( int argc, char ** argv ) { + printf( "Hello world.\n" ); + } diff --git a/tests/patches/git/binary-2.patch b/tests/patches/git/binary-2.patch new file mode 100644 index 0000000..721ba77 --- /dev/null +++ b/tests/patches/git/binary-2.patch @@ -0,0 +1,4 @@ +diff --git a/tests/patches/test.so b/tests/patches/test.so +new file mode 100644 +index 0000000..649223da +Binary files /dev/null and b/tests/patches/test.so differ diff --git a/tests/patches/git/binary.patch b/tests/patches/git/binary.patch new file mode 100644 index 0000000..e34d90b --- /dev/null +++ b/tests/patches/git/binary.patch @@ -0,0 +1,7 @@ +diff --git a/patches/test.so b/patches/test.so +new file mode 100644 +index 0..8 +GIT binary patch +literal 0 +HcmV?d00001 + diff --git a/tests/patches/negatives.c b/tests/patches/negatives.c new file mode 100644 index 0000000..197654b --- /dev/null +++ b/tests/patches/negatives.c @@ -0,0 +1,9 @@ +#include + +/* +-- This is a line with two negatives at the front. +*/ + +int main( int argc, char ** argv ) { + printf( "Hello world.\n" ); +} diff --git a/tests/patches/one-line.cpp b/tests/patches/one-line.cpp new file mode 100644 index 0000000..de287ac --- /dev/null +++ b/tests/patches/one-line.cpp @@ -0,0 +1 @@ +void main(int argc, char **argv) { return 0; } diff --git a/tests/patches/pluses.c b/tests/patches/pluses.c new file mode 100644 index 0000000..10701a2 --- /dev/null +++ b/tests/patches/pluses.c @@ -0,0 +1,9 @@ +#include + +/* +++ This is a line with two plus signs at the front. +*/ + +int main( int argc, char ** argv ) { + printf( "Hello world.\n" ); +} diff --git a/tests/test_apply.py b/tests/test_apply.py index 584a304..3d5e6f3 100755 --- a/tests/test_apply.py +++ b/tests/test_apply.py @@ -66,7 +66,7 @@ def test_context_comment(self): self.assertRegex( fakeOutput.getvalue(), 'Patch failed to apply with git apply' ) self.assertRegex( fakeOutput.getvalue(), '1 subpatches can be applied successfully:' ) - self.assertRegex( fakeOutput.getvalue(), 'would have been successfully applied \(dry run\)' ) + self.assertRegex( fakeOutput.getvalue(), r'would have been successfully applied \(dry run\)' ) def test_context_function(self): with patch('sys.stdout', new=StringIO()) as fakeOutput: @@ -78,7 +78,7 @@ def test_context_function(self): self.assertRegex( fakeOutput.getvalue(), 'Patch failed to apply with git apply' ) self.assertRegex( fakeOutput.getvalue(), '1 subpatches can be applied successfully:' ) - self.assertRegex( fakeOutput.getvalue(), 'would have been successfully applied \(dry run\)' ) + self.assertRegex( fakeOutput.getvalue(), r'would have been successfully applied \(dry run\)' ) def test_applied_offset(self): with patch('sys.stdout', new=StringIO()) as fakeOutput: @@ -112,6 +112,26 @@ def test_bad_index(self): self.assertNotRegex( fakeOutput.getvalue(), 'Subpatches that were applied by git apply:' ) self.assertRegex( fakeOutput.getvalue(), 'Subpatches that did not apply, and we could not find where the patch should be applied' ) + def test_binary(self): + with patch('sys.stdout', new=StringIO()) as fakeOutput: + apply.main( pathToPatch='patches/git/binary.patch', + dry_run=True, + reverse=False, + verbose=2, + ) + + self.assertRegex( fakeOutput.getvalue(), ':skipped' ) + + def test_binary2(self): + with patch('sys.stdout', new=StringIO()) as fakeOutput: + apply.main( pathToPatch='patches/git/binary-2.patch', + dry_run=True, + reverse=False, + verbose=2, + ) + + self.assertRegex( fakeOutput.getvalue(), 'cannot apply binary patch to ' ) + self.assertRegex( fakeOutput.getvalue(), r'Binary patch detected\.' ) if __name__ == "__main__": unittest.main() diff --git a/tests/test_basic.py b/tests/test_basic.py index c15b0d0..8183aee 100755 --- a/tests/test_basic.py +++ b/tests/test_basic.py @@ -44,15 +44,24 @@ def f(self): # There should only be one hunk in this file. self.assertEqual( len(patch_file.patches), 1 ) + testcase = self.id().split('.')[-1] + if testcase in self.errors: + expected = self.errors[testcase] + else: + expected = { + 'message': r'No context related issues found.', + 'canApply': precheckStatus.ALREADY_APPLIED + } + for hunk in patch_file.patches: result = context.context_changes(hunk) - self.assertEqual(result.messages, "No context related issues found.") + self.assertEqual(result.messages, expected['message']) for hunk in patch_file.patches: filename = os.path.join( os.getcwd(), hunk.getFileName() ) self.assertTrue( os.path.isfile(filename), filename ) result = hunk.canApply() - self.assertEqual( result, precheckStatus.ALREADY_APPLIED, hunk ) + self.assertEqual( result, expected['canApply'], hunk ) return f @@ -64,7 +73,16 @@ def setUp(self): def tearDown(self): os.chdir(self.oldcwd) - pass + errors = { + 'test_remove_offset': { + 'message': r'A context match was not found.', + 'canApply': precheckStatus.ALREADY_APPLIED + }, + 'test_remove_offset_similar2': { + 'message': r'A context match was not found.', + 'canApply': precheckStatus.ALREADY_APPLIED + }, + } class PatchTests(type): def __new__(mcls, name, bases, attrs): @@ -98,9 +116,9 @@ def f(self): for hunk in patch_file.patches: filename = os.path.join( os.getcwd(), hunk.getFileName() ) - self.assertTrue( os.path.isfile(filename), filename ) - result = hunk.canApply() - self.assertTrue( result, hunk ) + if not hunk.isNewFile(): + self.assertTrue( os.path.isfile(filename), filename ) + self.assertTrue( hunk.canApply(), hunk ) return f diff --git a/tests/test_changes.py b/tests/test_changes.py index 84db863..04e39f8 100755 --- a/tests/test_changes.py +++ b/tests/test_changes.py @@ -78,7 +78,7 @@ class TestChanges(unittest.TestCase, metaclass=PatchTests): 'canApply': precheckStatus.NO_MATCH_FOUND }, 'test_code_missing': { - 'message': r'^No context related issues found\.$', + 'message': r'^A context match was not found\.$', 'canApply': precheckStatus.CAN_APPLY }, } diff --git a/tests/test_context.py b/tests/test_context.py index e8dda2b..456be7b 100755 --- a/tests/test_context.py +++ b/tests/test_context.py @@ -90,11 +90,11 @@ class TestContext(unittest.TestCase, metaclass=ContextPatchTests): 'canApply': precheckStatus.NO_MATCH_FOUND }, 'test_variable_change': { - 'message': r'^This patch can be applied\.$', + 'message': r'recommend to not run this patch', 'canApply': precheckStatus.NO_MATCH_FOUND }, 'test_variable_change_declaration': { - 'message': r'^This patch can be applied\.$', + 'message': r'recommend to not run this patch', 'canApply': precheckStatus.NO_MATCH_FOUND }, 'test_variable_change_LHS': { diff --git a/tests/test_git.py b/tests/test_git.py index 2a2456f..35c4b9e 100755 --- a/tests/test_git.py +++ b/tests/test_git.py @@ -72,6 +72,14 @@ class TestPatches(unittest.TestCase, metaclass=PatchTests): 'test_bad_index': { 'result': 'skipped', 'success': False + }, + 'test_binary': { + 'result': 'skipped', + 'success': False + }, + 'test_binary_2': { + 'result': 'patch does not apply', + 'success': False } }