You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2015/12/16 13:42:07 UTC

svn commit: r1720351 - in /subversion/trunk/subversion: include/private/svn_diff_private.h libsvn_client/patch.c libsvn_diff/parse-diff.c tests/cmdline/patch_tests.py

Author: rhuijben
Date: Wed Dec 16 12:42:06 2015
New Revision: 1720351

URL: http://svn.apache.org/viewvc?rev=1720351&view=rev
Log:
Resolve issue #4609, by extending the diff parser with some additional
verifications and a match penalty.

Temporarily keep this as private api to allow backporting this patch.

* subversion/include/private/svn_diff_private.h
  (svn_diff_hunk__get_fuzz_penalty): New function.

* subversion/libsvn_client/patch.c
  (match_hunk): Check for fuzz penalties.

* subversion/libsvn_diff/parse-diff.c
  (svn_diff_hunk_t): Add two fields.
  (add_or_delete_single_line): Safely initialize everything to 0.
  (parse_next_hunk): Handle missing context lines by updating the number
    of lines in the hunk and applying a fuzz penalty.
  (svn_diff_hunk__get_fuzz_penalty): New function.

* subversion/tests/cmdline/patch_tests.py
  (missing_trailing_context): Remove XFail. Expect fuzz.

Modified:
    subversion/trunk/subversion/include/private/svn_diff_private.h
    subversion/trunk/subversion/libsvn_client/patch.c
    subversion/trunk/subversion/libsvn_diff/parse-diff.c
    subversion/trunk/subversion/tests/cmdline/patch_tests.py

Modified: subversion/trunk/subversion/include/private/svn_diff_private.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_diff_private.h?rev=1720351&r1=1720350&r2=1720351&view=diff
==============================================================================
--- subversion/trunk/subversion/include/private/svn_diff_private.h (original)
+++ subversion/trunk/subversion/include/private/svn_diff_private.h Wed Dec 16 12:42:06 2015
@@ -141,6 +141,13 @@ svn_diff_hunk__create_deletes_single_lin
                                           apr_pool_t *result_pool,
                                           apr_pool_t *scratch_pool);
 
+/** Fetches the penalty fuzz of the diff hunk. The patch file parser applies
+ * an additional penalty on some cases of bad patch files. These cases may
+ * include errors as headers that aren't consistent with bodies, etc.
+ */
+svn_linenum_t
+svn_diff_hunk__get_fuzz_penalty(const svn_diff_hunk_t *hunk);
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */

Modified: subversion/trunk/subversion/libsvn_client/patch.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/patch.c?rev=1720351&r1=1720350&r2=1720351&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/patch.c (original)
+++ subversion/trunk/subversion/libsvn_client/patch.c Wed Dec 16 12:42:06 2015
@@ -1567,12 +1567,20 @@ match_hunk(svn_boolean_t *matched, targe
   svn_linenum_t hunk_length;
   svn_linenum_t leading_context;
   svn_linenum_t trailing_context;
+  svn_linenum_t fuzz_penalty;
 
   *matched = FALSE;
 
   if (content->eof)
     return SVN_NO_ERROR;
 
+  fuzz_penalty = svn_diff_hunk__get_fuzz_penalty(hunk);
+
+  if (fuzz_penalty > fuzz)
+    return SVN_NO_ERROR;
+  else
+    fuzz -= fuzz_penalty;
+
   saved_line = content->current_line;
   lines_read = 0;
   lines_matched = FALSE;

Modified: subversion/trunk/subversion/libsvn_diff/parse-diff.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_diff/parse-diff.c?rev=1720351&r1=1720350&r2=1720351&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_diff/parse-diff.c (original)
+++ subversion/trunk/subversion/libsvn_diff/parse-diff.c Wed Dec 16 12:42:06 2015
@@ -89,6 +89,10 @@ struct svn_diff_hunk_t {
   /* Did we see a 'file does not end with eol' marker in this hunk? */
   svn_boolean_t original_no_final_eol;
   svn_boolean_t modified_no_final_eol;
+
+  /* Fuzz penalty, triggered by bad patch targets */
+  svn_linenum_t original_fuzz;
+  svn_linenum_t modified_fuzz;
 };
 
 struct svn_diff_binary_patch_t {
@@ -122,7 +126,7 @@ add_or_delete_single_line(svn_diff_hunk_
                           apr_pool_t *result_pool,
                           apr_pool_t *scratch_pool)
 {
-  svn_diff_hunk_t *hunk = apr_palloc(result_pool, sizeof(*hunk));
+  svn_diff_hunk_t *hunk = apr_pcalloc(result_pool, sizeof(*hunk));
   static const char *hunk_header[] = { "@@ -1 +0,0 @@\n", "@@ -0,0 +1 @@\n" };
   const apr_size_t header_len = strlen(hunk_header[add]);
   const apr_size_t len = strlen(line);
@@ -285,6 +289,12 @@ svn_diff_hunk_get_trailing_context(const
   return hunk->trailing_context;
 }
 
+svn_linenum_t
+svn_diff_hunk__get_fuzz_penalty(const svn_diff_hunk_t *hunk)
+{
+  return hunk->patch->reverse ? hunk->original_fuzz : hunk->modified_fuzz;
+}
+
 /* Baton for the base85 stream implementation */
 struct base85_baton_t
 {
@@ -1318,6 +1328,21 @@ parse_next_hunk(svn_diff_hunk_t **hunk,
 
   if (hunk_seen && start < end)
     {
+      /* Did we get the number of context lines announced in the header?
+
+         If not... let's limit the number from the header to what we
+         actually have, and apply a fuzz penalty */
+      if (original_lines)
+        {
+          (*hunk)->original_length -= original_lines;
+          (*hunk)->original_fuzz = original_lines;
+        }
+      if (modified_lines)
+        {
+          (*hunk)->modified_length -= modified_lines;
+          (*hunk)->modified_fuzz = modified_lines;
+        }
+
       (*hunk)->patch = patch;
       (*hunk)->apr_file = apr_file;
       (*hunk)->leading_context = leading_context;

Modified: subversion/trunk/subversion/tests/cmdline/patch_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/patch_tests.py?rev=1720351&r1=1720350&r2=1720351&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/patch_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/patch_tests.py Wed Dec 16 12:42:06 2015
@@ -7528,7 +7528,6 @@ def patch_move_and_change(sbox):
                                        [], True, True,
                                        '--reverse-diff')
 
-@XFail()
 @Issue(4609)
 def missing_trailing_context(sbox):
   "missing trailing context"
@@ -7564,8 +7563,10 @@ def missing_trailing_context(sbox):
   svntest.main.file_write(patch_file_path, ''.join(unidiff_patch))
 
   # GNU patch will apply the hunk with fuzz 1 and modify only the 'c' line.
+  # Our patch file finds the length mismatch and applies a penalty.
   expected_output = [
     'U         %s\n' % sbox.ospath('A/mu'),
+    '>         applied hunk @@ -1,4 +1,4 @@ with fuzz 1\n',
   ]
   expected_disk = svntest.main.greek_state.copy()
   expected_disk.tweak('A/mu', contents =