Hiding some directories

[Ville Skyttä]

On Thu, 2006-01-12 at 19:59 -0500, Jonathan Noack wrote:
> Ville Skyttä wrote:
> > 
> > No strong opinions nor objections if this is really needed, but couldn't
> > some extended patterns be used for that, and some commented out examples
> > of those be added to cvsweb.conf?  Maybe negative look-behind (see "man
> > perlre")?  It could get somewhat hairy though.
> 
> It probably could be done with some extended patterns but few of our 
> users would understand them (I certainly wouldn't!).  Having 
> @AllowedFiles means that we can give our users 2 options:
> 1) Allow all with @ForbiddenFiles override (This is the default).
> 2) Forbid all with @AllowedFiles override (which in turn is overridden 
> by @ForbiddenFiles).
> 
> This is quite powerful AND easy to configure.

Okay, fair enough.

> As we don't want to allow more than we intend, we must be more careful 
> with @AllowedFiles then with @ForbiddenFiles.  Here are some best 
> practices for @AllowedFiles:

This information would be an useful addition somewhere.  Maybe it's a
bit too much for a comment in cvsweb.conf though.  INSTALL?

> 3) Patterns for specific files should end with '$' to match the end of 
> filename.  For example: Use 'qr|^dir/file.txt$|o' instead of 
> 'qr|^dir/file.txt|o'.  The latter could erroneously match 
> dir/file.txt.old or dir/file.txt/real_file.txt.

"." should be replaced by "\." in the example regexps above.

> Why did I write that much?!?

:)

Some comments about the patch:

  +# file then a file/dir must be listed for access to be granted.

s/listed for/listed in it for/

  +  if (($cvsroot ne $path) && (defined(@AllowedFiles))) {

Why the first test?  Also, using defined() on an array is deprecated
(see perldoc -f defined).  I think this line could be reduced to
"if (@AllowedFiles) {".

  +# If @AllowedFiles is not defined, only @ForbiddenFiles is enforced.

s/is not defined/is empty/

  +#@AllowedFiles = (
  +   #qr|^my/+public/+dir|o,
  +#);

The first and third lines don't need to be commented out.

Could you post a revised patch along with a ChangeLog entry?  Also, all
configuration changes should be documented in the INSTALL file's
"Upgrade instructions" section.