svn-3649

Version:

1.6.x

Bug Link:

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

Patch Link:

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

Symptom:

svn mkdir  --parent always complains about the first base directory alphabetically that it does not exist (which actually exists) when two or more URLs are specified with different paths off the root of the repo.

How it is diagnosed:

reproduced!


[peh003@soprano repository]$ svnadmin create repos

[peh003@soprano repository]$ svn mkdir -m ' ' file:///home/yyzhou/peh003/repository/repos/test1 file:///home/yyzhou/peh003/repository/repos/test2

Committed revision 1.

[peh003@soprano repository]$ svn mkdir -m' ' --parents file://`pwd`/repos/test1/one/two file://`pwd`/repos/test2/one/two

svn: Path 'test1' not present

/*The expected result should be that the mkdir succeed.*/

[peh003@soprano repository]$ svn ls --depth=infinity file://`pwd`/repos

test1/                     /*it actually exists!*/

test2/

Root Cause:

Brief:

The base directory of the session is set incorrectly.

Detail:

There are four target directories to be made in sequence:

"test1/one"

"test1/one/two"

"test2/one"

"test2/one/two"

When they use two iterations to generate these four targets given the provided two urls

file:///home/yyzhou/peh003/repository/repos/test1/one/two",

file:///home/yyzhou/peh003/repository/repos/test2/one/two",

the session’s base directory is set twice in the meantime.  The first time it’s set to “test1”,

the second time it’s set to “test2”.

And then when they want to make the first target “test1/one”, they try to enter the parent part of “test1/one” that exists (i.e. “test1”) to make dir “one”. Since the session’s base directory is still “test2”, the full path of the parent part is then “test2/test1”, which of course doest not exist. so the error message complains “test1” does not exist, which is quite confusing. If it complains the “test2/test1” does not exist, it would save much time to diagnose this bug(at least for me).

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

Where the error message was printed?

static svn_error_t *

open_directory(...)

{

  ...

  SVN_ERR(svn_fs_check_path(&kind, eb->txn_root, full_path, pool));

  if (kind == svn_node_none)

    return svn_error_createf(SVN_ERR_FS_NOT_DIRECTORY, NULL,

                             _("Path '%s' not present"),

                             path);

  ...

  return SVN_NO_ERROR;

}

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

Path from fault to failure

static svn_error_t *
mkdir_urls(...)
{
 ...
 /* Find any non-existent parent directories */
         if (make_parents)
//make_parents = 1
           {
                     apr_array_header_t *all_urls = apr_array_make(pool, urls->nelts,
                                                   sizeof(const char *));

                      /*first_url =                                                 

"file:///home/yyzhou/peh003/repository/repos/test1/one/two" */
const char *first_url = APR_ARRAY_IDX(urls, 0, const char *);
apr_pool_t *iterpool = svn_pool_create(pool);

                      /*session->priv->fs_path->data is set to "/test1/one/two"*/
                     SVN_ERR(
svn_client__open_ra_session_internal(&ra_session,                                         first_url...));
                     
for (i = 0; i < urls->nelts; i++) //urls->nelts = 2
                       {
                                 const char *url = APR_ARRAY_IDX(urls, i, const char *);
                                 svn_pool_clear(iterpool);

                              /* Fault point!

                        In the first iteration, ra_session->priv->fs_path->data is                                 changed from "/test1/one/two" to "test1";

                        In the second iteration, ra_session->priv->fs_path->data is                                 changed from "/test1/one/two" to  "test2", later, this fault                                 takes a long way to propagate         to the failure point!!!!

                      */
                         SVN_ERR(
add_url_parents(ra_session, url, all_urls, iterpool,                                         pool));
       }
      svn_pool_destroy(iterpool);
      urls = all_urls;
 }
 ...
 qsort(targets->elts, targets->nelts, targets->elt_size,
       svn_sort_compare_paths);
/*        After qsort:

(gdb)  p ((const char **)(targets)->elts)[0]

$4 = 0x6543d0 "test1/one"

(gdb)  p ((const char **)(targets)->elts)[1]

$5 = 0x654408 "test1/one/two"

(gdb)  p ((const char **)(targets)->elts)[2]

$6 = 0x654440 "test2/one"ra_session->priv->fs_path->data

(gdb)  p ((const char **)(targets)->elts)[3]

$7 = 0x654478 "test2/one/two"

*/

/*We should reparent the ra_session->priv->fs_path->data to the common part of all the 4 target directories above , in this case since common is the repository’s root url, a_session->priv->fs_path->data will be set to"/", which is the root of all directories in svn’s repsentation, and when later opening the first target url,"test1/one", and the “test1” part will be joined with ra_session->priv->fs_path->data to open first, which is valid since "/test1" exists already”.

If the ra_session->priv->fs_path->data is still "test2", when joining a full path it will be "/test2/test1", which doesn’t exist!

*/

+ if (ra_session)
+  SVN_ERR(svn_ra_reparent(ra_session, common, pool));
  ...

 /*here ra_session->priv->fs_path->data’s wrong value will infects edit_baton-> base_path!!!!!*/
 SVN_ERR(
svn_ra_get_commit_editor3(ra_session, &editor, &edit_baton,...));

 /* Call the path-based editor driver. */
 err = svn_delta_path_driver(editor,
edit_baton, SVN_INVALID_REVNUM,
                             targets, path_driver_cb_func,
                             (void *)editor, pool);
 if (err)
//failed here!!
  ...
  /* Close the edit. */
  return editor->close_edit(edit_baton, pool);

}

