svn-3698

Version:

1.7.0

Bug Link:

http://subversion.tigris.org/issues/show_bug.cgi?id=3698

Patch Link:

http://svn.apache.org/viewvc?diff_format=h&view=revision&revision=1028152

Symptom:

*background: 'svn patch' : apply a unidiff patch to the working copy

'svn patch' should validate svn: properties.

for example, setting svn:executable on a directory should be prohibited. However, in the buggy version, we can set ‘svn:executable’ on a new directory with ‘svn patch’.

How it is diagnosed:

reproduced.

How to reproduce:

1. Run ‘svn patch PATCH.file’ to apply the patch.

 

PATCH.file is following:

Adds ‘unversioned-new directory’ “t5” with property “svn:executable” and “add”.

“svn:executable” on a directory is illegal but it goes on.

Index: t5

===================================================================

--- t5        (revision 10)

+++ t5 (working copy)

 

Property changes on: t5

___________________________________________________________________

Added: add

## -0,0 +1 ##

+add

Added: svn:executable

## -0,0 +1 ##

+*

 

2. Run ‘svn proplist t5’ to print out the changed property of the directory ‘t5’

Properties on 't5':

  svn:executable

add

 

·         ‘svn:executable’ is added illegally.

 

Root Cause:

Brief:

When creating a new directory by the patch file, there’s no routine to handle the illegal property setting.

Detail:

In this case, we check it out whether the property is legal or not by calling ‘svn_wc_canonicalize_svn_prop’. Also, if ‘dry-run’ is true, then the function just sees if the given operations work correctly. If ‘dry-run’ is not true, the function applies operations. If operation fails, then reject all hunks of operations if it is illegal.

 

--- subversion/trunk/subversion/libsvn_client/patch.c            2010/10/28 00:42:05            1028151

+++ subversion/trunk/subversion/libsvn_client/patch.c         2010/10/28 00:44:02            1028152

@@ -1636,13 +1636,12 @@ close_target_streams(const patch_target_

           SVN_ERR(svn_stream_close(prop_content_info->patched));

             }

 

-

-   /* .. And then streams associted with the file. The reject stream is

-        * shared between all target_content_info structures. */

+  /* .. and then streams associated with the file.

+   * ### We're not closing the reject stream -- it still needed and

+   * ### will be closed later in write_out_rejected_hunks(). */

   if (target->kind_on_disk == svn_node_file)

     SVN_ERR(svn_stream_close(target->content_info->stream));

   SVN_ERR(svn_stream_close(target->content_info->patched));

-  SVN_ERR(svn_stream_close(target->content_info->reject));

 

   return SVN_NO_ERROR;

 }

