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 21:43:22 CEST