You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Justin Clift <ju...@postgresql.org> on 2002/09/03 19:18:31 UTC

But with trailing slashes in SVNPath command in httpd.conf

Hi everyone,

Have been getting Apache2 (HEAD from a day or two ago) + Subversion
working on FreeBSD 4.6.2.

Something which just cropped up is a simple mistake in the httpd.conf
that people will make, and it's generating a subversion error.

This works:

<Location /foob>
   DAV svn
   SVNPath /usr/local/www/repos
</Location>


This generates an error:

<Location /foob>
   DAV svn
   SVNPath /usr/local/www/repos
</Location>

The error is:

assertion "is_canonical_nts (base, blen)" failed: file
"subversion/libsvn_subr/path.c", line 178
[Tue Sep 03 15:15:57 2002] [notice] child pid 38433 exit signal Abort
trap (6)

The repository really is at /usr/local/www/repos, so it's all correct. 
This is using svn r3118 as of a few hours ago, and is 100% reproducible.

Hope this helps.

:-)

Regards and best wishes,

Justin Clift

-- 
"My grandfather once told me that there are two kinds of people: those
who work and those who take the credit. He told me to try to be in the
first group; there was less competition there."
   - Indira Gandhi

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] But with trailing slashes in SVNPath command in httpd.conf

Posted by Branko Čibej <br...@xbc.nu>.
Philip Martin wrote:

>Branko ÄŒibej <br...@xbc.nu> writes:
>
>  
>
>>>NULL is not a canonical path, lots of the Subversion functions will
>>>dump core on a NULL path.  I suppose it could return the empty path
>>>("") but I don't think that's a good thing to do.
>>>
>>>      
>>>
>>If "" is the current directory these days, then canonicalizing NULL to
>>"" would definitely be wrong. :-)
>>    
>>
>
>Agreed.
>
>  
>
>>>I am happy saying that NULL is an error.
>>>
>>>      
>>>
>>Sure. The question is whether svn_path_canonicalize sould abort on
>>NULL, or pass it through and let some other code abort instead.
>>    
>>
>
>It would require special case code in svn_path_canonicalize_nts
>instead of at the places that call it, and not all the callers need it
>as some cannot have a NULL argument.  My first instinct is to say the
>current code is correct, it is the responsibilty of the caller not to
>pass NULL.
>
Heh, right, good point.

>  What about the (unused) stringbuf version of the function
>'void svn_path_canonicalize (svn_stringbuf_t *)' would that accept
>NULL as well?
>

It couldn't, could it. O.K., you've convinced me, so go ahead and commit 
that patch. :-)


-- 
Brane Čibej   <br...@xbc.nu>   http://www.xbc.nu/brane/


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

tossing stringbuf functions (was: [PATCH] But with trailing slashes...)

Posted by Greg Stein <gs...@lyra.org>.
On Wed, Sep 04, 2002 at 09:38:36PM +0100, Philip Martin wrote:
>...
> pass NULL.  What about the (unused) stringbuf version of the function
> 'void svn_path_canonicalize (svn_stringbuf_t *)' would that accept
> NULL as well?

Unused? Well, then (IMO) it should just go away.

I see no reason to apply canonicalization to stringbufs. The stringbuf type
should be reserved only to "uses" of appending lots of bytes to build up a
final string. We should not encourage other uses (via the presence of
functions which operate on stringbufs).

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] But with trailing slashes in SVNPath command in httpd.conf

Posted by Philip Martin <ph...@codematters.co.uk>.
Branko Čibej <br...@xbc.nu> writes:

> >NULL is not a canonical path, lots of the Subversion functions will
> >dump core on a NULL path.  I suppose it could return the empty path
> >("") but I don't think that's a good thing to do.
> >
> If "" is the current directory these days, then canonicalizing NULL to
> "" would definitely be wrong. :-)

Agreed.

> >I am happy saying that NULL is an error.
> >
> 
> Sure. The question is whether svn_path_canonicalize sould abort on
> NULL, or pass it through and let some other code abort instead.

It would require special case code in svn_path_canonicalize_nts
instead of at the places that call it, and not all the callers need it
as some cannot have a NULL argument.  My first instinct is to say the
current code is correct, it is the responsibilty of the caller not to
pass NULL.  What about the (unused) stringbuf version of the function
'void svn_path_canonicalize (svn_stringbuf_t *)' would that accept
NULL as well?

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] But with trailing slashes in SVNPath command in httpd.conf

Posted by Branko Čibej <br...@xbc.nu>.
Philip Martin wrote:

