[PATCH] Allow clients to check out special files with unknown type

Hi Peter,

So this is the patch I'm planning to commit, updated based on your
review. Any further comments?

We can cover EPERM handling in an followup commit, if we decide that's
appropriate.

[[[
Fix part of issue 2692: allow clients to checkout unknown special file
types as regular files with the svn:special property set (like symlinks
on Windows), instead of treating an unknown special file type as a fatal
error (which, since we don't validate the type on commit, allows one
client to DoS all the others).

This incidentally also fixes special_tests test 8, since the error there
is caused by the same problem.

Review by: lundblad

* subversion/libsvn_subr/subst.c
  (create_special_file_from_stringbuf): Create a regular file containing
    the special file's internal representation if the special file type
    was unknown, re-using the code for creating 'symlinks' on Windows
    otherwise.

* subversion/tests/cmdline/special_tests.py
  (checkout_repo_with_unknown_special_type): New. Check that we can
    check out a repository containing a special file of unknown type.
  (test_list): Unmark merge_file_into_symlink as XFail, and add
    checkout_repo_with_unknown_special_type to test the issue directly.

* subversion/tests/cmdline/special_tests_data/bad-special-type.dump
  New. Dumpfile for checkout_repo_with_unknown_special_type.
]]]

Regards,
Malcolm