svn_error_t *

svn_delta_path_driver(...)

{

          ...

          /* Sort the paths in a depth-first directory-ish order. */

          qsort(paths->elts, paths->nelts, paths->elt_size, svn_sort_compare_paths);

/*(gdb) p ((const char **)(paths)->elts)[0]

$7 = 0x654920 "test1/one"

...*/

          path = APR_ARRAY_IDX(paths, 0, const char *); //path="test1/one"

          if (svn_path_is_empty(path))

                  …

          else

            {

                     /*

edit_baton->base_path="/test2", which will infect                         db->edit_baton->base_path

*/

                      SVN_ERR(editor->open_root(edit_baton, revision, subpool, &db));

            }

          item->pool = subpool;

          item->dir_baton = db;

          APR_ARRAY_PUSH(db_stack, void *) = item; //db_stack get infected!!!!

  /* Now, loop over the commit items, traversing the URL tree and

     driving the editor. */

for (; i < paths->nelts; i++)

{

         ...

              /* Get the next path. */

              path = APR_ARRAY_IDX(paths, i, const char *);//"test1/one"

               ...

              svn_path_split(path, &pdir, &bname, iterpool);//pdir="test1"

              if (strlen(pdir) > common_len)

                {

                          ...

                          while (1)

                            {

                                      const char *rel = pdir;// rel="test1"

                            ...

                                          /* Open the subdirectory. */

                        //fails here!

                                           SVN_ERR(open_dir(db_stack, editor, rel, revision, pool));

                             …

                }

          }

}

             ...

}

static svn_error_t *

open_dir(apr_array_header_t *db_stack,...const char *path,...)

{

  ...

  SVN_ERR_ASSERT(db_stack && db_stack->nelts);

  item = APR_ARRAY_IDX(db_stack, db_stack->nelts - 1, void *);

  parent_db = item->dir_baton;

  subpool = svn_pool_create(pool);

  /*path="test1",db->edit_baton->base_path ="/test2", which got infection            

    from an error value long away from ra_session in mkdir_urls*/

  SVN_ERR(editor->open_directory(path, parent_db, revision, subpool, &db));//fails here!

  ...

  return SVN_NO_ERROR;

}

open_directory(...)

{

  struct dir_baton *pb = parent_baton;

  struct edit_baton *eb = pb->edit_baton;

  svn_node_kind_t kind;

  const char *full_path = svn_path_join(eb->base_path, path, pool);      

   //for the buggy case,base_path="/test2”, full_path="/test2/test1"

   //for the correct case,base_path="/”,  full_path="/test1"

  /* Check PATH in our transaction.  If it does not exist,

     return a 'Path not present' error. */

  SVN_ERR(svn_fs_check_path(&kind, eb->txn_root, full_path, pool));

  if (kind == svn_node_none)

    return svn_error_createf(SVN_ERR_FS_NOT_DIRECTORY, NULL,

                             _("Path '%s' not present"),

                             path);

         ...

          return SVN_NO_ERROR;

}

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

How the error value is generated

(The below use the first iteration of  the above for loop  to show how ra_session->priv->fs_path->data get infected!!)

/* Append URL, and all it's non-existent parent directories, to TARGETS.

   Use TEMPPOOL for temporary allocations and POOL for any additions to

   TARGETS.

   But actually, This function has the "side effect" to change ra_session’s                        

   ra_session->priv->fs_path->data value !!

*/

