You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by iv...@apache.org on 2011/07/18 15:58:32 UTC

svn commit: r1147882 - /subversion/trunk/subversion/libsvn_repos/hooks.c

Author: ivan
Date: Mon Jul 18 13:58:31 2011
New Revision: 1147882

URL: http://svn.apache.org/viewvc?rev=1147882&view=rev
Log:
Convert lock token returned from pre-lock hook from native encoding to UTF-8.

* subversion/libsvn_repos/hooks.c
  (svn_repos__hooks_pre_lock): Convert hook output from native encoding to 
   UTF-8.

Modified:
    subversion/trunk/subversion/libsvn_repos/hooks.c

Modified: subversion/trunk/subversion/libsvn_repos/hooks.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/hooks.c?rev=1147882&r1=1147881&r2=1147882&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_repos/hooks.c (original)
+++ subversion/trunk/subversion/libsvn_repos/hooks.c Mon Jul 18 13:58:31 2011
@@ -616,8 +616,20 @@ svn_repos__hooks_pre_lock(svn_repos_t *r
 
       SVN_ERR(run_hook_cmd(&buf, SVN_REPOS__HOOK_PRE_LOCK, hook, args, NULL,
                            pool));
+
       if (token)
-        *token = buf->data;
+        {
+          svn_error_t *err;
+          /* Convert hook output from native encoding to UTF-8. */
+          err = svn_utf_cstring_to_utf8(token, buf->data, pool);
+          if (err)
+            {
+              return svn_error_create(SVN_ERR_REPOS_HOOK_FAILURE, err,
+                                      _("Output of pre-lock hook could not be "
+                                        "translated from the native locale to "
+                                        "UTF-8."));
+            }
+        }
     }
   else if (token)
     *token = "";



Re: svn commit: r1147882 - /subversion/trunk/subversion/libsvn_repos/hooks.c

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
The questions I see are:

* is there a bug in ra_dav's handling of some non-ASCI lock tokens?

* is there a missing input validation?  If so, what validation, and does
  it belong in _repos or _fs?

