[Please keep the thread on the list]
Hugo Bastos Weber wrote:
> Banko, thanks for the comments. My tests didn't show the result you
> wrote, please read on:
>
Oh duh sorry, I totally misread your patch. Shame on me, I forgot that
we canonicalizing paths strips trailing slashes, so you'll never get a
file:///X:/ in that function if it's called properly.
So your patch appears to be correct, but I can't test it as I currently
don't have a Windows development box. I'd personally avoid the strlen,
though, it's wasteful and not necessary;
(!dup_path[3] || dup_path[3] == '/')
works just as well.
-- Brane
> the following comment was taken from
> subversion/libsvn_ra_local/split_url.c, line 79:
>
> /* On Windows, we'll typically have to skip the leading / if the
> path starts with a drive letter. Like most Web browsers, We
> support two variants of this scheme:
> ...
>
> I tried file:///X:/ on Mozilla Firefox and it works perfectlly.
>
> subversion/libsvn_ra_local/split_url.c, line 102, skips the leading
> slash when it enters the following:
>
> if (!hostname && dup_path[1] && strchr(valid_drive_letters,
> dup_path[1])
> && (dup_path[2] == ':' || dup_path[2] == '|')
> && dup_path[3] == '/')
> {
>
> My tests:
> if called file:///x:/ then dup_path is "/x:", then dup_path[3] is not
> == '/'
> if called file:///x:. then dup_path is "/x:.", then dup_path[3] is not
> == '/'
> if called file:///x:/. then dup_path is "/x:", then dup_path[3] is not
> == '/'
> if called file://x:/ then an error show "contains only a hostname, no
> path"
>
> The solution in my patch is to change the if to:
>
> if (!hostname && dup_path[1] && strchr(valid_drive_letters,
> dup_path[1])
> && (dup_path[2] == ':' || dup_path[2] == '|')
> && (dup_path[3] == '/' || strlen(dup_path) == 3)
> {
>
> now:
>
> if called file:///x:/ then dup_path is "/x:", then dup_path[3] is not
> == '/' however dup_path[2] == ':' and strlen(dup_path) == 3 <-- OK, it
> works, it's canonical
> if called file:///x:. then dup_path is "/x:.", then dup_path[3] is not
> == '/' and strlen(dup_path) != 3 <-- Don't work
> if called file:///x:/. then dup_path is "/x:", then dup_path[3] is not
> == '/' just like the first case, it works too.
> if called file://x:/ then an error show "contains only a hostname, no
> path" <-- still the same behavior
>
>
> Branko, if you try to run file:///X:/, file:///X:. or file:///X:/.
> you'll see none of them work. Also, the patch is not acception
> file:///X:. because it will have a lenght == 4 no 3.
>
>
> Waiting for more feedbacks,
> Hugo.
>
>
>
>
>
>
>
>>>> Branko Čibej <brane_at_xbc.nu> 10/14/09 1:55 am >>>
>>>>
> Hugo Bastos Weber wrote:
>
>> [[[
>>
>> * subversion/libsvn_ra_local/split_url.c
>> (svn_ra_local__split_URL): Allow file:///X:/ in addition to
>>
> file:///X:/path. This fix allows one to use repository in the root drive
> lettler.
>
>> ]]]
>>
>>
>
> Isn't the log message wrong? You're not allowing file:///X:/ which is
> already supported, but file:///X:. But that's not a canonical URL (we
> "know" that /^[A-Za-z]:/ is a directory on Windows), and
> libsvn_ra_local
> has no business accepting non-canonical paths. The bug is IMHO in the
> code that calls, e.g., svn_ra_open with a non-canonical URL.
>
> -- Brane
>
>
>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2407695
Received on 2009-10-14 20:49:47 CEST