From 74152744f0d56c2d0211728206a218a33df41a5d Mon Sep 17 00:00:00 2001 From: Jonathon Mah Date: Sun, 10 Apr 2011 04:10:03 -0700 Subject: [PATCH 01/10] Fix escaping of paths with spaces Signed-off-by: Lars Hjemli --- html.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/html.c b/html.c index a60bc13..6f31097 100644 --- a/html.c +++ b/html.c @@ -18,7 +18,7 @@ static const char* url_escape_table[256] = { "%00", "%01", "%02", "%03", "%04", "%05", "%06", "%07", "%08", "%09", "%0a", "%0b", "%0c", "%0d", "%0e", "%0f", "%10", "%11", "%12", "%13", "%14", "%15", "%16", "%17", "%18", "%19", "%1a", "%1b", "%1c", "%1d", - "%1e", "%1f", "+", 0, "%22", "%23", 0, "%25", "%26", "%27", 0, 0, 0, + "%1e", "%1f", "%20", 0, "%22", "%23", 0, "%25", "%26", "%27", 0, 0, 0, "%2b", 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, "%3c", "%3d", "%3e", "%3f", 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, "%5c", 0, "%5e", 0, "%60", 0, 0, 0, 0, 0, @@ -162,9 +162,9 @@ void html_url_path(const char *txt) while(t && *t){ int c = *t; const char *e = url_escape_table[c]; - if (e && c!='+' && c!='&' && c!='+') { + if (e && c!='+' && c!='&') { html_raw(txt, t - txt); - html_raw(e, 3); + html(e); txt = t+1; } t++; @@ -179,9 +179,11 @@ void html_url_arg(const char *txt) while(t && *t){ int c = *t; const char *e = url_escape_table[c]; + if (c == ' ') + e = "+"; if (e) { html_raw(txt, t - txt); - html_raw(e, strlen(e)); + html(e); txt = t+1; } t++; From afe04daa3380ae144c2c9b486674c98e5dd082a3 Mon Sep 17 00:00:00 2001 From: Lars Hjemli Date: Sun, 22 May 2011 12:21:31 +0200 Subject: [PATCH 02/10] tests/setup.sh: add support for known bugs This patch makes it possible to add tests for known bugs without aborting the testrun. Signed-off-by: Lars Hjemli --- tests/setup.sh | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/setup.sh b/tests/setup.sh index 30f90d5..9f66d89 100755 --- a/tests/setup.sh +++ b/tests/setup.sh @@ -101,6 +101,12 @@ tests_done() run_test() { + bug=0 + if test "$1" = "BUG" + then + bug=1 + shift + fi desc=$1 script=$2 test_count=$(expr $test_count + 1) @@ -109,9 +115,15 @@ run_test() eval "$2" >>test-output.log 2>>test-output.log res=$? printf "test %d: exitcode=%d\n" $test_count $res >>test-output.log - if test $res = 0 + if test $res = 0 -a $bug = 0 then printf " %2d) %-60s [ok]\n" $test_count "$desc" + elif test $res = 0 -a $bug = 1 + then + printf " %2d) %-60s [BUG FIXED]\n" $test_count "$desc" + elif test $bug = 1 + then + printf " %2d) %-60s [KNOWN BUG]\n" $test_count "$desc" else test_failed=$(expr $test_failed + 1) printf " %2d) %-60s [failed]\n" $test_count "$desc" From 084ca50972b4be120eba8d22ce585766ae315c36 Mon Sep 17 00:00:00 2001 From: Lars Hjemli Date: Sun, 22 May 2011 12:22:56 +0200 Subject: [PATCH 03/10] tests: add tests for links with space in path and/or args These tests tries to detect bad links in various pages. On the log page, there currently exists links which are not properly escaped due to the use of cgit_fileurl() when building the link. For now, this bug is simply tagged as such. Signed-off-by: Lars Hjemli --- tests/setup.sh | 11 ++++++++--- tests/t0101-index.sh | 1 + tests/t0103-log.sh | 10 ++++++++++ 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/tests/setup.sh b/tests/setup.sh index 9f66d89..b2f1169 100755 --- a/tests/setup.sh +++ b/tests/setup.sh @@ -20,10 +20,10 @@ mkrepo() { name=$1 count=$2 dir=$PWD - test -d $name && return + test -d "$name" && return printf "Creating testrepo %s\n" $name - mkdir -p $name - cd $name + mkdir -p "$name" + cd "$name" git init n=1 while test $n -le $count @@ -50,6 +50,7 @@ setup_repos() mkrepo trash/repos/foo 5 >/dev/null mkrepo trash/repos/bar 50 >/dev/null mkrepo trash/repos/foo+bar 10 testplus >/dev/null + mkrepo "trash/repos/with space" 2 >/dev/null cat >trash/cgitrc <foo+bar<" trash/tmp' run_test 'verify foo+bar link' 'grep -e "/foo+bar/" trash/tmp' +run_test 'verify "with%20space" link' 'grep -e "/with%20space/" trash/tmp' run_test 'no tree-link' '! grep -e "foo/tree" trash/tmp' run_test 'no log-link' '! grep -e "foo/log" trash/tmp' diff --git a/tests/t0103-log.sh b/tests/t0103-log.sh index b08cd29..def5c18 100755 --- a/tests/t0103-log.sh +++ b/tests/t0103-log.sh @@ -12,4 +12,14 @@ run_test 'generate bar/log' 'cgit_url "bar/log" >trash/tmp' run_test 'find commit 1' 'grep -e "commit 1" trash/tmp' run_test 'find commit 50' 'grep -e "commit 50" trash/tmp' +run_test 'generate "with%20space/log?qt=grep&q=commit+1"' ' + cgit_url "with+space/log&qt=grep&q=commit+1" >trash/tmp +' +run_test 'find commit 1' 'grep -e "commit 1" trash/tmp' +run_test 'find link with %20 in path' 'grep -e "/with%20space/log/?qt=grep" trash/tmp' +run_test 'find link with + in arg' 'grep -e "/log/?qt=grep&q=commit+1" trash/tmp' +run_test BUG 'no links with space in path' '! grep -e "href=./with space/" trash/tmp' +run_test 'no links with space in arg' '! grep -e "q=commit 1" trash/tmp' +run_test 'commit 2 is not visible' '! grep -e "commit 2" trash/tmp' + tests_done From dc1a8eadd4c063fe6782fa99f9db41c46b85d048 Mon Sep 17 00:00:00 2001 From: Lars Hjemli Date: Sun, 22 May 2011 12:45:32 +0200 Subject: [PATCH 04/10] shared.c: do not modify const memory Noticed-by: zhongjj Signed-off-by: Lars Hjemli --- shared.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/shared.c b/shared.c index 7ec2e19..3926b4a 100644 --- a/shared.c +++ b/shared.c @@ -100,23 +100,15 @@ void *cgit_free_commitinfo(struct commitinfo *info) char *trim_end(const char *str, char c) { int len; - char *s, *t; if (str == NULL) return NULL; - t = (char *)str; - len = strlen(t); - while(len > 0 && t[len - 1] == c) + len = strlen(str); + while(len > 0 && str[len - 1] == c) len--; - if (len == 0) return NULL; - - c = t[len]; - t[len] = '\0'; - s = xstrdup(t); - t[len] = c; - return s; + return xstrndup(str, len); } char *strlpart(char *txt, int maxlen) From c9059710e7a1dbd47c22c412e0ba8f591105e3cf Mon Sep 17 00:00:00 2001 From: Lukas Fleischer Date: Wed, 30 Mar 2011 19:17:58 +0200 Subject: [PATCH 05/10] Remove unused variable from cgit_diff_tree(). Seen with "-Wunused-but-set-variable". Signed-off-by: Lukas Fleischer Signed-off-by: Lars Hjemli --- shared.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/shared.c b/shared.c index 3926b4a..3778a5b 100644 --- a/shared.c +++ b/shared.c @@ -303,7 +303,6 @@ void cgit_diff_tree(const unsigned char *old_sha1, filepair_fn fn, const char *prefix, int ignorews) { struct diff_options opt; - int ret; int prefixlen; diff_setup(&opt); @@ -324,9 +323,9 @@ void cgit_diff_tree(const unsigned char *old_sha1, diff_setup_done(&opt); if (old_sha1 && !is_null_sha1(old_sha1)) - ret = diff_tree_sha1(old_sha1, new_sha1, "", &opt); + diff_tree_sha1(old_sha1, new_sha1, "", &opt); else - ret = diff_root_tree_sha1(new_sha1, "", &opt); + diff_root_tree_sha1(new_sha1, "", &opt); diffcore_std(&opt); diff_flush(&opt); } From 070e109c1413d28b54eb6123a9fd24ac98897554 Mon Sep 17 00:00:00 2001 From: Lukas Fleischer Date: Thu, 31 Mar 2011 01:21:39 +0200 Subject: [PATCH 06/10] Fix memory leak in http_parse_querystring(). Signed-off-by: Lukas Fleischer Signed-off-by: Lars Hjemli --- html.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/html.c b/html.c index 6f31097..a0f6db4 100644 --- a/html.c +++ b/html.c @@ -290,12 +290,12 @@ char *convert_query_hexchar(char *txt) int http_parse_querystring(const char *txt_, void (*fn)(const char *name, const char *value)) { - char *t, *txt, *value = NULL, c; + char *o, *t, *txt, *value = NULL, c; if (!txt_) return 0; - t = txt = strdup(txt_); + o = t = txt = strdup(txt_); if (t == NULL) { printf("Out of memory\n"); exit(1); @@ -318,5 +318,6 @@ int http_parse_querystring(const char *txt_, void (*fn)(const char *name, const } if (t!=txt) (*fn)(txt, value); + free(o); return 0; } From a0bf375a1a9b74056a913f3687c6f5b42ad4acf6 Mon Sep 17 00:00:00 2001 From: Lukas Fleischer Date: Tue, 5 Apr 2011 10:35:43 +0200 Subject: [PATCH 07/10] Avoid null pointer dereference in reencode(). Returning "*txt" if "txt" is a null pointer is a bad thing. Spotted with clang-analyzer. Signed-off-by: Lukas Fleischer Signed-off-by: Lars Hjemli --- parsing.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/parsing.c b/parsing.c index c9e4350..151c0fe 100644 --- a/parsing.c +++ b/parsing.c @@ -103,7 +103,10 @@ const char *reencode(char **txt, const char *src_enc, const char *dst_enc) { char *tmp; - if (!txt || !*txt || !src_enc || !dst_enc) + if (!txt) + return NULL; + + if (!*txt || !src_enc || !dst_enc) return *txt; /* no encoding needed if src_enc equals dst_enc */ From 9afc883297b0d0943e9b358d2299950f33e8e5ed Mon Sep 17 00:00:00 2001 From: Lukas Fleischer Date: Tue, 5 Apr 2011 10:38:53 +0200 Subject: [PATCH 08/10] Avoid null pointer dereference in cgit_print_diff(). When calling cgit_print_diff() with a bad new_rev and a NULL old_rev, checking for new_rev's parent commit will result in a null pointer dereference. Returning on an invalid commit before dereferencing fixes this. Spotted with clang-analyzer. Signed-off-by: Lukas Fleischer Signed-off-by: Lars Hjemli --- ui-diff.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ui-diff.c b/ui-diff.c index a7bc667..d21541b 100644 --- a/ui-diff.c +++ b/ui-diff.c @@ -345,8 +345,10 @@ void cgit_print_diff(const char *new_rev, const char *old_rev, const char *prefi return; } commit = lookup_commit_reference(new_rev_sha1); - if (!commit || parse_commit(commit)) + if (!commit || parse_commit(commit)) { cgit_print_error(fmt("Bad commit: %s", sha1_to_hex(new_rev_sha1))); + return; + } if (old_rev) get_sha1(old_rev, old_rev_sha1); @@ -362,8 +364,10 @@ void cgit_print_diff(const char *new_rev, const char *old_rev, const char *prefi return; } commit2 = lookup_commit_reference(old_rev_sha1); - if (!commit2 || parse_commit(commit2)) + if (!commit2 || parse_commit(commit2)) { cgit_print_error(fmt("Bad commit: %s", sha1_to_hex(old_rev_sha1))); + return; + } } if ((ctx.qry.ssdiff && !ctx.cfg.ssdiff) || (!ctx.qry.ssdiff && ctx.cfg.ssdiff)) From c8ea73caabcb16ffb74baa70d35650027ed772c4 Mon Sep 17 00:00:00 2001 From: Lars Hjemli Date: Mon, 23 May 2011 23:10:37 +0200 Subject: [PATCH 09/10] ui-repolist.c: do not return random/stale data from read_agefile When git/date.c:parse_date() cannot parse its input it returns -1. But read_agefile() checks if the result is different from zero, essentialy returning random data from the date buffer when parsing fails. This patch fixes the issue by verifying that the result from parse_date() is positive. Noticed-by: Julius Plenz Signed-off-by: Lars Hjemli --- ui-repolist.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui-repolist.c b/ui-repolist.c index 2c98668..e138f59 100644 --- a/ui-repolist.c +++ b/ui-repolist.c @@ -20,7 +20,7 @@ time_t read_agefile(char *path) if (readfile(path, &buf, &size)) return -1; - if (parse_date(buf, buf2, sizeof(buf2))) + if (parse_date(buf, buf2, sizeof(buf2)) > 0) result = strtoul(buf2, NULL, 10); else result = 0; From ec79265f2053e6dc20e0ec486719f5954d2be83d Mon Sep 17 00:00:00 2001 From: Mark Lodato Date: Fri, 13 May 2011 19:59:07 -0400 Subject: [PATCH 10/10] fix virtual-root if script-name is "" In d0cb841 (Avoid trailing slash in virtual-root), virtual-root was set from script-name using trim_end(). However, if script-name was the empty string (""), which happens when cgit is used to serve the root path on a domain (/), trim_end() returns NULL and cgit acts like virtual-root is not available. Now, set virtual-root to "" in this case, which fixes this bug. Signed-off-by: Lars Hjemli --- cgit.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cgit.c b/cgit.c index e302a7c..e498030 100644 --- a/cgit.c +++ b/cgit.c @@ -757,8 +757,11 @@ int main(int argc, const char **argv) * that virtual-root equals SCRIPT_NAME, minus any possibly * trailing slashes. */ - if (!ctx.cfg.virtual_root) + if (!ctx.cfg.virtual_root && ctx.cfg.script_name) { ctx.cfg.virtual_root = trim_end(ctx.cfg.script_name, '/'); + if (!ctx.cfg.virtual_root) + ctx.cfg.virtual_root = ""; + } /* If no url parameter is specified on the querystring, lets * use PATH_INFO as url. This allows cgit to work with virtual