You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by gs...@apache.org on 2010/04/18 23:16:55 UTC

svn commit: r935413 - /subversion/trunk/subversion/libsvn_wc/adm_crawler.c

Author: gstein
Date: Sun Apr 18 21:16:55 2010
New Revision: 935413

URL: http://svn.apache.org/viewvc?rev=935413&view=rev
Log:
Fix potential URI encoding/decoding bug.

* subversion/libsvn_wc/adm_crawler.c:
  (report_revisions_and_depths): wrap a couple comments. when the child is
    stripped off the URI, then decode it before comparison.

Modified:
    subversion/trunk/subversion/libsvn_wc/adm_crawler.c

Modified: subversion/trunk/subversion/libsvn_wc/adm_crawler.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm_crawler.c?rev=935413&r1=935412&r2=935413&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/adm_crawler.c (original)
+++ subversion/trunk/subversion/libsvn_wc/adm_crawler.c Sun Apr 18 21:16:55 2010
@@ -360,8 +360,8 @@ report_revisions_and_depths(svn_wc__db_t
             }
           else
             {
-              /* We want to pull in the excluded target. So, report it as deleted,
-                 and server will respond properly. */
+              /* We want to pull in the excluded target. So, report it as
+                 deleted, and server will respond properly. */
               if (! report_everything)
                 SVN_ERR(reporter->delete_path(report_baton,
                                               this_path, iterpool));
@@ -424,10 +424,10 @@ report_revisions_and_depths(svn_wc__db_t
           else
             missing = TRUE;
 
-          /* If a node is still missing from disk here, we have no way to recreate
-             it locally, so report as missing and move along.  Again, don't bother
-             if we're reporting everything, because the dir is already missing on
-             the server. */
+          /* If a node is still missing from disk here, we have no way to
+             recreate it locally, so report as missing and move along.
+             Again, don't bother if we're reporting everything, because the
+             dir is already missing on the server. */
           if (missing && wrk_kind == svn_wc__db_kind_dir
                && (depth > svn_depth_files || depth == svn_depth_unknown))
             {
@@ -450,7 +450,8 @@ report_revisions_and_depths(svn_wc__db_t
           const char *childname = svn_uri_is_child(dir_repos_relpath,
                                                    this_repos_relpath, NULL);
 
-          if (!childname || strcmp(childname, child) != 0)
+          if (childname == NULL
+              || strcmp(svn_path_uri_decode(childname, iterpool), child) != 0)
             this_switched = TRUE;
           else
             this_switched = FALSE;



Re: svn commit: r935413 - /subversion/trunk/subversion/libsvn_wc/adm_crawler.c

Posted by Greg Stein <gs...@gmail.com>.
On Sun, Apr 18, 2010 at 17:53, Bert Huijben <be...@qqmail.nl> wrote:
>
>
>> -----Original Message-----
>> From: gstein@apache.org [mailto:gstein@apache.org]
>> Sent: zondag 18 april 2010 23:17
>> To: commits@subversion.apache.org
>> Subject: svn commit: r935413 -
>> /subversion/trunk/subversion/libsvn_wc/adm_crawler.c
>>
>> Author: gstein
>> Date: Sun Apr 18 21:16:55 2010
>> New Revision: 935413
>>
>> URL: http://svn.apache.org/viewvc?rev=935413&view=rev
>> Log:
>> Fix potential URI encoding/decoding bug.
>>
>> * subversion/libsvn_wc/adm_crawler.c:
>>   (report_revisions_and_depths): wrap a couple comments. when the child
>> is
>>     stripped off the URI, then decode it before comparison.
>>
>> Modified:
>>     subversion/trunk/subversion/libsvn_wc/adm_crawler.c
>>
>> Modified: subversion/trunk/subversion/libsvn_wc/adm_crawler.c
>> URL:
>> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm_
>> crawler.c?rev=935413&r1=935412&r2=935413&view=diff
>> =======================================================================
>> =======
>> --- subversion/trunk/subversion/libsvn_wc/adm_crawler.c (original)
>> +++ subversion/trunk/subversion/libsvn_wc/adm_crawler.c Sun Apr 18
>> 21:16:55 2010
>> @@ -360,8 +360,8 @@ report_revisions_and_depths(svn_wc__db_t
>>              }
>>            else
>>              {
>> -              /* We want to pull in the excluded target. So, report it
>> as deleted,
>> -                 and server will respond properly. */
>> +              /* We want to pull in the excluded target. So, report it
>> as
>> +                 deleted, and server will respond properly. */
>>                if (! report_everything)
>>                  SVN_ERR(reporter->delete_path(report_baton,
>>                                                this_path, iterpool));
>> @@ -424,10 +424,10 @@ report_revisions_and_depths(svn_wc__db_t
>>            else
>>              missing = TRUE;
>>
>> -          /* If a node is still missing from disk here, we have no way
>> to recreate
>> -             it locally, so report as missing and move along.  Again,
>> don't bother
>> -             if we're reporting everything, because the dir is already
>> missing on
>> -             the server. */
>> +          /* If a node is still missing from disk here, we have no way
>> to
>> +             recreate it locally, so report as missing and move along.
>> +             Again, don't bother if we're reporting everything,
>> because the
>> +             dir is already missing on the server. */
>>            if (missing && wrk_kind == svn_wc__db_kind_dir
>>                 && (depth > svn_depth_files || depth ==
>> svn_depth_unknown))
>>              {
>> @@ -450,7 +450,8 @@ report_revisions_and_depths(svn_wc__db_t
>>            const char *childname = svn_uri_is_child(dir_repos_relpath,
>>                                                     this_repos_relpath,
>> NULL);
>
> s/svn_uri/svn_relpath/
>>
>> -          if (!childname || strcmp(childname, child) != 0)
>> +          if (childname == NULL
>> +              || strcmp(svn_path_uri_decode(childname, iterpool),
>> child) != 0)
>
> A repos relpath is already uri_decoded, so this would break paths that have a literal %20 somewhere. (It would be correct if it was a real uri instead of a relpath)

Thanks. I didn't notice that the svn_uri_is_child function was taking
relpaths. Just that its result was not being decoded...

Fixed in r937748

Cheers,
-g

RE: svn commit: r935413 - /subversion/trunk/subversion/libsvn_wc/adm_crawler.c

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: gstein@apache.org [mailto:gstein@apache.org]
> Sent: zondag 18 april 2010 23:17
> To: commits@subversion.apache.org
> Subject: svn commit: r935413 -
> /subversion/trunk/subversion/libsvn_wc/adm_crawler.c
> 
> Author: gstein
> Date: Sun Apr 18 21:16:55 2010
> New Revision: 935413
> 
> URL: http://svn.apache.org/viewvc?rev=935413&view=rev
> Log:
> Fix potential URI encoding/decoding bug.
> 
> * subversion/libsvn_wc/adm_crawler.c:
>   (report_revisions_and_depths): wrap a couple comments. when the child
> is
>     stripped off the URI, then decode it before comparison.
> 
> Modified:
>     subversion/trunk/subversion/libsvn_wc/adm_crawler.c
> 
> Modified: subversion/trunk/subversion/libsvn_wc/adm_crawler.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm_
> crawler.c?rev=935413&r1=935412&r2=935413&view=diff
> =======================================================================
> =======
> --- subversion/trunk/subversion/libsvn_wc/adm_crawler.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/adm_crawler.c Sun Apr 18
> 21:16:55 2010
> @@ -360,8 +360,8 @@ report_revisions_and_depths(svn_wc__db_t
>              }
>            else
>              {
> -              /* We want to pull in the excluded target. So, report it
> as deleted,
> -                 and server will respond properly. */
> +              /* We want to pull in the excluded target. So, report it
> as
> +                 deleted, and server will respond properly. */
>                if (! report_everything)
>                  SVN_ERR(reporter->delete_path(report_baton,
>                                                this_path, iterpool));
> @@ -424,10 +424,10 @@ report_revisions_and_depths(svn_wc__db_t
>            else
>              missing = TRUE;
> 
> -          /* If a node is still missing from disk here, we have no way
> to recreate
> -             it locally, so report as missing and move along.  Again,
> don't bother
> -             if we're reporting everything, because the dir is already
> missing on
> -             the server. */
> +          /* If a node is still missing from disk here, we have no way
> to
> +             recreate it locally, so report as missing and move along.
> +             Again, don't bother if we're reporting everything,
> because the
> +             dir is already missing on the server. */
>            if (missing && wrk_kind == svn_wc__db_kind_dir
>                 && (depth > svn_depth_files || depth ==
> svn_depth_unknown))
>              {
> @@ -450,7 +450,8 @@ report_revisions_and_depths(svn_wc__db_t
>            const char *childname = svn_uri_is_child(dir_repos_relpath,
>                                                     this_repos_relpath,
> NULL);

s/svn_uri/svn_relpath/
> 
> -          if (!childname || strcmp(childname, child) != 0)
> +          if (childname == NULL
> +              || strcmp(svn_path_uri_decode(childname, iterpool),
> child) != 0)

A repos relpath is already uri_decoded, so this would break paths that have a literal %20 somewhere. (It would be correct if it was a real uri instead of a relpath)

	Bert