Index: subversion/libsvn_subr/subst.c
===================================================================
--- subversion/libsvn_subr/subst.c (revision 23417)
+++ subversion/libsvn_subr/subst.c (working copy)
@@ -1568,13 +1568,13 @@ static svn_error_t *
 create_special_file_from_stringbuf(svn_stringbuf_t *src, const char *dst,
                                    apr_pool_t *pool)
 {
- svn_error_t *err;
   char *identifier, *remainder;
   const char *dst_tmp;
+ svn_boolean_t create_using_internal_representation = FALSE;
 
   /* Separate off the identifier. The first space character delimits
      the identifier, after which any remaining characters are specific
- to the actual special device being created. */
+ to the actual special file type being created. */
   identifier = src->data;
   for (remainder = identifier; *remainder; remainder++)
     {
@@ -1590,38 +1590,45 @@ create_special_file_from_stringbuf(svn_s
     {
       /* For symlinks, the type specific data is just a filesystem
          path that the symlink should reference. */
- err = svn_io_create_unique_link(&dst_tmp, dst, remainder,
- ".tmp", pool);
+ svn_error_t *err = svn_io_create_unique_link(&dst_tmp, dst, remainder,
+ ".tmp", pool);
+
+ /* If we had an error, check to see if it was because symlinks are
+ not supported on the platform. If so, fall back
+ to using the internal representation. */
+ if (err)
+ {
+ if (err->apr_err == SVN_ERR_UNSUPPORTED_FEATURE)
+ {
+ svn_error_clear(err);
+ create_using_internal_representation = TRUE;
+ }
+ else
+ return err;
+ }
     }
   else
     {
- /* We should return a valid error here. */
- return svn_error_createf(SVN_ERR_UNSUPPORTED_FEATURE, NULL,
- _("'%s' has unsupported special file type "
- "'%s'"), src->data, identifier);
+ /* Just create a normal file using the internal special file
+ representation. We don't want a commit of an unknown special
+ file type to DoS all the clients. */
+ create_using_internal_representation = TRUE;
     }
 
- /* If we had an error, check to see if it was because this type of
- special device is not supported. */
- if (err)
+ /* If nothing else worked, write out the internal representation to
+ a file that can be edited by the user. */
+ if (create_using_internal_representation)
     {
- if (err->apr_err == SVN_ERR_UNSUPPORTED_FEATURE)
- {
- apr_file_t *dst_tmp_file;
- apr_size_t written;
+ apr_file_t *dst_tmp_file;
+ apr_size_t written;
 
- svn_error_clear(err);
 
- /* Fall back to just copying the text-base. */
- SVN_ERR(svn_io_open_unique_file2(&dst_tmp_file, &dst_tmp,
- dst, ".tmp", svn_io_file_del_none,
- pool));
- SVN_ERR(svn_io_file_write_full(dst_tmp_file, src->data, src->len,
- &written, pool));
- SVN_ERR(svn_io_file_close(dst_tmp_file, pool));
- }
- else
- return err;
+ SVN_ERR(svn_io_open_unique_file2(&dst_tmp_file, &dst_tmp,
+ dst, ".tmp", svn_io_file_del_none,
+ pool));
+ SVN_ERR(svn_io_file_write_full(dst_tmp_file, src->data, src->len,
+ &written, pool));
+ SVN_ERR(svn_io_file_close(dst_tmp_file, pool));
     }
 
   /* Do the atomic rename from our temporary location. */
Index: subversion/tests/cmdline/special_tests_data/bad-special-type.dump
===================================================================
--- subversion/tests/cmdline/special_tests_data/bad-special-type.dump (revision 0)
+++ subversion/tests/cmdline/special_tests_data/bad-special-type.dump (revision 0)
@@ -0,0 +1,47 @@
+SVN-fs-dump-format-version: 2
+
+UUID: 51ec0b24-bd07-11db-97f6-8dc527e3df93
+
+Revision-number: 0
+Prop-content-length: 56
+Content-length: 56
+
+K 8
+svn:date
+V 27
+2007-02-15T15:16:13.268177Z
+PROPS-END
+
+Revision-number: 1
+Prop-content-length: 143
+Content-length: 143
+
+K 7
+svn:log
+V 41
+Commit a special file with a unknown type
+K 10
+svn:author
+V 7
+malcolm
+K 8
+svn:date
+V 27
+2007-02-15T15:18:08.532835Z
+PROPS-END
+
+Node-path: special
+Node-kind: file
+Node-action: add
+Prop-content-length: 33
+Text-content-length: 11
+Text-content-md5: 12dfeb21c0626cc041b5de87de30c661
+Content-length: 44
+
+K 11
+svn:special
+V 1
+*
+PROPS-END
+gimble wabe
+
Index: subversion/tests/cmdline/special_tests.py
===================================================================
--- subversion/tests/cmdline/special_tests.py (revision 23417)
+++ subversion/tests/cmdline/special_tests.py (working copy)
@@ -531,6 +531,33 @@ def diff_symlink_to_dir(sbox):
   svntest.actions.run_and_verify_svn(None, expected_output, [], 'diff',
                                      link_path)
 
+# Issue 2692 (part of): Check that the client can check out a repository
+# that contains an unknown special file type.
+def checkout_repo_with_unknown_special_type(sbox):
+ "checkout repository with unknown special file type"
+
+ # Create virgin repos and working copy
+ svntest.main.safe_rmtree(sbox.repo_dir, 1)
+ svntest.main.create_repos(sbox.repo_dir)
+
+ # Load the dumpfile into the repos.
+ data_dir = os.path.join(os.path.dirname(sys.argv[0]),
+ 'special_tests_data')
+ dump_str = file(os.path.join(data_dir,
+ "bad-special-type.dump"), "rb").read()
+ svntest.actions.run_and_verify_load(sbox.repo_dir, dump_str)
+
+ expected_output = svntest.wc.State(sbox.wc_dir, {
+ 'special': Item(status='A '),
+ })
+ expected_wc = svntest.wc.State('', {
+ 'special' : Item(contents='gimble wabe'),
+ })
+ svntest.actions.run_and_verify_checkout(sbox.repo_url,
+ sbox.wc_dir,
+ expected_output,
+ expected_wc)
+
 
 ########################################################################
 # Run the tests
@@ -545,9 +572,10 @@ test_list = [ None,
               Skip(replace_symlink_with_file, (os.name != 'posix')),
               Skip(remove_symlink, (os.name != 'posix')),
               Skip(merge_symlink_into_file, (os.name != 'posix')),
- XFail(Skip(merge_file_into_symlink, (os.name != 'posix'))),
+ Skip(merge_file_into_symlink, (os.name != 'posix')),
               checkout_repo_with_symlinks,
               XFail(Skip(diff_symlink_to_dir, (os.name != 'posix'))),
+ checkout_repo_with_unknown_special_type,
              ]
 
 if __name__ == '__main__':