Daniel Shahaf wrote on Mon, Jul 18, 2011 at 22:50:40 +0300:
> Ivan notes on IRC that this fixes issues with non-ASCII lock tokens.
> (At least, he reports errors in 'svn ls -v' over http://; but for me 
> 'ls' and 'info' work fine over svn://.)
> 
> Are lock tokens even permitted to be non-ASCII?
> (both backends generate ASCII-only lock tokens (in the same manner))
> 
> If they are, are they required to be in UTF-8?  Or can they be, say,
> arbitrary octet strings?
> 
> 
> Once we know the answers to these questions we can determine whether
> this patch is correct in its handling of non-ASCII lock tokens and
> whether there's a separate ra_dav bug that needs to be fixed.
> 
> 
> ivan@apache.org wrote on Mon, Jul 18, 2011 at 13:58:32 -0000:
> > Author: ivan
> > Date: Mon Jul 18 13:58:31 2011
> > New Revision: 1147882
> > 
> > URL: http://svn.apache.org/viewvc?rev=1147882&view=rev
> > Log:
> > Convert lock token returned from pre-lock hook from native encoding to UTF-8.
> > 
> > * subversion/libsvn_repos/hooks.c
> >   (svn_repos__hooks_pre_lock): Convert hook output from native encoding to 
> >    UTF-8.
> > 
> > Modified:
> >     subversion/trunk/subversion/libsvn_repos/hooks.c
> > 
> > Modified: subversion/trunk/subversion/libsvn_repos/hooks.c
> > URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/hooks.c?rev=1147882&r1=1147881&r2=1147882&view=diff
> > ==============================================================================
> > --- subversion/trunk/subversion/libsvn_repos/hooks.c (original)
> > +++ subversion/trunk/subversion/libsvn_repos/hooks.c Mon Jul 18 13:58:31 2011
> > @@ -616,8 +616,20 @@ svn_repos__hooks_pre_lock(svn_repos_t *r
> >  
> >        SVN_ERR(run_hook_cmd(&buf, SVN_REPOS__HOOK_PRE_LOCK, hook, args, NULL,
> >                             pool));
> > +
> >        if (token)
> > -        *token = buf->data;
> > +        {
> > +          svn_error_t *err;
> > +          /* Convert hook output from native encoding to UTF-8. */
> > +          err = svn_utf_cstring_to_utf8(token, buf->data, pool);
> > +          if (err)
> > +            {
> > +              return svn_error_create(SVN_ERR_REPOS_HOOK_FAILURE, err,
> > +                                      _("Output of pre-lock hook could not be "
> > +                                        "translated from the native locale to "
> > +                                        "UTF-8."));
> > +            }
> > +        }
> >      }
> >    else if (token)
> >      *token = "";
> > 
> > 

Re: svn commit: r1147882 - /subversion/trunk/subversion/libsvn_repos/hooks.c

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
The questions I see are:

* is there a bug in ra_dav's handling of some non-ASCI lock tokens?

* is there a missing input validation?  If so, what validation, and does
  it belong in _repos or _fs?

Daniel Shahaf wrote on Mon, Jul 18, 2011 at 22:50:40 +0300:
> Ivan notes on IRC that this fixes issues with non-ASCII lock tokens.
> (At least, he reports errors in 'svn ls -v' over http://; but for me 
> 'ls' and 'info' work fine over svn://.)
> 
> Are lock tokens even permitted to be non-ASCII?
> (both backends generate ASCII-only lock tokens (in the same manner))
> 
> If they are, are they required to be in UTF-8?  Or can they be, say,
> arbitrary octet strings?
> 
> 
> Once we know the answers to these questions we can determine whether
> this patch is correct in its handling of non-ASCII lock tokens and
> whether there's a separate ra_dav bug that needs to be fixed.
> 
> 
> ivan@apache.org wrote on Mon, Jul 18, 2011 at 13:58:32 -0000:
> > Author: ivan
> > Date: Mon Jul 18 13:58:31 2011
> > New Revision: 1147882
> > 
> > URL: http://svn.apache.org/viewvc?rev=1147882&view=rev
> > Log:
> > Convert lock token returned from pre-lock hook from native encoding to UTF-8.
> > 
> > * subversion/libsvn_repos/hooks.c
> >   (svn_repos__hooks_pre_lock): Convert hook output from native encoding to 
> >    UTF-8.
> > 
> > Modified:
> >     subversion/trunk/subversion/libsvn_repos/hooks.c
> > 
> > Modified: subversion/trunk/subversion/libsvn_repos/hooks.c
> > URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/hooks.c?rev=1147882&r1=1147881&r2=1147882&view=diff
> > ==============================================================================
> > --- subversion/trunk/subversion/libsvn_repos/hooks.c (original)
> > +++ subversion/trunk/subversion/libsvn_repos/hooks.c Mon Jul 18 13:58:31 2011
> > @@ -616,8 +616,20 @@ svn_repos__hooks_pre_lock(svn_repos_t *r
> >  
> >        SVN_ERR(run_hook_cmd(&buf, SVN_REPOS__HOOK_PRE_LOCK, hook, args, NULL,
> >                             pool));
> > +
> >        if (token)
> > -        *token = buf->data;
> > +        {
> > +          svn_error_t *err;
> > +          /* Convert hook output from native encoding to UTF-8. */
> > +          err = svn_utf_cstring_to_utf8(token, buf->data, pool);
> > +          if (err)
> > +            {
> > +              return svn_error_create(SVN_ERR_REPOS_HOOK_FAILURE, err,
> > +                                      _("Output of pre-lock hook could not be "
> > +                                        "translated from the native locale to "
> > +                                        "UTF-8."));
> > +            }
> > +        }
> >      }
> >    else if (token)
> >      *token = "";
> > 
> > 

Re: svn commit: r1147882 - /subversion/trunk/subversion/libsvn_repos/hooks.c

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Ivan notes on IRC that this fixes issues with non-ASCII lock tokens.
(At least, he reports errors in 'svn ls -v' over http://; but for me 
'ls' and 'info' work fine over svn://.)

Are lock tokens even permitted to be non-ASCII?
(both backends generate ASCII-only lock tokens (in the same manner))

If they are, are they required to be in UTF-8?  Or can they be, say,
arbitrary octet strings?


Once we know the answers to these questions we can determine whether
this patch is correct in its handling of non-ASCII lock tokens and
whether there's a separate ra_dav bug that needs to be fixed.


ivan@apache.org wrote on Mon, Jul 18, 2011 at 13:58:32 -0000:
> Author: ivan
> Date: Mon Jul 18 13:58:31 2011
> New Revision: 1147882
> 
> URL: http://svn.apache.org/viewvc?rev=1147882&view=rev
> Log:
> Convert lock token returned from pre-lock hook from native encoding to UTF-8.
> 
> * subversion/libsvn_repos/hooks.c
>   (svn_repos__hooks_pre_lock): Convert hook output from native encoding to 
>    UTF-8.
> 
> Modified:
>     subversion/trunk/subversion/libsvn_repos/hooks.c
> 
> Modified: subversion/trunk/subversion/libsvn_repos/hooks.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/hooks.c?rev=1147882&r1=1147881&r2=1147882&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_repos/hooks.c (original)
> +++ subversion/trunk/subversion/libsvn_repos/hooks.c Mon Jul 18 13:58:31 2011
> @@ -616,8 +616,20 @@ svn_repos__hooks_pre_lock(svn_repos_t *r
>  
>        SVN_ERR(run_hook_cmd(&buf, SVN_REPOS__HOOK_PRE_LOCK, hook, args, NULL,
>                             pool));
> +
>        if (token)
> -        *token = buf->data;
> +        {
> +          svn_error_t *err;
> +          /* Convert hook output from native encoding to UTF-8. */
> +          err = svn_utf_cstring_to_utf8(token, buf->data, pool);
> +          if (err)
> +            {
> +              return svn_error_create(SVN_ERR_REPOS_HOOK_FAILURE, err,
> +                                      _("Output of pre-lock hook could not be "
> +                                        "translated from the native locale to "
> +                                        "UTF-8."));
> +            }
> +        }
>      }
>    else if (token)
>      *token = "";
> 
> 

Re: svn commit: r1147882 - /subversion/trunk/subversion/libsvn_repos/hooks.c

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Ivan notes on IRC that this fixes issues with non-ASCII lock tokens.
(At least, he reports errors in 'svn ls -v' over http://; but for me 
'ls' and 'info' work fine over svn://.)

Are lock tokens even permitted to be non-ASCII?
(both backends generate ASCII-only lock tokens (in the same manner))

If they are, are they required to be in UTF-8?  Or can they be, say,
arbitrary octet strings?


Once we know the answers to these questions we can determine whether
this patch is correct in its handling of non-ASCII lock tokens and
whether there's a separate ra_dav bug that needs to be fixed.


ivan@apache.org wrote on Mon, Jul 18, 2011 at 13:58:32 -0000:
> Author: ivan
> Date: Mon Jul 18 13:58:31 2011
> New Revision: 1147882
> 
> URL: http://svn.apache.org/viewvc?rev=1147882&view=rev
> Log:
> Convert lock token returned from pre-lock hook from native encoding to UTF-8.
> 
> * subversion/libsvn_repos/hooks.c
>   (svn_repos__hooks_pre_lock): Convert hook output from native encoding to 
>    UTF-8.
> 
> Modified:
>     subversion/trunk/subversion/libsvn_repos/hooks.c
> 
> Modified: subversion/trunk/subversion/libsvn_repos/hooks.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/hooks.c?rev=1147882&r1=1147881&r2=1147882&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_repos/hooks.c (original)
> +++ subversion/trunk/subversion/libsvn_repos/hooks.c Mon Jul 18 13:58:31 2011
> @@ -616,8 +616,20 @@ svn_repos__hooks_pre_lock(svn_repos_t *r
>  
>        SVN_ERR(run_hook_cmd(&buf, SVN_REPOS__HOOK_PRE_LOCK, hook, args, NULL,
>                             pool));
> +
>        if (token)
> -        *token = buf->data;
> +        {
> +          svn_error_t *err;
> +          /* Convert hook output from native encoding to UTF-8. */
> +          err = svn_utf_cstring_to_utf8(token, buf->data, pool);
> +          if (err)
> +            {
> +              return svn_error_create(SVN_ERR_REPOS_HOOK_FAILURE, err,
> +                                      _("Output of pre-lock hook could not be "
> +                                        "translated from the native locale to "
> +                                        "UTF-8."));
> +            }
> +        }
>      }
>    else if (token)
>      *token = "";
> 
>