subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From gst...@apache.org
Subject svn commit: r1306111 - in /subversion/trunk/subversion: libsvn_ra_svn/client.c svnserve/serve.c
Date Wed, 28 Mar 2012 01:51:54 GMT
Author: gstein
Date: Wed Mar 28 01:51:54 2012
New Revision: 1306111

URL: http://svn.apache.org/viewvc?rev=1306111&view=rev
Log:
Fix a problem in the svn protocol's get_dir() functionality. The tuple
specification allowed for a NULL created-date, but the client would
segfault with that input (via svn_time_from_string).

This is related to the work in r1296628 and r1305964. I believe this
"happened to work" until r1296628. Before that revision, the value
((time_t) -1) was stored into svn_dirent_t.time which is an apr_time_t
(a 64-bit value, compared to time_t's typical 32 bits). This likely
caused a later check to think a time was present, and for a time
string to be generated (thus: never NULL).

So... when a created-date is missing, we fill in the epoch date.
Hopefully, the client will translate this back to 0, and consider the
date "missing". As a precaution, the client will also check for NULL
to avoid malicious servers passing NULL in the tuple.

Note: I took a brief look at the 1.6.x client, and this failure is
present. Thus, we must pass non-NULL for compatibility purposes, to
avoid older clients from segfaulting.

* subversion/libsvn_ra_svn/client.c:
  (ra_svn_get_dir): beware a NULL CDATE value. add commentary.

* subversion/svnserve/serve.c:
  (get_dir): tighten the scope of a bunch of variables for clarity.
    use actual variables rather than a structure (ie. nix ENTRY). use
    the new MISSING_DATE localvar when created-date is missing. leave
    commentary for future readers.

Modified:
    subversion/trunk/subversion/libsvn_ra_svn/client.c
    subversion/trunk/subversion/svnserve/serve.c

Modified: subversion/trunk/subversion/libsvn_ra_svn/client.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_svn/client.c?rev=1306111&r1=1306110&r2=1306111&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_svn/client.c (original)
+++ subversion/trunk/subversion/libsvn_ra_svn/client.c Wed Mar 28 01:51:54 2012
@@ -1152,7 +1152,18 @@ static svn_error_t *ra_svn_get_dir(svn_r
       dirent->size = size;/* FIXME: svn_filesize_t */
       dirent->has_props = has_props;
       dirent->created_rev = crev;
-      SVN_ERR(svn_time_from_cstring(&dirent->time, cdate, pool));
+      /* NOTE: the tuple's format string says CDATE may be NULL. But this
+         function does not allow that. The server has always sent us some
+         random date, however, so this just happens to work. But let's
+         be wary of servers that are (improperly) fixed to send NULL.
+
+         Note: they should NOT be "fixed" to send NULL, as that would break
+         any older clients which received that NULL. But we may as well
+         be defensive against a malicous server.  */
+      if (cdate == NULL)
+        dirent->time = 0;
+      else
+        SVN_ERR(svn_time_from_cstring(&dirent->time, cdate, pool));
       dirent->last_author = cauthor;
       apr_hash_set(*dirents, name, APR_HASH_KEY_STRING, dirent);
     }

Modified: subversion/trunk/subversion/svnserve/serve.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnserve/serve.c?rev=1306111&r1=1306110&r2=1306111&view=diff
==============================================================================
--- subversion/trunk/subversion/svnserve/serve.c (original)
+++ subversion/trunk/subversion/svnserve/serve.c Wed Mar 28 01:51:54 2012
@@ -1470,9 +1470,9 @@ static svn_error_t *get_dir(svn_ra_svn_c
                             apr_array_header_t *params, void *baton)
 {
   server_baton_t *b = baton;
-  const char *path, *full_path, *file_path, *cdate;
+  const char *path, *full_path;
   svn_revnum_t rev;
-  apr_hash_t *entries, *props = NULL, *file_props;
+  apr_hash_t *entries, *props = NULL;
   apr_hash_index_t *hi;
   svn_fs_root_t *root;
   apr_pool_t *subpool;
@@ -1548,6 +1548,9 @@ static svn_error_t *get_dir(svn_ra_svn_c
   /* Fetch the directory entries if requested and send them immediately. */
   if (want_contents)
     {
+      /* Use epoch for a placeholder for a missing date.  */
+      const char *missing_date = svn_time_to_cstring(0, pool);
+
       SVN_CMD_ERR(svn_fs_dir_entries(&entries, root, full_path, pool));
 
       /* Transform the hash table's FS entries into dirents.  This probably
@@ -1557,9 +1560,15 @@ static svn_error_t *get_dir(svn_ra_svn_c
         {
           const char *name = svn__apr_hash_index_key(hi);
           svn_fs_dirent_t *fsent = svn__apr_hash_index_val(hi);
+          const char *file_path;
 
-          svn_dirent_t entry;
-          memset(&entry, 0, sizeof(entry));
+          /* The fields in the entry tuple.  */
+          svn_node_kind_t entry_kind = svn_node_none;
+          svn_filesize_t entry_size = 0;
+          svn_boolean_t has_props = FALSE;
+          svn_revnum_t created_rev = 0; /* ### SVN_INVALID_REVNUM  */
+          const char *cdate = NULL;
+          const char *last_author = NULL;
 
           svn_pool_clear(subpool);
 
@@ -1569,41 +1578,49 @@ static svn_error_t *get_dir(svn_ra_svn_c
             continue;
 
           if (dirent_fields & SVN_DIRENT_KIND)
-              entry.kind = fsent->kind;
+              entry_kind = fsent->kind;
 
           if (dirent_fields & SVN_DIRENT_SIZE)
-              if (entry.kind != svn_node_dir)
-                SVN_CMD_ERR(svn_fs_file_length(&entry.size, root, file_path,
+              if (entry_kind != svn_node_dir)
+                SVN_CMD_ERR(svn_fs_file_length(&entry_size, root, file_path,
                                                subpool));
 
           if (dirent_fields & SVN_DIRENT_HAS_PROPS)
             {
+              apr_hash_t *file_props;
+
               /* has_props */
               SVN_CMD_ERR(svn_fs_node_proplist(&file_props, root, file_path,
                                                subpool));
-              entry.has_props = (apr_hash_count(file_props) > 0);
+              has_props = (apr_hash_count(file_props) > 0);
             }
 
-          cdate = NULL;
           if ((dirent_fields & SVN_DIRENT_LAST_AUTHOR)
               || (dirent_fields & SVN_DIRENT_TIME)
               || (dirent_fields & SVN_DIRENT_CREATED_REV))
             {
               /* created_rev, last_author, time */
-              SVN_CMD_ERR(svn_repos_get_committed_info(&entry.created_rev,
+              SVN_CMD_ERR(svn_repos_get_committed_info(&created_rev,
                                                        &cdate,
-                                                       &entry.last_author, 
+                                                       &last_author, 
                                                        root,
                                                        file_path,
                                                        subpool));
             }
 
+          /* The client does not properly handle a missing CDATE. For
+             interoperability purposes, we must fill in some junk.
+
+             See libsvn_ra_svn/client.c:ra_svn_get_dir()  */
+          if (cdate == NULL)
+            cdate = missing_date;
+
           /* Send the entry. */
           SVN_ERR(svn_ra_svn_write_tuple(conn, pool, "cwnbr(?c)(?c)", name,
-                                         svn_node_kind_to_word(entry.kind),
-                                         (apr_uint64_t) entry.size,
-                                         entry.has_props, entry.created_rev,
-                                         cdate, entry.last_author));
+                                         svn_node_kind_to_word(entry_kind),
+                                         (apr_uint64_t) entry_size,
+                                         has_props, created_rev,
+                                         cdate, last_author));
         }
       svn_pool_destroy(subpool);
     }



Mime
View raw message