[Proftpd-devel] Re: Bug#410 -- broken Ignore Hidden, and more (long)

[TJ S. <tj...@di...>]

jss>Ok... good thing you've got this list. ;)

Well, it's the original one I cobbled together before putting it into
bugzilla, which is probably why it doesn't list the ASCII handling bug,
and a few others.

jss>I have contacted Johnie Ingram regarding the two above.  He was,
jss>ummm .., not-commitant. :( That means I'll probably be pulling
jss>mod_sqlpw and friends.

Hmmm...is there any way to _not_ pull those modules?  I know of at least
one proftpd-developer willing to help maintain those in some capacity, and
we can pitch in, too.  I think quite a few proftpd users use those SQL
databases for non-system authentication information, and probably logging.
I'd hate to deprive them of some nifty functionality.

jss>> Bug#410 -- IgnoreHidden broken -- patch, but not committed. Ok,
jss>this is the "problem one" we need to talk about.  Do you think a
jss>patch could be produced that ONLY fixed IgnoreHidden without
jss>changing too much of dirtree.c?  I had a couple of issues with
jss>this one .. but I can't remember what they were without seeing the
jss>patch.  Can you send to me?  Also, can you split out #422 from it
jss>(below), because I would like to get that one in and there is no
jss>reason not to.

OK -- I've attached three different patches: one for Bug#422 by itself,
one to fix the HideNoAccess bug (it doesn't properly handle a boolean
argument), and one to fix the IgnoreHidden bug.  I should've separated the
HideNoAccess and IgnoreHidden patches, but they both cropped up in the
same coding spurt, and it was easier (read: lazy) to just run cvs diff
once.

I feel some explanation for the IgnoreHidden patch is in order ;) (btw, a
lot of the patch is probably formatting/spacing "corrections" -- a lot of
code doesn't look right in vi.  And the any patches I generate, which
happen to change the spacing, don't come out right; at least, when I apply
the patches to a pristine CVS download to double-check, the bad spacing is
still there.  I suspect some sort of spacing interaction among my vi
settings, diff, patch, and/or cvs.  I apologize for any nasty-looking code
this generates...sorry.)

First, one major source of change, which affects mod_ls, mod_xfer, and
dirtree.c, is this:

+int dir_check_hidden(const char *path);
 int dir_check_op_mode(pool*,char*,int,int,int,int);
-int dir_check_full(pool*,char*,char*,char*,int*);
-int dir_check(pool*,char*,char*,char*,int*);
-int dir_check_canon(pool*,char*,char*,char*,int*);

The dir_check_*() functions took an int * as their last argument, which
signalled the "hidden-ness" of the file.  Tracking that variable, I found
that these functions themselves didn't use that variable as much as pass
it on to dir_check_op().  Instead, I created the dir_check_hidden()
function, to be called instead of passing that pointer around.  Plus, it
creates a very good central location for manipulating file "hidden-ness"
in other ways in the future -- I was thinking of how to do my
HideFiles/ShowFiles patch when I was writing this patch.  The ls_perms_*()
functions in mod_ls similarly changed.

The IgnoreHidden patch isn't strictly confined to src/dirtree.c,
because part of the problem occurs in mod_ls.c -- this is another place
where the patch changes things a little.  To listdir() was added:

+         /* make sure IgnoreHidden is properly honored.  "." and
+           * ".." are not to be treated as hidden files, though
+          */
+         if (is_dotdir(*s) || !ignore_hidden)
+            d = listfile(cmd,workp,*s);

I think, in this case, treating "." and ".." as files lead to a path
traversal that wasn't right.

is_dotdir() was moved to dirtree.c, where it seemed to fit, IMHO, more
with the path-handling functions there, than in mod_ls.

jss>Also, there was #399, IIRC, this had to do with \r removal in ascii mode.  I
jss>sent you a couple messages on it.

Yeah...this one is going to be a bit tough.  Could try to handle the input
stream character-by-character...but then you lose the buffered I/O.
Otherwise, could do the "lag one character behind" approach, as you
mentioned.  Neither is particularly appealing.  Lose the buffers, or keep
the buffers and deal with the buffer boundaries...=P

----------------------------------------------------------------
  TJ Saunders			<tj...@di...>
----------------------------------------------------------------