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

Re: Issue with merging files containing spaces

From: Karl Fogel <kfogel_at_red-bean.com>
Date: Tue, 02 Sep 2008 14:30:46 -0400

"C. Michael Pilato" <cmpilato_at_collab.net> writes:
> Matthew Inger wrote:
>> I'm running into an issue when doing branch merges. It seems that file
>> names with spaces in them
>> cannot properly be merged, resulting in the merging error:
>>
>> svn: URL 'http://host/svn/repos/trunk/dir with spaces/file.txt' is
>> malformed or the scheme or host or path is missing.
>>
>> I've isolated down the cause of the problem to the following function:
>>
>> int ne_uri_parse(const char *uri, ne_uri *parsed);
>>
>> It seems that when trying to parse the path component, it is doing the
>> following:
>>
>> while (uri_lookup(*p) & URI_SEGCHAR)
>> p++;
>>
>>
>> The problem here is that definition of URI_SEGCHAR includes only the
>> following:
>>
>> FS -- /
>> PC -- %
>> PS -- +
>> SD -- ! $ & ' ( ) * + , ; =
>> CL -- :
>> AL -- Alpabet
>> DG -- Digit
>> DS -- dash
>> DT -- .
>> US -- _
>> TD -- ~
>>
>> Notice that spaces are not included in this definition.
>>
>> So the solution is either to change how neon parses the URI, or to
>> properly escape the spaces with a "+" symbol
>> in the "session.c" file which is calling into the neon library.
>>
>> Any thoughts?
>
> Subversion should be properly URI-encoding the paths it sends through Neon,
> and apparently it is not. That's a bug. (Though, the correct URI encoded
> form of a space character is "%20" -- the plus sign is, I believe, only for
> when the space occurs in the query parameters portion of a URL, not the path
> part.)

This is a bit of a drive-by reply, but I just responded to a tentative
patch by Fyodor Sheremetyev that might be related. Below is my
response, which quotes his patch, and here is the thread his post was
part of:

   http://subversion.tigris.org/servlets/BrowseList?list=dev&by=thread&from=645165

Is this the same problem you noticed, Matthew?

-Karl

> "Fyodor Sheremetyev" <fyodor_at_visualsvn.com> writes:
> > On Fri, Aug 29, 2008 at 12:44 AM, David Huang <david.huang_at_alterity.com> wrote:
> >> It looks like Fyodor Sheremetyev was able to create a repro script... here's
> >> the call stack:
> >
> > I'm trying to fix this problem. Attached is a short and dirty patch
> > that makes my reproduction script work. But I'm absolutely unsure that
> > it fixes the problem correctly.
> >
> > I see no consistency in using svn_path_uri_encode and
> > svn_path_uri_decode in the code and couldn't find any guidelines
> > regarding uri encoding. Which level should do that by design? Please
> > could somebody point at the right direction.
>
> I think your patch is correct. I've quoted it below so others can see
> it. A couple of comments:
>
> 1. A log message always helps :-). See
> http://subversion.tigris.org/hacking.html#log-messages
>
> 2. You attached it with MIME-type "application/octet-stream", which
> means that mailreaders can't display it inline. If you use
> "text/plain" instead, that makes things a bit easier. See
> http://subversion.tigris.org/hacking.html#patches
>
> The guideline for URI encoding and decoding is just to follow the doc
> strings of whatever functions the data will be passed to. If the
> function says it needs URI-encoded data, then make sure it receives
> that. If it does not, then usually it takes un-encoded data.
>
> -Karl
>
> > Index: subversion/libsvn_client/merge.c
> > ===================================================================
> > --- subversion/libsvn_client/merge.c (revision 32799)
> > +++ subversion/libsvn_client/merge.c (working copy)
> > @@ -6291,9 +6291,9 @@
> > over and over. */
> > /* We never merged to the source. Just return the branch point. */
> > const char *yc_ancestor_path,
> > - *source_url = svn_path_join(source_repos_root, source_repos_rel_path,
> > + *source_url = svn_path_join(source_repos_root, svn_path_uri_encode(source_repos_rel_path, subpool),
> > subpool),
> > - *target_url = svn_path_join(source_repos_root, target_repos_rel_path,
> > + *target_url = svn_path_join(source_repos_root, svn_path_uri_encode(target_repos_rel_path, subpool),
> > subpool);
> >
> > SVN_ERR(svn_client__get_youngest_common_ancestor(&yc_ancestor_path,
> > @@ -6306,7 +6306,7 @@
> > _("'%s@%ld' must be ancestrally related to "
> > "'%s@%ld'"), source_url, source_rev,
> > target_url, target_rev);
> > - *url_left = svn_path_join(source_repos_root, yc_ancestor_path, pool);
> > + *url_left = svn_path_join(source_repos_root, svn_path_uri_encode(yc_ancestor_path, pool), pool);
> > *source_mergeinfo_p = apr_hash_make(pool);
> >
> > svn_pool_destroy(subpool);

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-09-02 20:31:03 CEST

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