"Matthew Inger" <mattinger_at_gmail.com> writes:
> 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.
Hmmm... do you think you can find those other locations? It would be
great to nail this down, now that we (maybe) are on the right track.
-K
> 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);
---------------------------------------------------------------------
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 22:30:05 CEST