After trying this, it does not solve the issue I'm having, though I believe
it's definately related, and perhaps the suggested fix needs to be applied
at other locations in the code.
On Tue, Sep 2, 2008 at 3:26 PM, Matthew Inger <mattinger_at_gmail.com> wrote:
> It looks like it could be related. I had concentrated on the interaction
> with neon, and not what was building
> the actualy URLs being passed, which it looks like this other thread was
> looking it.
>
> Looking at the code changes, then this looks absolutely correct, and is
> much preferrable to what i had done.
>
>
>
> On Tue, Sep 2, 2008 at 2:30 PM, Karl Fogel <kfogel_at_red-bean.com> wrote:
>
>> "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);
>>
>
>
Received on 2008-09-02 22:29:23 CEST