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

RE: [PATCH] Fix issue #2068

From: Bert Huijben <rhuijben_at_sharpsvn.net>
Date: Tue, 17 Mar 2009 14:09:17 +0100

> -----Original Message-----
> From: Senthil Kumaran S [mailto:senthil_at_collab.net]
> Sent: Tuesday, March 17, 2009 12:32 PM
> To: David Glasser
> Cc: Bert Huijben; dev_at_subversion.tigris.org; C. Michael Pilato
> Subject: Re: [PATCH] Fix issue #2068
>
> David Glasser wrote:
> > On Thu, Mar 12, 2009 at 3:03 AM, Bert Huijben <rhuijben_at_sharpsvn.net>
> wrote:
> >> Maybe we should use a double semicolon as escape sequence for
> passing a
> >> semicolon. I don't like the idea of breaking all old config files,
> registry
> >> settings, etc..
> >>
> >> This fix only removes the semicolon problem, but it breaks putting |
> >> characters in properties.
> >
> > That's a great suggestion, since in the previous format, any double
> > semicolon was completely redundant.
>
> As per above suggestion, I am sending this updated patch. Let me know
> if it
> needs any change, if not I shall commit it tomorrow.
>
> [[[
> Fix issue #2068.
>
> * subversion/libsvn_client/add.c
> (auto_props_enumerator): Add logic to escape double semicolons.
>
> Suggested by: rhuijben
> ]]]
>
> Thank You.

Looking at the patch,
Index: subversion/libsvn_client/add.c
===================================================================
--- subversion/libsvn_client/add.c (revision 36615)
+++ subversion/libsvn_client/add.c (working copy)
@@ -113,8 +113,17 @@
     {
       int len;
       const char *this_value;
- char *equal_sign = strchr(property, '=');
+ char *equal_sign;
+ char *more_prop;
 
+ if (property[(strlen(property)+1)] == ';')
+ {
+ more_prop = apr_strtok(NULL, ";", &last_token);
+ property = apr_pstrcat(autoprops->pool, property, ";",
+ more_prop, NULL);
+ }
+ equal_sign = strchr(property, '=');
+
       if (equal_sign)
         {
           *equal_sign = '\0';

--------------------

I see a few issues

* It misses an end of the string test (property[(strlen(property)+1)] could
be outside the allocated buffer)
* It doesn't handle multiple escaped semicolons as
"myprop=string;;;;continued;nextprop=q" and
"myprop=string;;and;;string;nextprop=q"

I'm not sure if these issues are all fixable using apr_strtok, as that
function replaces the ';' with a '\0'.

(The first case could be checked by comparing *last_token to '\0', but that
is based on knowledge of the current implementation of apr_strtok())

        Bert

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1340969
Received on 2009-03-17 14:23:23 CET

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