[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Fix for issue 3813

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Wed, 22 Jun 2011 09:29:29 +0300

Noorul Islam K M wrote on Wed, Jun 22, 2011 at 11:37:13 +0530:
>
> I am not really sure whether this patch is correct one since the comment
> in diff_editor.c says this
>
> /* This is the file that will contain the pristine repository version. It
> is created in the admin temporary area. This file continues to exists
> until after the diff callback is run, at which point it is deleted. */
>
> I can't think of the drawbacks of creating this particular temp file in
> /tmp.
>

From looking at the code, svn_io_open_unique_file3() would force the
file to have a mode of 0600|umask() instead of just 0600; i.e., the
contents of the files being diffed would leak, even if the wc root dir
was mode 0700 (in which case they don't currently leak).

Is that a problem?

(The comment in svn_io_open_unique_file3() indicates that there's some
historical reason, catering to specific API consumers, for extending the
permissions of the tempfile.)

> Log
>
> [[[
> Fix for issue 3813.
>
> * subversion/libsvn_wc/diff_editor.c
> (apply_textdelta): Use the default temporary directory instead of
> admin temporary area to create temporary file.
>
> Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
> ]]]
>
> Thanks and Regards
> Noorul
>

> Index: subversion/libsvn_wc/diff_editor.c
> ===================================================================
> --- subversion/libsvn_wc/diff_editor.c (revision 1137953)
> +++ subversion/libsvn_wc/diff_editor.c (working copy)
> @@ -1518,7 +1518,6 @@
> struct file_baton *fb = file_baton;
> struct window_handler_baton *whb;
> struct edit_baton *eb = fb->eb;
> - const char *temp_dir;
> svn_stream_t *source;
> svn_stream_t *temp_stream;
>
> @@ -1532,13 +1531,8 @@
>
>
>
> - /* This is the file that will contain the pristine repository version. It
> - is created in the admin temporary area. This file continues to exists
> - until after the diff callback is run, at which point it is deleted. */
> - SVN_ERR(svn_wc__db_temp_wcroot_tempdir(&temp_dir, eb->db, fb->local_abspath,
> - pool, pool));
> SVN_ERR(svn_stream_open_unique(&temp_stream, &fb->temp_file_path,
> - temp_dir, svn_io_file_del_on_pool_cleanup,
> + NULL, svn_io_file_del_on_pool_cleanup,
> fb->pool, fb->pool));
>
> whb = apr_pcalloc(fb->pool, sizeof(*whb));
Received on 2011-06-22 08:30:22 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.