static svn_error_t *

add_url_parents(svn_ra_session_t *ra_session,

                const char *url,

                apr_array_header_t *targets,

                apr_pool_t *temppool,

                apr_pool_t *pool)

{

  svn_node_kind_t kind;

  const char *parent_url;

  /*url = "file:///home/yyzhou/peh003/repository/repos/test1/one/two"

    parent_url = "file:///home/yyzhou/peh003/repository/repos/test1/one"

  */

  svn_path_split(url, &parent_url, NULL, pool);

  //ra_session->priv->fs_path->data is set to "test1/one"

  SVN_ERR(svn_ra_reparent(ra_session, parent_url, temppool));

 //but since "test1/one" does not exist, after this check, it will do recursive call

  SVN_ERR(svn_ra_check_path(ra_session, "", SVN_INVALID_REVNUM, &kind,

                            temppool));

  if (kind == svn_node_none) // kind = svn_node_none

          //in the recursive call, ra_session->priv->fs_path->data is set to "test1”!

            SVN_ERR(add_url_parents(ra_session, parent_url, targets, temppool, pool));

  APR_ARRAY_PUSH(targets, const char *) = url;

  return SVN_NO_ERROR;

}

svn_error_t *svn_ra_reparent(svn_ra_session_t *session,

                             const char *url,

                             apr_pool_t *pool)

{

  /*first time add_url_parents call it                                   

url="file:///home/yyzhou/peh003/repository/repos/test1/one"

     second time url="file:///home/yyzhou/peh003/repository/repos/test1/"

  */

  const char *repos_root;

  SVN_ERR(svn_ra_get_repos_root2(session, &repos_root, pool));

 /*repos_root=“file:///home/yyzhou/peh003/repository/repos”*/

  if (! svn_uri_is_ancestor(repos_root, url))

        ...

  return session->vtable->reparent(session, url, pool);

}

static svn_error_t *

svn_ra_local__reparent(svn_ra_session_t *session,

                       const char *url,

                       apr_pool_t *pool)

{

  svn_ra_local__session_baton_t *sess = session->priv;

  const char *relpath = "";

  /*first time url ="file:///home/yyzhou/peh003/repository/repos/test1/one"  */

 /*second time url ="file:///home/yyzhou/peh003/repository/repos/test1"  */

  if (strcmp(url, sess->repos_url) != 0)

            //first time relpath="test1/one"

         // second time relpath="test1”

            relpath = svn_uri_is_child(sess->repos_url, url, pool);

  if (! relpath)

           …

  relpath = apr_pstrcat(pool, "/", svn_path_uri_decode(relpath, pool),

                        (char *)NULL);

  /*sess->fs_path->data isset to the correct value "test1" in the second call*/

  svn_stringbuf_set(sess->fs_path, relpath);

  return SVN_NO_ERROR;

}

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

How the error value get corrected

svn_error_t *svn_ra_reparent(svn_ra_session_t *session,

                             const char *url,

                             apr_pool_t *pool)

{

  /*url=“file:///home/yyzhou/peh003/repository/repos”, i.e. repository’s root url*/

  const char *repos_root;

  /* Make sure the new URL is in the same repository, so that the

     implementations don't have to do it. */

  SVN_ERR(svn_ra_get_repos_root2(session, &repos_root, pool));

  /*repos_root==url*/

  if (! svn_uri_is_ancestor(repos_root, url))

        ...

  return session->vtable->reparent(session, url, pool);

}

static svn_error_t *

svn_ra_local__reparent(svn_ra_session_t *session,

                       const char *url,

                       apr_pool_t *pool)

{

  svn_ra_local__session_baton_t *sess = session->priv;

  const char *relpath = "";

  /*url ==  sess->repos_url = "file:///home/yyzhou/peh003/repository/repos" */

  if (strcmp(url, sess->repos_url) != 0)

    relpath = svn_uri_is_child(sess->repos_url, url, pool);

  if (! relpath)

           …

  relpath = apr_pstrcat(pool, "/", svn_path_uri_decode(relpath, pool),

                        (char *)NULL); //relpath = "/"

  /*sess->fs_path->data is at last set to the correct value "/"*/

  svn_stringbuf_set(sess->fs_path, relpath);

  return SVN_NO_ERROR;

}

Failure symptom category

refuse valid input/early termination

Is there any log message?

Yes

How can ErrLog automatically insert the log message?

return value check