| Message ID | 3343338e-9262-dfd1-90ef-36908c91dad4@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Directory traversal fixes | expand |
On Fri, May 7, 2021 at 9:22 AM Derrick Stolee <stolee@gmail.com> wrote: > > On 5/7/2021 12:04 AM, Elijah Newren via GitGitGadget wrote: > > This patchset fixes a few directory traversal issues, where fill_directory() > > would traverse into directories that it shouldn't and not traverse into > > directories that it should. One of these issues was reported recently on > > this list[1], another was found at $DAYJOB. > > > > The fifth patch might have backward compatibility implications, but is easy > > to review. Even if the logic in dir.c makes your eyes glaze over, at least > > take a look at the fifth patch. > > My eyes were glazing over, so I went to read the whole treat_directory() > method and its related documentation comment. I found it to be a bit > confusing that it was referencing names that were deprecated 12 years ago. > > Here is a patch that you could add to this series to improve these > comments. > > Thanks, > -Stolee > > -- >8 -- > > From 587a94ac396c969b6e7734ee46afeac20e87ccb9 Mon Sep 17 00:00:00 2001 > From: Derrick Stolee <dstolee@microsoft.com> > Date: Fri, 7 May 2021 12:14:13 -0400 > Subject: [PATCH] dir: update stale description of treat_directory() > > The documentation comment for treat_directory() was originally written > in 095952 (Teach directory traversal about subprojects, 2007-04-11) > which was before the 'struct dir_struct' split its bitfield of named > options into a 'flags' enum in 7c4c97c0 (Turn the flags in struct > dir_struct into a single variable, 2009-02-16). When those flags > changed, the comment became stale, since members like > 'show_other_directories' transitioned into flags like > DIR_SHOW_OTHER_DIRECTORIES. > > Update the comments for treat_directory() to use these flag names rather > than the old member names. > > Signed-off-by: Derrick Stolee <dstolee@microsoft.com> > --- > dir.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/dir.c b/dir.c > index 3beb8e17a83..0a0138bc1aa 100644 > --- a/dir.c > +++ b/dir.c > @@ -1749,13 +1749,13 @@ static enum exist_status directory_exists_in_index(struct index_state *istate, > * Case 3: if we didn't have it in the index previously, we > * have a few sub-cases: > * > - * (a) if "show_other_directories" is true, we show it as > - * just a directory, unless "hide_empty_directories" is > + * (a) if DIR_SHOW_OTHER_DIRECTORIES flag is set, we show it as > + * just a directory, unless DIR_HIDE_EMPTY_DIRECTORIES is > * also true, in which case we need to check if it contains any > * untracked and / or ignored files. > - * (b) if it looks like a git directory, and we don't have > - * 'no_gitlinks' set we treat it as a gitlink, and show it > - * as a directory. > + * (b) if it looks like a git directory and we don't have the > + * DIR_NO_GITLINKS flag, then we treat it as a gitlink, and > + * show it as a directory. > * (c) otherwise, we recurse into it. > */ > static enum path_treatment treat_directory(struct dir_struct *dir, > @@ -1843,7 +1843,6 @@ static enum path_treatment treat_directory(struct dir_struct *dir, > return path_recurse; > } > > - /* This is the "show_other_directories" case */ > assert(dir->flags & DIR_SHOW_OTHER_DIRECTORIES); > > /* > @@ -1858,7 +1857,7 @@ static enum path_treatment treat_directory(struct dir_struct *dir, > /* Special cases for where this directory is excluded/ignored */ > if (excluded) { > /* > - * In the show_other_directories case, if we're not > + * If DIR_SHOW_OTHER_DIRECTORIES is set and we're not > * hiding empty directories, there is no need to > * recurse into an ignored directory. > */ > -- > 2.31.1.vfs.0.0.80.gb082c853c0e Looks good to me; I'll give it some more time for other comments to come in, but when I re-roll, I'll include this patch of yours.
diff --git a/dir.c b/dir.c index 3beb8e17a83..0a0138bc1aa 100644 --- a/dir.c +++ b/dir.c @@ -1749,13 +1749,13 @@ static enum exist_status directory_exists_in_index(struct index_state *istate, * Case 3: if we didn't have it in the index previously, we * have a few sub-cases: * - * (a) if "show_other_directories" is true, we show it as - * just a directory, unless "hide_empty_directories" is + * (a) if DIR_SHOW_OTHER_DIRECTORIES flag is set, we show it as + * just a directory, unless DIR_HIDE_EMPTY_DIRECTORIES is * also true, in which case we need to check if it contains any * untracked and / or ignored files. - * (b) if it looks like a git directory, and we don't have - * 'no_gitlinks' set we treat it as a gitlink, and show it - * as a directory. + * (b) if it looks like a git directory and we don't have the + * DIR_NO_GITLINKS flag, then we treat it as a gitlink, and + * show it as a directory. * (c) otherwise, we recurse into it. */ static enum path_treatment treat_directory(struct dir_struct *dir, @@ -1843,7 +1843,6 @@ static enum path_treatment treat_directory(struct dir_struct *dir, return path_recurse; } - /* This is the "show_other_directories" case */ assert(dir->flags & DIR_SHOW_OTHER_DIRECTORIES); /* @@ -1858,7 +1857,7 @@ static enum path_treatment treat_directory(struct dir_struct *dir, /* Special cases for where this directory is excluded/ignored */ if (excluded) { /* - * In the show_other_directories case, if we're not + * If DIR_SHOW_OTHER_DIRECTORIES is set and we're not * hiding empty directories, there is no need to * recurse into an ignored directory. */