>Branko ÄŒibej <br...@xbc.nu> writes:
>
>  
>
>>Is there reason why svn_path_canonicalize_nts shouldnt accept a NULL
>>path and return NULL?
>>    
>>
>
>NULL is not a canonical path, lots of the Subversion functions will
>dump core on a NULL path.  I suppose it could return the empty path
>("") but I don't think that's a good thing to do.
>
If "" is the current directory these days, then canonicalizing NULL to 
"" would definitely be wrong. :-)

>I am happy saying that NULL is an error.
>  
>
Sure. The question is whether svn_path_canonicalize sould abort on NULL, 
or pass it through and let some other code abort instead.


-- 
Brane Čibej   <br...@xbc.nu>   http://www.xbc.nu/brane/


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] But with trailing slashes in SVNPath command in httpd.conf

Posted by Philip Martin <ph...@codematters.co.uk>.
Branko Čibej <br...@xbc.nu> writes:

> Is there reason why svn_path_canonicalize_nts shouldnt accept a NULL
> path and return NULL?

NULL is not a canonical path, lots of the Subversion functions will
dump core on a NULL path.  I suppose it could return the empty path
("") but I don't think that's a good thing to do.  I am happy saying
that NULL is an error.

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] But with trailing slashes in SVNPath command in httpd.conf

Posted by Branko Čibej <br...@xbc.nu>.
Philip Martin wrote:

>Ben Collins-Sussman <su...@collab.net> writes:
>
>  
>
>>Justin Clift <ju...@postgresql.org> writes:
>>
>>    
>>
>>>This works:
>>>
>>><Location /foob>
>>>   DAV svn
>>>   SVNPath /usr/local/www/repos
>>></Location>
>>>
>>>
>>>This generates an error:
>>>
>>><Location /foob>
>>>   DAV svn
>>>   SVNPath /usr/local/www/repos
>>></Location>
>>>      
>>>
>>A bug in the bug report.  The failing block looks like this:
>>
>>  SVNPath /usr/local/www/repos/
>>    
>>
>
>I don't know much about mod_dav_svn, but this works for me.  Does it
>look OK?
>

Looks fine to me.

>* subversion/mod_dav_svn/mod_dav_svn.c
>  (dav_svn_get_fs_path, dav_svn_get_fs_parent_path): Canonicalize paths.
>
>
>
>Index: ../svn/subversion/mod_dav_svn/mod_dav_svn.c
>===================================================================
>--- ../svn/subversion/mod_dav_svn/mod_dav_svn.c
>+++ ../svn/subversion/mod_dav_svn/mod_dav_svn.c	Wed Sep  4 20:44:33 2002
>@@ -210,7 +210,9 @@
>     dav_svn_dir_conf *conf;
> 
>     conf = ap_get_module_config(r->per_dir_config, &dav_svn_module);
>-    return conf->fs_path;
>+    return (conf->fs_path
>+            ? svn_path_canonicalize_nts (conf->fs_path, r->pool)
>+            : NULL);
> }
>

Is there reason why svn_path_canonicalize_nts shouldnt accept a NULL 
path and return NULL?

-- 
Brane Čibej   <br...@xbc.nu>   http://www.xbc.nu/brane/


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] But with trailing slashes in SVNPath command in httpd.conf

Posted by Ben Collins-Sussman <su...@collab.net>.
That patch looks good to me!  Greg?


Philip Martin <ph...@codematters.co.uk> writes:

> Ben Collins-Sussman <su...@collab.net> writes:
> 
> > Justin Clift <ju...@postgresql.org> writes:
> > 
> > > This works:
> > > 
> > > <Location /foob>
> > >    DAV svn
> > >    SVNPath /usr/local/www/repos
> > > </Location>
> > > 
> > > 
> > > This generates an error:
> > > 
> > > <Location /foob>
> > >    DAV svn
> > >    SVNPath /usr/local/www/repos
> > > </Location>
> > 
> > A bug in the bug report.  The failing block looks like this:
> > 
> >   SVNPath /usr/local/www/repos/
> 
> I don't know much about mod_dav_svn, but this works for me.  Does it
> look OK?
> 
> 
> * subversion/mod_dav_svn/mod_dav_svn.c
>   (dav_svn_get_fs_path, dav_svn_get_fs_parent_path): Canonicalize paths.
> 
> 
> 
> Index: ../svn/subversion/mod_dav_svn/mod_dav_svn.c
> ===================================================================
> --- ../svn/subversion/mod_dav_svn/mod_dav_svn.c
> +++ ../svn/subversion/mod_dav_svn/mod_dav_svn.c	Wed Sep  4 20:44:33 2002
> @@ -210,7 +210,9 @@
>      dav_svn_dir_conf *conf;
>  
>      conf = ap_get_module_config(r->per_dir_config, &dav_svn_module);
> -    return conf->fs_path;
> +    return (conf->fs_path
> +            ? svn_path_canonicalize_nts (conf->fs_path, r->pool)
> +            : NULL);
>  }
>  
>  const char *dav_svn_get_fs_parent_path(request_rec *r)
> @@ -218,7 +220,9 @@
>      dav_svn_dir_conf *conf;
>  
>      conf = ap_get_module_config(r->per_dir_config, &dav_svn_module);
> -    return conf->fs_parent_path;
> +    return (conf->fs_parent_path
> +            ? svn_path_canonicalize_nts (conf->fs_parent_path, r->pool)
> +            : NULL);
>  }
>  
>  
> 
> 
> -- 
> Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] But with trailing slashes in SVNPath command in httpd.conf

