Re: [PATCH v2] list-objects-filter: disable 'sparse:path' filters
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 28, 2019, 19:41 UTC
- Message-ID
- <xmqqo93m1i49.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <20190525142834.6168-1-chriscool@tuxfamily.org>
Christian Couder <christian.couder@gmail.com> writes:
Show 39 quoted lines
> If someone wants to use as a filter a sparse file that is in the > repository, something like "--filter=sparse:oid=<ref>:<path>" > already works. > > So 'sparse:path' is only interesting if the sparse file is not in > the repository. In this case though the current implementation has > a big security issue, as it makes it possible to ask the server to > read any file, like for example /etc/password, and to explore the > filesystem, as well as individual lines of files. > > If someone is interested in using a sparse file that is not in the > repository as a filter, then at the minimum a config option, such > as "uploadpack.sparsePathFilter", should be implemented first to > restrict the directory from which the files specified by > 'sparse:path' can be read. > > For now though, let's just disable 'sparse:path' filters. > > Helped-by: Matthew DeVore <matvore@google.com> > Helped-by: Jeff Hostetler <git@jeffhostetler.com> > Signed-off-by: Christian Couder <chriscool@tuxfamily.org> > --- > > Changes since the RFC version are the following: > > - improved the error message when 'sparse:path' is used, > - updated "git-completion.bash", > - freed "sparse_path_value" field in list_objects_filter_release(), > - updated tests (t5317 and t6112). > > Thanks to Matthew and Jeff for the suggestions. > > contrib/completion/git-completion.bash | 2 +- > list-objects-filter-options.c | 10 ++-- > list-objects-filter-options.h | 2 - > list-objects-filter.c | 22 -------- > t/t5317-pack-objects-filter-objects.sh | 71 +++++--------------------- > t/t6112-rev-list-filters-objects.sh | 39 +++++--------- > 6 files changed, 33 insertions(+), 113 deletions(-)
What is curious is that this does not touch Documentation/ hierarchy at all---is that a sign that nobody makes any serious use of the --filter=... thing and we can freely drop "features" around it when we see it necessary (like in this case)?
Or do we need something like this on top (or squashed in)? I can live with or without "Note that..." myself.
Thanks.
Documentation/rev-list-options.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index ddbc1de43f..73aafea8d6 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -725,9 +725,6 @@ specification contained in the blob (or blob-expression) '<blob-ish>' to omit blobs that would not be not required for a sparse checkout on the requested refs. + -The form '--filter=sparse:path=<path>' similarly uses a sparse-checkout -specification contained in <path>. -+ The form '--filter=tree:<depth>' omits all blobs and trees whose depth from the root tree is >= <depth> (minimum depth if an object is located at multiple depths in the commits traversed). <depth>=0 will not include @@ -737,6 +734,9 @@ tree and blobs which are referenced directly by a commit reachable from <commit> or an explicitly-given object. <depth>=2 is like <depth>=1 while also including trees and blobs one more level removed from an explicitly-given commit or tree. ++ +Note that the form '--filter=sparse:path=<path>' that wants to read from +an arbitrary path on the filesystem is not supported, for security reasons. --no-filter:: Turn off any previous `--filter=` argument.