From eab3746b3026950ed62842c1e5641556d7131a5b Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Sun, 15 Sep 2013 22:23:39 -0700 Subject: [PATCH] More filtering tests including order This adds more tests of filters, including the ident filter when mixed with custom filters. I was able to combine with the reverse filter and demonstrate that the order of filter application with the default priority constants matches the order of core Git. Also, this fixes two issues in the ident filter: preventing ident expansion on binary files and avoiding a NULL dereference when dollar sign characters are found without Id. --- include/git2/sys/filter.h | 14 +++++++++ src/ident.c | 7 ++++- tests-clar/checkout/crlf.c | 36 ++++++++++++++++++--- tests-clar/filter/custom.c | 64 ++++++++++++++++++++++++++++++++++++-- 4 files changed, 114 insertions(+), 7 deletions(-) diff --git a/include/git2/sys/filter.h b/include/git2/sys/filter.h index 9a6720a3e..aa89c7b56 100644 --- a/include/git2/sys/filter.h +++ b/include/git2/sys/filter.h @@ -29,9 +29,23 @@ GIT_EXTERN(git_filter *) git_filter_lookup(const char *name); #define GIT_FILTER_CRLF "crlf" #define GIT_FILTER_IDENT "ident" +/** + * This is priority that the internal CRLF filter will be registered with + */ #define GIT_FILTER_CRLF_PRIORITY 0 + +/** + * This is priority that the internal ident filter will be registered with + */ #define GIT_FILTER_IDENT_PRIORITY 100 +/** + * This is priority to use with a custom filter to imitate a core Git + * filter driver, so that it will be run last on checkout and first on + * checkin. You do not have to use this, but it helps compatibility. + */ +#define GIT_FILTER_DRIVER_PRIORITY 200 + /** * Create a new empty filter list * diff --git a/src/ident.c b/src/ident.c index 3ea949859..23c407f16 100644 --- a/src/ident.c +++ b/src/ident.c @@ -8,6 +8,7 @@ #include "git2/sys/filter.h" #include "filter.h" #include "buffer.h" +#include "buf_text.h" static int ident_find_id( const char **id_start, const char **id_end, const char *start, size_t len) @@ -24,7 +25,7 @@ static int ident_find_id( len = remaining - 1; } - if (len < 3) + if (!found || len < 3) return GIT_ENOTFOUND; *id_start = found; @@ -99,6 +100,10 @@ static int ident_apply( { GIT_UNUSED(self); GIT_UNUSED(payload); + /* Don't filter binary files */ + if (git_buf_text_is_binary(from)) + return GIT_ENOTFOUND; + if (git_filter_source_mode(src) == GIT_FILTER_SMUDGE) return ident_insert_id(to, from, src); else diff --git a/tests-clar/checkout/crlf.c b/tests-clar/checkout/crlf.c index 4953609cc..9a4cbd313 100644 --- a/tests-clar/checkout/crlf.c +++ b/tests-clar/checkout/crlf.c @@ -159,21 +159,30 @@ void test_checkout_crlf__with_ident(void) cl_git_mkfile("crlf/lf.ident", ALL_LF_TEXT_RAW "\n$Id: initial content$\n"); cl_git_mkfile("crlf/crlf.ident", ALL_CRLF_TEXT_RAW "\r\n$Id$\r\n\r\n"); + cl_git_mkfile("crlf/more1.identlf", "$Id$\n" MORE_LF_TEXT_RAW); + cl_git_mkfile("crlf/more2.identcrlf", "\r\n$Id: $\r\n" MORE_CRLF_TEXT_RAW); cl_git_pass(git_repository_index(&index, g_repo)); cl_git_pass(git_index_add_bypath(index, "lf.ident")); cl_git_pass(git_index_add_bypath(index, "crlf.ident")); + cl_git_pass(git_index_add_bypath(index, "more1.identlf")); + cl_git_pass(git_index_add_bypath(index, "more2.identcrlf")); cl_repo_commit_from_index(NULL, g_repo, NULL, 0, "Some ident files\n"); git_checkout_head(g_repo, &opts); - /* check that blob has $Id$ */ + /* check that blobs have $Id$ */ cl_git_pass(git_blob_lookup(&blob, g_repo, & git_index_get_bypath(index, "lf.ident", 0)->oid)); cl_assert_equal_s( ALL_LF_TEXT_RAW "\n$Id$\n", git_blob_rawcontent(blob)); + git_blob_free(blob); + cl_git_pass(git_blob_lookup(&blob, g_repo, + & git_index_get_bypath(index, "more2.identcrlf", 0)->oid)); + cl_assert_equal_s( + "\n$Id$\n" MORE_CRLF_TEXT_AS_LF, git_blob_rawcontent(blob)); git_blob_free(blob); /* check that filesystem is initially untouched - matching core Git */ @@ -184,20 +193,39 @@ void test_checkout_crlf__with_ident(void) /* check that forced checkout rewrites correctly */ p_unlink("crlf/lf.ident"); - p_unlink("crlf/crlflf.ident"); + p_unlink("crlf/crlf.ident"); + p_unlink("crlf/more1.identlf"); + p_unlink("crlf/more2.identcrlf"); git_checkout_head(g_repo, &opts); - if (GIT_EOL_NATIVE == GIT_EOL_LF) + if (GIT_EOL_NATIVE == GIT_EOL_LF) { cl_assert_equal_file( ALL_LF_TEXT_RAW "\n$Id: fcf6d4d9c212dc66563b1171b1cd99953c756467$\n", 0, "crlf/lf.ident"); - else + cl_assert_equal_file( + ALL_CRLF_TEXT_AS_LF + "\n$Id: f2c66ad9b2b5a734d9bf00d5000cc10a62b8a857$\n\n", + 0, "crlf/crlf.ident"); + } else { cl_assert_equal_file( ALL_LF_TEXT_AS_CRLF "\r\n$Id: fcf6d4d9c212dc66563b1171b1cd99953c756467$\r\n", 0, "crlf/lf.ident"); + cl_assert_equal_file( + ALL_CRLF_TEXT_RAW + "\r\n$Id: f2c66ad9b2b5a734d9bf00d5000cc10a62b8a857$\r\n\r\n", + 0, "crlf/crlf.ident"); + } + + cl_assert_equal_file( + "$Id: f7830382dac1f1583422be5530fdfbd26289431b$\n" + MORE_LF_TEXT_AS_LF, 0, "crlf/more1.identlf"); + + cl_assert_equal_file( + "\r\n$Id: 74677a68413012ce8d7e7cfc3f12603df3a3eac4$\r\n" + MORE_CRLF_TEXT_AS_CRLF, 0, "crlf/more2.identcrlf"); git_index_free(index); } diff --git a/tests-clar/filter/custom.c b/tests-clar/filter/custom.c index a2752efa4..d6ad4b7a3 100644 --- a/tests-clar/filter/custom.c +++ b/tests-clar/filter/custom.c @@ -6,8 +6,9 @@ #include "git2/sys/filter.h" #include "git2/sys/repository.h" -#define BITFLIP_FILTER_PRIORITY 20 -#define REVERSE_FILTER_PRIORITY 25 +/* picked these to be >= GIT_FILTER_DRIVER_PRIORITY */ +#define BITFLIP_FILTER_PRIORITY 200 +#define REVERSE_FILTER_PRIORITY 250 #define VERY_SECURE_ENCRYPTION(b) ((b) ^ 0xff) @@ -255,3 +256,62 @@ void test_filter_custom__can_register_a_custom_filter_in_the_repository(void) cl_assert_equal_sz(0, git_filter_list_length(fl)); git_filter_list_free(fl); } + +void test_filter_custom__order_dependency(void) +{ + git_index *index; + git_blob *blob; + git_buf buf = { 0 }; + + /* so if ident and reverse are used together, an interesting thing + * happens - a reversed "$Id$" string is no longer going to trigger + * ident correctly. When checking out, the filters should be applied + * in order CLRF, then ident, then reverse, so ident expansion should + * work correctly. On check in, the content should be reversed, then + * ident, then CRLF filtered. Let's make sure that works... + */ + + cl_git_mkfile( + "empty_standard_repo/.gitattributes", + "hero.*.rev-ident text ident reverse eol=lf\n"); + + cl_git_mkfile( + "empty_standard_repo/hero.1.rev-ident", + "This is a test\n$Id$\nHave fun!\n"); + + cl_git_mkfile( + "empty_standard_repo/hero.2.rev-ident", + "Another test\n$dI$\nCrazy!\n"); + + cl_git_pass(git_repository_index(&index, g_repo)); + cl_git_pass(git_index_add_bypath(index, "hero.1.rev-ident")); + cl_git_pass(git_index_add_bypath(index, "hero.2.rev-ident")); + cl_repo_commit_from_index(NULL, g_repo, NULL, 0, "Filter chains\n"); + git_index_free(index); + + cl_git_pass(git_blob_lookup(&blob, g_repo, + & git_index_get_bypath(index, "hero.1.rev-ident", 0)->oid)); + cl_assert_equal_s( + "\n!nuf evaH\n$dI$\ntset a si sihT", git_blob_rawcontent(blob)); + cl_git_pass(git_blob_filtered_content(&buf, blob, "hero.1.rev-ident", 0)); + /* no expansion because id was reversed at checkin and now at ident + * time, reverse is not applied yet */ + cl_assert_equal_s( + "This is a test\n$Id$\nHave fun!\n", buf.ptr); + git_blob_free(blob); + + cl_git_pass(git_blob_lookup(&blob, g_repo, + & git_index_get_bypath(index, "hero.2.rev-ident", 0)->oid)); + cl_assert_equal_s( + "\n!yzarC\n$Id$\ntset rehtonA", git_blob_rawcontent(blob)); + cl_git_pass(git_blob_filtered_content(&buf, blob, "hero.2.rev-ident", 0)); + /* expansion because reverse was applied at checkin and at ident time, + * reverse is not applied yet */ + cl_assert_equal_s( + "Another test\n$59001fe193103b1016b27027c0c827d036fd0ac8 :dI$\nCrazy!\n", buf.ptr); + cl_assert_equal_i(0, git_oid_strcmp( + git_blob_id(blob), "8ca0df630d728c0c72072b6101b301391ef10095")); + git_blob_free(blob); + + git_buf_free(&buf); +}