Posted by Greg Stein <gs...@lyra.org>.
On Wed, Sep 04, 2002 at 08:53:17PM +0100, Philip Martin wrote:
>...
> > A bug in the bug report.  The failing block looks like this:
> > 
> >   SVNPath /usr/local/www/repos/
> 
> I don't know much about mod_dav_svn, but this works for me.  Does it
> look OK?

The paths should be canonicalized as they go into the 'conf' record, rather
than each time they come out.

Otherwise, yes... the concept seems fine!

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

[PATCH] But with trailing slashes in SVNPath command in httpd.conf

Posted by Philip Martin <ph...@codematters.co.uk>.
Ben Collins-Sussman <su...@collab.net> writes:

> Justin Clift <ju...@postgresql.org> writes:
> 
> > This works:
> > 
> > <Location /foob>
> >    DAV svn
> >    SVNPath /usr/local/www/repos
> > </Location>
> > 
> > 
> > This generates an error:
> > 
> > <Location /foob>
> >    DAV svn
> >    SVNPath /usr/local/www/repos
> > </Location>
> 
> A bug in the bug report.  The failing block looks like this:
> 
>   SVNPath /usr/local/www/repos/

I don't know much about mod_dav_svn, but this works for me.  Does it
look OK?


* subversion/mod_dav_svn/mod_dav_svn.c
  (dav_svn_get_fs_path, dav_svn_get_fs_parent_path): Canonicalize paths.



Index: ../svn/subversion/mod_dav_svn/mod_dav_svn.c
===================================================================
--- ../svn/subversion/mod_dav_svn/mod_dav_svn.c
+++ ../svn/subversion/mod_dav_svn/mod_dav_svn.c	Wed Sep  4 20:44:33 2002
@@ -210,7 +210,9 @@
     dav_svn_dir_conf *conf;
 
     conf = ap_get_module_config(r->per_dir_config, &dav_svn_module);
-    return conf->fs_path;
+    return (conf->fs_path
+            ? svn_path_canonicalize_nts (conf->fs_path, r->pool)
+            : NULL);
 }
 
 const char *dav_svn_get_fs_parent_path(request_rec *r)
@@ -218,7 +220,9 @@
     dav_svn_dir_conf *conf;
 
     conf = ap_get_module_config(r->per_dir_config, &dav_svn_module);
-    return conf->fs_parent_path;
+    return (conf->fs_parent_path
+            ? svn_path_canonicalize_nts (conf->fs_parent_path, r->pool)
+            : NULL);
 }
 
 


-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Bug with trailing slashes in SVNPath command in httpd.conf

Posted by Justin Clift <ju...@postgresql.org>.
Ben Collins-Sussman wrote:
> 
<snip>
> A bug in the bug report.  The failing block looks like this:
> 
>   SVNPath /usr/local/www/repos/

Arrghh.  Sorry about that.  Even did a typo in the Subject: line for
good measure.

:(

+ Justin

-- 
"My grandfather once told me that there are two kinds of people: those
who work and those who take the credit. He told me to try to be in the
first group; there was less competition there."
   - Indira Gandhi

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: But with trailing slashes in SVNPath command in httpd.conf

Posted by Ben Collins-Sussman <su...@collab.net>.
Justin Clift <ju...@postgresql.org> writes:

> This works:
> 
> <Location /foob>
>    DAV svn
>    SVNPath /usr/local/www/repos
> </Location>
> 
> 
> This generates an error:
> 
> <Location /foob>
>    DAV svn
>    SVNPath /usr/local/www/repos
> </Location>

A bug in the bug report.  The failing block looks like this:

  SVNPath /usr/local/www/repos/


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org