@@ -2148,6 +2147,8 @@ write_out_rejected_hunks(patch_target_t

                          svn_boolean_t dry_run,

                          apr_pool_t *pool)

 {

+  SVN_ERR(svn_stream_close(target->content_info->reject));

+

   if (! dry_run && (target->had_rejects || target->had_prop_rejects))

         {

           /* Write out rejected hunks, if any. */

@@ -2171,14 +2172,6 @@ install_patched_prop_targets(patch_targe

   apr_hash_index_t *hi;

   apr_pool_t *iterpool;

-  if (dry_run)

-        {

-          if (! target->has_text_changes && target->kind_on_disk == svn_node_none)

-            target->added = TRUE;

-

-          return SVN_NO_ERROR;

-        }

-

   iterpool = svn_pool_create(scratch_pool);

 

   for (hi = apr_hash_first(scratch_pool, target->prop_targets);

@@ -2190,14 +2183,17 @@ install_patched_prop_targets(patch_targe

           svn_stream_t *patched_stream;

           svn_stringbuf_t *line;

           svn_stringbuf_t *prop_content;

+          const svn_string_t *prop_val;

           const char *eol_str;

           svn_boolean_t eof;

+          svn_error_t *err;

 

       svn_pool_clear(iterpool);

 

           /* For a deleted prop we only set the value to NULL. */

           if (prop_target->operation == svn_diff_op_deleted)

             {

+              if (! dry_run)

           SVN_ERR(svn_wc_prop_set4(ctx->wc_ctx, target->local_abspath,

                                        prop_target->name, NULL,

                                    TRUE /* skip_checks */,

@@ -2246,30 +2242,74 @@ install_patched_prop_targets(patch_targe

               && target->kind_on_disk == svn_node_none

               && ! target->added)

             {

-          SVN_ERR(svn_io_file_create(target->local_abspath, "", scratch_pool));

+              if (! dry_run)

+                {

+              SVN_ERR(svn_io_file_create(target->local_abspath, "",

+                                         scratch_pool));

           SVN_ERR(svn_wc_add_from_disk(ctx->wc_ctx, target->local_abspath,

                                        ctx->cancel_func, ctx->cancel_baton,

-                                           NULL, NULL, /* suppress notification */

+                                               /* suppress notification */

+                                           NULL, NULL,

                                        iterpool));

+                }

 

// only if it is not dry-run, creating a new directory with the given property.

 

               target->added = TRUE;

             }

 

-          /* ### How should we handle SVN_ERR_ILLEGAL_TARGET and

-           * ### SVN_ERR_BAD_MIME_TYPE?

-           *

-           * ### stsp: I'd say reject the property hunk.

-           * ###           We should verify all modified prop hunk texts using

-           * ###           svn_wc_canonicalize_svn_prop() before starting the

-           * ###           patching process, to reject them as early as possible. */

-      SVN_ERR(svn_wc_prop_set4(ctx->wc_ctx, target->local_abspath,

+          /* Attempt to set the property, and reject all hunks if this fails. */

+          prop_val = svn_string_create_from_buf(prop_content, iterpool);

+          if (dry_run)

+            {

+              const svn_string_t *canon_propval;

+

+              err = svn_wc_canonicalize_svn_prop(&canon_propval,

                                prop_target->name,

-                               svn_string_create_from_buf(prop_content,

-                                                          iterpool),

-                                   TRUE /* skip_checks */,

+                                             prop_val, target->local_abspath,

+                                             target->db_kind,

+                                             TRUE, /* ### Skipping checks */

+                                             NULL, NULL,

+                                                 iterpool);

+            }

+          else

+            {

+              err = (svn_wc_prop_set4(ctx->wc_ctx, target->local_abspath,

+                                  prop_target->name, prop_val,

+                                  TRUE, /* ### Skipping checks */

                                NULL, NULL,

                                iterpool));

         }

 

+          if (err)

+            {

+              /* ### The errors which svn_wc_canonicalize_svn_prop() will

+               * ### return aren't documented. */

+              if (err->apr_err == SVN_ERR_ILLEGAL_TARGET ||

+                  err->apr_err == SVN_ERR_NODE_UNEXPECTED_KIND ||

+                  err->apr_err == SVN_ERR_IO_UNKNOWN_EOL ||

+                  err->apr_err == SVN_ERR_BAD_MIME_TYPE ||

+                  err->apr_err == SVN_ERR_CLIENT_INVALID_EXTERNALS_DESCRIPTION)

+                {

+                  int i;

+

+                  svn_error_clear(err);

+

+                  for (i = 0; i < prop_target->content_info->hunks->nelts; i++)

+                    {

+                      hunk_info_t *hunk_info;

+

+                      hunk_info = APR_ARRAY_IDX(prop_target->content_info->hunks,

+                                                i, hunk_info_t *);

+                      hunk_info->rejected = TRUE;

+                      SVN_ERR(reject_hunk(target, prop_target->content_info,

+                                      hunk_info, prop_target->name,

+                                      iterpool));

+                    }

+                }

+              else

+                return svn_error_return(err);

+            }

+

+        }

+

// if this operation is illegal, reject all the hunks using ‘reject_hunk’ function.

 

   svn_pool_destroy(iterpool);

 

   return SVN_NO_ERROR;

Failure type

wrong result

Is there any log message?

No

Can ErrLog insert a log message?

No