On Tue, Sep 9, 2008 at 1:23 AM, Julian Foad <julianfoad_at_btopenworld.com> wrote:
> 1) Can you (or someone) confirm that the test fails without the fix, and
> passes with the fix? I can only test on ra-local and ra-svn at the
> moment, and this new test passed both, even without the fix. I'm not
> sure if that's expected. I guess it means those protocols don't check
> for well-formed URLs.
>
> If the test and the fix are behaving, we should commit this.
I can confirm that the test fails on http without the fix and succeeds
with the fix.
> 2) Having become aware of this problem, if we look further, there are
> lots more calls to svn_path_join() that look wrong. Especially, 'grep
> "url.*svn_path_join"' in Subversion source code reveals a bunch that
> mostly look wrong. It also reveals a few that use uri_encode() on the
> second argument that could be replaced by svn_path_url_add_component().
>
> Do you fancy (a) fixing those and/or (b) figuring out tests for
> exercising those?
Yes, I'm going to take deeper and wider look at the encoding/decoding
problem after this patch is reviewed and committed.
> 3) Can we modify our test suite to include paths with spaces in, and
> thereby catch many URI encoding errors automatically? We can add
> specific tests for specific cases, but I'm wondering if we can modify
> the Greek Tree to include some nasty names that will get exercised
> through several different code paths.
Having all such cases embeded in the whole test suite might make it
more complex to understand and maintain. I think it's more of a broken
design than a bug and can be fixed by explicitly setting boundaries
which would care about URI encoding (and the rest of the code would be
using decoded URIs).
---------------------------------------------------------------------
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-09 07:08:40 CEST