Re: svn commit: r40349 - trunk/subversion/libsvn_subr

[Paul Burba-3]

On Sun, Nov 1, 2009 at 6:02 PM, Stefan Sperling <stsp@...> wrote:

> Author: stsp
> Date: Sun Nov  1 15:02:09 2009
> New Revision: 40349
>
> Log:
> * subversion/libsvn_subr/io.c
>  (svn_io_open_unique_file3): Make permissions of temporary files created
>   by svn_io_file_mktemp() less restrictive if the umask allows this.
>   Fixes permission problems related to temporary files with restrictive
>   permissions being copied or renamed into the working copy.
>
> Modified:
>   trunk/subversion/libsvn_subr/io.c
>
> Modified: trunk/subversion/libsvn_subr/io.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_subr/io.c?pathrev=40349&r1=40348&r2=40349
> ==============================================================================
> --- trunk/subversion/libsvn_subr/io.c   Sun Nov  1 14:53:12 2009        (r40348)
> +++ trunk/subversion/libsvn_subr/io.c   Sun Nov  1 15:02:09 2009        (r40349)
> @@ -3667,6 +3667,7 @@ svn_io_open_unique_file3(apr_file_t **fi
>  {
>   apr_file_t *tempfile;
>   const char *tempname;
> +  apr_fileperms_t perms;
>   char *path;
>   struct temp_file_cleanup_s *baton = NULL;
>   apr_int32_t flags = (APR_READ | APR_WRITE | APR_CREATE | APR_EXCL |
> @@ -3710,6 +3711,18 @@ svn_io_open_unique_file3(apr_file_t **fi
>   SVN_ERR(svn_io_file_mktemp(&tempfile, path, flags, result_pool));
>   SVN_ERR(svn_io_file_name_get(&tempname, tempfile, scratch_pool));
>
> +  /* ### svn_io_file_mktemp() creates files with mode 0600.
> +   * ### As of r40264, tempfiles created by svn_io_open_unique_file3()
> +   * ### often end up being copied or renamed into the working copy.
> +   * ### This will cause working files having mode 0600 while users might
> +   * ### expect to see 644 or 664. Ideally, permissions should be tweaked
> +   * ### by our callers after installing tempfiles in the WC, but until
> +   * ### that's done we need to avoid breaking pre-r40264 behaviour.
> +   * ### So we tweak perms of the tempfile here, but only if the umask
> +   * ### allows it. */
> +  SVN_ERR(merge_default_file_perms(tempfile, &perms, scratch_pool));
> +  SVN_ERR(svn_io_file_perms_set(tempname, perms, scratch_pool));
> +

Stefan,

This should be blocked in an #ifndef WIN32 yes?
merge_default_file_perms isn't defined on Windows, so this breaks the
build.

Paul

>   if (file)
>     *file = tempfile;
>   else
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageId=2413540
>

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2413817

[Stefan Sperling-7]

On Mon, Nov 02, 2009 at 09:48:09AM -0500, Paul Burba wrote:

> On Sun, Nov 1, 2009 at 6:02 PM, Stefan Sperling <stsp@...> wrote:
> > Author: stsp
> > Date: Sun Nov  1 15:02:09 2009
> > New Revision: 40349
> >
> > Log:
> > * subversion/libsvn_subr/io.c
> >  (svn_io_open_unique_file3): Make permissions of temporary files created
> >   by svn_io_file_mktemp() less restrictive if the umask allows this.
> >   Fixes permission problems related to temporary files with restrictive
> >   permissions being copied or renamed into the working copy.
> >
> > Modified:
> >   trunk/subversion/libsvn_subr/io.c
> >
> > Modified: trunk/subversion/libsvn_subr/io.c
> > URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_subr/io.c?pathrev=40349&r1=40348&r2=40349
> > ==============================================================================
> > --- trunk/subversion/libsvn_subr/io.c   Sun Nov  1 14:53:12 2009        (r40348)
> > +++ trunk/subversion/libsvn_subr/io.c   Sun Nov  1 15:02:09 2009        (r40349)
> > @@ -3667,6 +3667,7 @@ svn_io_open_unique_file3(apr_file_t **fi
> >  {
> >   apr_file_t *tempfile;
> >   const char *tempname;
> > +  apr_fileperms_t perms;
> >   char *path;
> >   struct temp_file_cleanup_s *baton = NULL;
> >   apr_int32_t flags = (APR_READ | APR_WRITE | APR_CREATE | APR_EXCL |
> > @@ -3710,6 +3711,18 @@ svn_io_open_unique_file3(apr_file_t **fi
> >   SVN_ERR(svn_io_file_mktemp(&tempfile, path, flags, result_pool));
> >   SVN_ERR(svn_io_file_name_get(&tempname, tempfile, scratch_pool));
> >
> > +  /* ### svn_io_file_mktemp() creates files with mode 0600.
> > +   * ### As of r40264, tempfiles created by svn_io_open_unique_file3()
> > +   * ### often end up being copied or renamed into the working copy.
> > +   * ### This will cause working files having mode 0600 while users might
> > +   * ### expect to see 644 or 664. Ideally, permissions should be tweaked
> > +   * ### by our callers after installing tempfiles in the WC, but until
> > +   * ### that's done we need to avoid breaking pre-r40264 behaviour.
> > +   * ### So we tweak perms of the tempfile here, but only if the umask
> > +   * ### allows it. */
> > +  SVN_ERR(merge_default_file_perms(tempfile, &perms, scratch_pool));
> > +  SVN_ERR(svn_io_file_perms_set(tempname, perms, scratch_pool));
> > +
>
> Stefan,
>
> This should be blocked in an #ifndef WIN32 yes?

Yes! Fixed in r40353.

> merge_default_file_perms isn't defined on Windows, so this breaks the
> build.

Sorry about that. Please go ahead fix (or in extreme cases, revert)
commits that break the build.

Stefan

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2413867