Diff options
| -rw-r--r-- | .gitattributes | 3 | |
| -rw-r--r-- | tests/Makefile | 20 | |
| -rwxr-xr-x | tests/setup.sh | 58 | |
| -rwxr-xr-x | tests/t0001-validate-git-versions.sh | 24 | |
| -rwxr-xr-x | tests/t0010-validate-html.sh | 30 | |
| -rwxr-xr-x | tests/t0020-validate-cache.sh | 43 | |
| -rwxr-xr-x | tests/t0101-index.sh | 7 | |
| -rwxr-xr-x | tests/t0102-summary.sh | 7 | |
| -rwxr-xr-x | tests/t0103-log.sh | 6 | |
| -rwxr-xr-x | tests/t0104-tree.sh | 6 | |
| -rwxr-xr-x | tests/t0105-commit.sh | 6 | |
| -rwxr-xr-x | tests/t0106-diff.sh | 32 | |
| -rwxr-xr-x | tests/t0107-snapshot.sh | 7 | |
| -rwxr-xr-x | tests/t0108-patch.sh | 7 | |
| -rwxr-xr-x | tests/t0109-gitconfig.sh | 23 | |
| -rwxr-xr-x | tests/t0110-rawdiff.sh | 9 | |
| -rwxr-xr-x | tests/t0111-filter.sh | 18 | |
| -rwxr-xr-x | tests/t0200-security.sh | 66 | |
| -rwxr-xr-x | tests/t0201-limits.sh | 46 | |
| -rwxr-xr-x | tests/t0202-stats.sh | 17 | |
| -rwxr-xr-x | tests/t0203-dates.sh | 13 |
21 files changed, 313 insertions, 135 deletions
diff --git a/.gitattributes b/.gitattributes index e13769c..319f6e1 100644 --- a/.gitattributes +++ b/.gitattributes @@ -4,6 +4,9 @@ *.c text eol=lf *.h text eol=lf +*.py text eol=lf +*.sh text eol=lf + *.txt text eol=lf Makefile text eol=lf diff --git a/tests/Makefile b/tests/Makefile index e159487..1047de5 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -1,17 +1,29 @@ +# Runs the cgit test suite. Every t[0-9][0-9][0-9][0-9]-*.sh script beside this +# file is a self-contained test built on the test library in the bundled Git +# tree, so all that is needed here is a shell to run each one under and a way +# to pass extra options along. The top-level Makefile reaches this through its +# test target, once cgit and the Git tree it links against have been built. + +# SHELL_PATH comes from Git's platform detection, and cgit.conf may override it, +# so both are read before anything below uses it. include ../vendor/git/config.mak.uname -include ../cgit.conf SHELL_PATH ?= $(SHELL) + +# Escaped for the single quotes that the recipe below puts around the path. SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH)) -T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh) +TESTS = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh) -all: $(T) +all: $(TESTS) -$(T): +$(TESTS): @'$(SHELL_PATH_SQ)' $@ $(CGIT_TEST_OPTS) clean: $(RM) -r 'trash directory'.* test-results -.PHONY: all $(T) clean +# Each test is named after a file that already exists, so without this make +# would decide there is nothing to do and run none of them. +.PHONY: all $(TESTS) clean diff --git a/tests/setup.sh b/tests/setup.sh index 39819e4..93a663a 100755 --- a/tests/setup.sh +++ b/tests/setup.sh @@ -1,24 +1,15 @@ -# This file should be sourced by all test-scripts -# -# Main functions: -# prepare_tests(description) - setup for testing, i.e. create repos+config -# run_test(description, script) - run one test, i.e. eval script -# -# Helper functions -# cgit_query(querystring) - call cgit with the specified querystring -# cgit_url(url) - call cgit with the specified virtual url -# -# Example script: -# -# . setup.sh -# prepare_tests "html validation" -# run_test 'repo index' 'cgit_url "/" | tidy -e' -# run_test 'repo summary' 'cgit_url "/foo" | tidy -e' - -# We don't want to run Git commands through Valgrind, so we filter out the -# --valgrind option here and handle it ourselves. We copy the arguments -# assuming that none contain a newline, although other whitespace is -# preserved. +# Shared groundwork sourced by every test script. It builds a handful of small +# repositories under repos/ and writes a cgitrc pointing at them, then offers +# the helpers that ask cgit for a page so a test only has to look at what comes +# back. Sourcing it also brings in the test library from the bundled Git tree, +# which is where test_expect_success and the trash directory come from. Set +# CGIT_TEST_NO_CREATE_REPOS to get the helpers without paying for the fixtures. + +# The Git test library would run every Git command under Valgrind if it saw +# --valgrind, and only cgit itself is worth watching, so the option is taken out +# here and acted on further down. The arguments are carried across as a newline +# separated list, which keeps any other whitespace in them intact but assumes +# that none holds a newline of its own. LF=' ' test_argv= @@ -44,10 +35,13 @@ IFS=$OLDIFS : ${TEST_DIRECTORY=$(pwd)/../vendor/git/t} : ${TEST_OUTPUT_DIRECTORY=$(pwd)} +# The library would otherwise leave a repository of its own in the trash +# directory, and every repository these tests want is built by mkrepo below. TEST_NO_CREATE_REPO=YesPlease . "$TEST_DIRECTORY"/test-lib.sh -# Prepend the directory containing cgit to PATH. +# The tests run cgit by name, so the binary just built has to come ahead of any +# copy already installed. Under Valgrind the wrappers take that place instead. if test -n "$cgit_valgrind" then GIT_VALGRIND="$TEST_DIRECTORY/valgrind" @@ -60,27 +54,33 @@ fi FILTER_DIRECTORY=$(cd ../filters && pwd) +# Lua filters only work when cgit was compiled with Lua, so the tests that use +# them have to ask the binary rather than assume. if cgit --version | grep -F -q "[+] Lua scripting"; then export CGIT_HAS_LUA=1 else export CGIT_HAS_LUA=0 fi +# Creates a repository holding the given number of one file commits. A third +# argument asks for a variant, either testplus for a file and a branch whose +# names have to be escaped in a URL, or commit-graph for a written commit graph. mkrepo() { - name=$1 - count=$2 - test_create_repo "$name" + repo=$1 + commits=$2 + variant=$3 + test_create_repo "$repo" ( - cd "$name" + cd "$repo" n=1 - while test $n -le $count + while test $n -le $commits do echo $n >file-$n git add file-$n git commit -m "commit $n" n=$(expr $n + 1) done - case "$3" in + case "$variant" in testplus) echo "hello" >a+b git add a+b @@ -170,6 +170,8 @@ cgit_url() CGIT_CONFIG="$PWD/cgitrc" QUERY_STRING="url=$1" cgit } +# The response headers run down to the first empty line, and a test comparing +# two pages wants to see the bodies alone. strip_headers() { while read -r line do diff --git a/tests/t0001-validate-git-versions.sh b/tests/t0001-validate-git-versions.sh index 65c3c6a..2117764 100755 --- a/tests/t0001-validate-git-versions.sh +++ b/tests/t0001-validate-git-versions.sh @@ -1,5 +1,12 @@ #!/bin/sh +# Checks that the Git version cgit says it is built for is the one it is +# actually built against. The top-level Makefile names a version, the tree +# under vendor/git records its own, and the submodule is pinned to a tag, so +# all three have to agree or cgit is being built on something other than what +# it claims. Set CGIT_TEST_NO_GIT_VERSION to YesPlease when Git comes from +# elsewhere and the comparison has nothing to say. + if [ "${CGIT_TEST_NO_GIT_VERSION}" = "YesPlease" ]; then exit 0 fi @@ -15,10 +22,9 @@ test_expect_success 'extract Git version from Makefile' ' }" ../../Makefile >makefile_version ' -# Note that Git's GIT-VERSION-GEN script applies "s/-/./g" to the version -# string to produce the internal version in the GIT-VERSION-FILE, so we -# must apply the same transformation to the version in the Makefile before -# comparing them. +# Git's GIT-VERSION-GEN script applies "s/-/./g" to the version string on its +# way into the GIT-VERSION-FILE, so the same has to be done to the version in +# the Makefile before the two can be compared. test_expect_success 'test Git version matches Makefile' ' ( cat ../../vendor/git/GIT-VERSION-FILE || echo "No GIT-VERSION-FILE" ) | sed -e "s/GIT_VERSION[ ]*=[ ]*//" -e "s/\\.dirty$//" >git_version && @@ -26,6 +32,8 @@ test_expect_success 'test Git version matches Makefile' ' test_cmp git_version makefile_git_version ' +# A tree unpacked from a tarball has no submodule pin to describe, so there is +# nothing to compare against and the test says so rather than failing. test_expect_success 'test submodule version matches Makefile' ' if ! test -e ../../vendor/git/.git then @@ -33,12 +41,12 @@ test_expect_success 'test submodule version matches Makefile' ' else ( cd ../.. && - sm_oid=$(git ls-files --stage -- vendor/git | + submodule_oid=$(git ls-files --stage -- vendor/git | sed -e "s/^[0-9]* \\([0-9a-f]*\\) [0-9] .*$/\\1/") && cd vendor/git && - git describe --match "v[0-9]*" $sm_oid - ) | sed -e "s/^v//" -e "s/-/./" >sm_version && - test_cmp sm_version makefile_version + git describe --match "v[0-9]*" $submodule_oid + ) | sed -e "s/^v//" -e "s/-/./" >submodule_version && + test_cmp submodule_version makefile_version fi ' diff --git a/tests/t0010-validate-html.sh b/tests/t0010-validate-html.sh index 91433f8..308cef7 100755 --- a/tests/t0010-validate-html.sh +++ b/tests/t0010-validate-html.sh @@ -1,20 +1,30 @@ #!/bin/sh +# Runs the tidy checker over one page of each kind cgit renders, so that markup +# broken enough to confuse a browser is caught here rather than in the browser. +# Only the shape of the markup matters, since what the pages actually say is +# the business of the t01xx scripts. Tidy is optional and old versions of it +# predate the elements cgit uses, so the whole file steps aside when a usable +# one cannot be found. + test_description='Validate html with tidy' . ./setup.sh - test_url() { - tidy_opt="-eq" - test -z "$NO_TIDY_WARNINGS" || tidy_opt+=" --show-warnings no" + tidy_options="-eq" + test -z "$NO_TIDY_WARNINGS" || + tidy_options="$tidy_options --show-warnings no" cgit_url "$1" >tidy-$test_count.tmp || return + # Tidy reads what it is given as a whole document, so the response + # headers are dropped before it sees the page. sed -e "1,4d" tidy-$test_count.tmp >tidy-$test_count || return - "$tidy" $tidy_opt tidy-$test_count - rc=$? + "$tidy" $tidy_options tidy-$test_count + status=$? - # tidy returns with exitcode 1 on warnings, 2 on error - if test $rc = 2 + # Tidy leaves with 1 for warnings and 2 for errors, and only an error is + # worth failing the test over. + if test $status = 2 then false else @@ -22,15 +32,15 @@ test_url() fi } -tidy=`which tidy 2>/dev/null` +tidy=$(which tidy 2>/dev/null) test -n "$tidy" || { skip_all='Skipping html validation tests: tidy not found' test_done exit } -# Releases of tidy that predate HTML5 reject the semantic elements (header, -# nav, main, footer) that cgit emits, reporting them as errors. Skip the +# Releases of tidy that predate HTML5 reject the semantic elements cgit emits, +# header, nav, main and footer among them, reporting each as an error. Skip the # validation unless tidy is new enough to understand them. printf '<!DOCTYPE html>\n<html lang="en"><head><title>t</title></head><body><header>x</header></body></html>\n' \ | "$tidy" -q -e >/dev/null 2>&1 diff --git a/tests/t0020-validate-cache.sh b/tests/t0020-validate-cache.sh index 510e51c..0cf7433 100755 --- a/tests/t0020-validate-cache.sh +++ b/tests/t0020-validate-cache.sh @@ -1,5 +1,12 @@ #!/bin/sh +# Exercises the cache, which keeps a rendered page in a slot on disk and +# replays it for the next request that asks for the same thing. The first three +# tests set cache-size to nothing, to one slot and to the full table in turn, +# then count what the requests left behind. The rest cover the two ways a slot +# can come out wrong, a page longer than the output buffer and a key too long +# to be read back. + test_description='Validate cache' . ./setup.sh @@ -70,15 +77,14 @@ test_expect_success 'verify cache-size=1021' ' cgit_url "foo/ls_cache" >output.full && strip_headers <output.full >output && test_line_count = 13 output && - # Check that ls_cache output is cached correctly cgit_url "foo/ls_cache" >output.second && test_cmp output.full output.second ' -# --- A cached page must hold everything the uncached one produced ----------- -# Page output is buffered, so the slot is only complete if the buffer is -# emptied before the generated content is measured. The blob here is larger -# than one buffer, so a missing flush would truncate the cached copy. +# Page output is buffered, so a slot only holds the whole page if the buffer is +# emptied before the size of what was generated is taken. The blob set up here +# is larger than one buffer, so a missing flush would leave the cached copy cut +# short. test_expect_success 'set up a repo with a page larger than the output buffer' ' mkrepo repos/bigpage 1 && ( @@ -98,12 +104,15 @@ test_expect_success 'set up a repo with a page larger than the output buffer' ' rm -rf cache2 && mkdir cache2 ' -bigq() { CGIT_CONFIG="$PWD/bigrc" QUERY_STRING="$1" cgit; } +bigpage_query() +{ + CGIT_CONFIG="$PWD/bigrc" QUERY_STRING="$1" cgit +} test_expect_success 'the first request fills the slot and the second replays it' ' - bigq "url=bigpage/tree/big.txt" >big.first && + bigpage_query "url=bigpage/tree/big.txt" >big.first && test $(wc -c <big.first) -gt 65536 && - bigq "url=bigpage/tree/big.txt" >big.second && + bigpage_query "url=bigpage/tree/big.txt" >big.second && strip_headers <big.first >big.first.body && strip_headers <big.second >big.second.body && test_cmp big.first.body big.second.body @@ -113,21 +122,21 @@ test_expect_success 'the cached body matches one generated with the cache off' ' sed -e "s/^cache-size=64$/cache-size=0/" bigrc >bignocache && CGIT_CONFIG="$PWD/bignocache" QUERY_STRING="url=bigpage/tree/big.txt" cgit >big.nocache && strip_headers <big.nocache >big.nocache.body && - # The footer carries the time the page was generated, which differs - # between the two runs by construction. - sed -e "s/generated by .*//" big.nocache.body >big.a && - sed -e "s/generated by .*//" big.second.body >big.b && - test_cmp big.a big.b + # The footer carries the time the page was generated, which the two runs + # cannot agree on. + sed -e "s/generated by .*//" big.nocache.body >big.nocache.timeless && + sed -e "s/generated by .*//" big.second.body >big.cached.timeless && + test_cmp big.nocache.timeless big.cached.timeless ' test_expect_success 'the page ends where it should, so nothing was dropped' ' tail -c 200 big.second.body | grep "</html>" ' -# --- A key too long to read back must not claim a slot ---------------------- -# A slot stores its key ahead of the content and only the first few kilobytes -# are read back, so a longer key could never match. Such a request used to -# write a slot on every visit that no later request could ever use. +# A slot stores its key ahead of the content and only the first few kilobytes of +# it are read back, so a key longer than that could never match. A request +# carrying one used to leave a slot behind on every visit that nothing could +# ever go on to use. test_expect_success 'an over-long cache key leaves no slot behind' ' rm -rf cache3 && mkdir cache3 && sed -e "s|^cache-root=.*|cache-root=$PWD/cache3|" bigrc >bigkeyrc && diff --git a/tests/t0101-index.sh b/tests/t0101-index.sh index 82ef9b0..be382ad 100755 --- a/tests/t0101-index.sh +++ b/tests/t0101-index.sh @@ -1,5 +1,12 @@ #!/bin/sh +# The index page is what cgit serves at the root of a site, one row for every +# repository named in cgitrc. These checks look for each repository the shared +# setup builds along with its description, and for the escaping a name that +# contains a plus or a space needs before it can go into a link. They also +# confirm the index stays a plain list, without the tree and log links that +# belong to a repository's own pages. + test_description='Check content on index page' . ./setup.sh diff --git a/tests/t0102-summary.sh b/tests/t0102-summary.sh index f1c1e78..a953ce3 100755 --- a/tests/t0102-summary.sh +++ b/tests/t0102-summary.sh @@ -1,5 +1,12 @@ #!/bin/sh +# The summary page is the landing page for a single repository, holding its +# most recent commits together with its branches and its tags. The shared +# setup caps that log at five entries, so the checks against the fifty commit +# repository are really checking that the cut lands where it should. They also +# confirm the clone url template from cgitrc has had the repository name +# substituted into it. + test_description='Check content on summary page' . ./setup.sh diff --git a/tests/t0103-log.sh b/tests/t0103-log.sh index bdf1435..41b9769 100755 --- a/tests/t0103-log.sh +++ b/tests/t0103-log.sh @@ -1,5 +1,11 @@ #!/bin/sh +# The log page lists the commits on a branch and can narrow that list down +# with a search. The searching checks run against the repository whose name +# contains a space and search for a term containing a space, so every link +# cgit writes back out has to escape both the path it points at and the query +# it carries, and the two are escaped differently. + test_description='Check content on log page' . ./setup.sh diff --git a/tests/t0104-tree.sh b/tests/t0104-tree.sh index 2e140f5..c5c5c4c 100755 --- a/tests/t0104-tree.sh +++ b/tests/t0104-tree.sh @@ -1,5 +1,11 @@ #!/bin/sh +# The tree page browses a repository at one revision, either as a listing of a +# directory or as a single file with an anchor on every line. The checks +# against the repository named foo+bar cover a file name and a branch name +# that both contain a plus, which cgit has to spell one way inside a path and +# another way inside a query. + test_description='Check content on tree page' . ./setup.sh diff --git a/tests/t0105-commit.sh b/tests/t0105-commit.sh index 1a12ee3..47edc5f 100755 --- a/tests/t0105-commit.sh +++ b/tests/t0105-commit.sh @@ -1,5 +1,11 @@ #!/bin/sh +# The commit page shows a single commit, its message, the files it touched and +# the diff for them. Most of these checks read the markup cgit emits for the +# tree link, the parent link, the subject and the diffstat. The last few ask +# for the root commit of a repository, which has no parent and so goes through +# the code that compares a commit against an empty tree. + test_description='Check content on commit page' . ./setup.sh diff --git a/tests/t0106-diff.sh b/tests/t0106-diff.sh index 4074276..175f609 100755 --- a/tests/t0106-diff.sh +++ b/tests/t0106-diff.sh @@ -1,5 +1,12 @@ #!/bin/sh +# The diff page renders the change a commit made, either as unified text or, +# when dt=1 asks for it, as a side by side table. The first checks read the +# markup for one added file in the repository the shared setup builds. The +# rest build small repositories of their own, shaped so the side by side +# renderer meets the two cases it used to get wrong, a hunk covering a single +# line and a file where every line changed. + test_description='Check content on diff page' . ./setup.sh @@ -16,10 +23,10 @@ test_expect_success 'find added line' ' grep "<div class=.add.>+5</div>" tmp ' -# --- Side-by-side line numbers for a hunk that covers a single line --------- -# git writes the length only when a hunk spans more than one line, so a header -# reads "@@ -1 +1 @@". Requiring a comma after the number left those hunks -# numbered from zero, which dropped the line-number link entirely. +# git writes a hunk length only when the hunk spans more than one line, so the +# header for a single line reads "@@ -1 +1 @@" with no comma in it. Requiring +# that comma left those hunks numbered from zero, which dropped the line +# number link altogether. test_expect_success 'set up a repo with single-line hunks' ' mkrepo repos/hunk1 1 && ( @@ -51,10 +58,11 @@ test_expect_success 'ssdiff still numbers a hunk that carries a length' ' grep "#n1.>1</a>" tmp ' -# --- Intra-line highlighting across many changed pairs ---------------------- -# The character-level highlight reuses one table across every changed pair. -# Lines shrink down the file so a short comparison always follows a longer one, -# which is where a stale cell would show up as misplaced del and add spans. +# The character level highlight reuses one table across every changed pair, so +# these lines get shorter as the file goes on and each comparison is smaller +# than the one before it. That is where a cell left over from the previous +# pair shows itself, as a del or an add span sitting over the wrong +# characters. test_expect_success 'set up a repo with shrinking changed lines' ' mkrepo repos/lcs 1 && ( @@ -78,10 +86,10 @@ test_expect_success 'set up a repo with shrinking changed lines' ' test_expect_success 'every changed pair gets both a del and an add span' ' CGIT_CONFIG="$PWD/lcsrc" QUERY_STRING="url=lcs/diff/&dt=1" cgit >tmp && - dels=$(grep -o "<span class=.del.>" tmp | wc -l) && - adds=$(grep -o "<span class=.add.>" tmp | wc -l) && - test "$dels" -gt 0 && - test "$dels" -eq "$adds" + deletions=$(grep -o "<span class=.del.>" tmp | wc -l) && + additions=$(grep -o "<span class=.add.>" tmp | wc -l) && + test "$deletions" -gt 0 && + test "$deletions" -eq "$additions" ' test_expect_success 'stripping the highlight leaves every line intact' ' diff --git a/tests/t0107-snapshot.sh b/tests/t0107-snapshot.sh index 0811ec4..0762ab2 100755 --- a/tests/t0107-snapshot.sh +++ b/tests/t0107-snapshot.sh @@ -1,5 +1,12 @@ #!/bin/sh +# The snapshot page hands a branch back as a compressed archive. Most of the +# formats the shared setup enables are fetched here, then their headers are +# read, their bodies unpacked and the files inside compared against the +# repository that setup built. A format needing a helper program that is not +# always installed sits behind a prerequisite, so a machine without lzip, xz, +# zstd or unzip skips those checks instead of failing them. + test_description='Verify snapshot' . ./setup.sh diff --git a/tests/t0108-patch.sh b/tests/t0108-patch.sh index ffcba96..3d38e55 100755 --- a/tests/t0108-patch.sh +++ b/tests/t0108-patch.sh @@ -1,5 +1,12 @@ #!/bin/sh +# The patch page serves a commit as a mail ready patch, so that a change read +# from cgit can be fed straight to git am. The checks compare that output +# against git format-patch for a single commit and for a range of them, once +# the CGI headers have been stripped, which means the two have to agree line +# for line. The last one sets max-patch-count so a range wider than the limit +# comes back cut short. + test_description='Check content on patch page' . ./setup.sh diff --git a/tests/t0109-gitconfig.sh b/tests/t0109-gitconfig.sh index 189ef28..ff4dbb3 100755 --- a/tests/t0109-gitconfig.sh +++ b/tests/t0109-gitconfig.sh @@ -1,8 +1,16 @@ #!/bin/sh +# Guards the promise that cgit reads nothing out of the home directory of +# whichever account the web server happens to run as. Every page is fetched +# under strace with HOME pointed at a path that is known not to exist, and +# the run fails if any access call names that path, which is how a stray +# read of a personal gitconfig would show up. + test_description='Ensure that git does not access $HOME' . ./setup.sh +# strace needs ptrace, which a system can refuse even where the binary is +# installed, so a working run is checked as well as a present binary. test -n "$(which strace 2>/dev/null)" || { skip_all='Skipping access validation tests: strace not found' test_done @@ -16,16 +24,21 @@ strace true 2>/dev/null || { } test_no_home_access () { - non_existent_path="/path/to/some/place/that/does/not/possibly/exist" - while test -d "$non_existent_path"; do - non_existent_path="$non_existent_path/$(date +%N)" + # A home that happened to exist would be one git may legitimately + # read, leaving the check below with nothing to catch, so extend the + # path until nothing is there. + missing_home="/path/to/some/place/that/does/not/possibly/exist" + depth=0 + while test -d "$missing_home"; do + depth=$((depth + 1)) + missing_home="$missing_home/$depth" done && strace \ - -E HOME="$non_existent_path" \ + -E HOME="$missing_home" \ -E CGIT_CONFIG="$PWD/cgitrc" \ -E QUERY_STRING="url=$1" \ -e access -f -o strace.out cgit && - ! grep "$non_existent_path" strace.out + ! grep "$missing_home" strace.out } test_no_home_access_success() { diff --git a/tests/t0110-rawdiff.sh b/tests/t0110-rawdiff.sh index 66fa7d5..23d37a5 100755 --- a/tests/t0110-rawdiff.sh +++ b/tests/t0110-rawdiff.sh @@ -1,5 +1,11 @@ #!/bin/sh +# Checks the rawdiff page, which serves a diff as plain text with no markup +# around it so that the result can be fed straight to patch. Every test runs +# the git command the page is meant to mirror and compares the two byte for +# byte, covering an ordinary commit, the initial commit that has no parent, +# and a range spanning several commits. + test_description='Check content on rawdiff page' . ./setup.sh @@ -7,6 +13,9 @@ test_expect_success 'generate foo/rawdiff' ' cgit_query "url=foo/rawdiff" >tmp ' +# cgit writes its CGI headers and the blank line that ends them ahead of the +# body while git writes only the diff, so the head of the page comes off +# before the two are compared. test_expect_success 'compare with output of git-diff(1)' ' git --git-dir="$PWD/repos/foo/.git" diff HEAD^.. >tmp2 && sed "1,4d" tmp >tmp_ && diff --git a/tests/t0111-filter.sh b/tests/t0111-filter.sh index da4172f..88533f4 100755 --- a/tests/t0111-filter.sh +++ b/tests/t0111-filter.sh @@ -1,5 +1,11 @@ #!/bin/sh +# Checks the source, about, commit and email filters, which hand a piece of +# page content to an outside program before it is written out. The whole +# body runs once per kind of filter, the exec kind always and the lua kind +# only where cgit was built with lua, because both have to produce the same +# page from the same repository. + test_description='Check filtered content' . ./setup.sh @@ -42,11 +48,13 @@ do grep "<committer@example.com> commit C O MITTER <COMMITTER@EXAMPLE.COM>" tmp ' - # Page output is buffered, so anything written before a filter opens has - # to leave the buffer before the filter takes over stdout, and anything - # written while it is open has to leave before stdout is handed back. - # A missed flush reorders the page rather than losing it, so check that - # the markup around the filtered text is still on the right side of it. + # Page output is buffered, so whatever was written before a filter + # opens has to leave the buffer before the filter takes over stdout, + # and whatever was written while it was open has to leave before + # stdout is handed back, since those bytes are meant for the filter. + # A missed flush reorders the page rather than losing any of it, + # so the two tests below confirm that the markup around the filtered + # text is still on the side of it that it belongs on. test_expect_success "the $prefix source filter output stays inside its cell" " cgit_url 'filter-$prefix/tree/a%2bb' >tmp && tr -d '\n' <tmp >flat.out && diff --git a/tests/t0200-security.sh b/tests/t0200-security.sh index 9238262..f475301 100755 --- a/tests/t0200-security.sh +++ b/tests/t0200-security.sh @@ -1,10 +1,19 @@ #!/bin/sh +# Collects the regression tests for the security fixes and for the behaviour +# this fork adds on top of upstream cgit. Each case builds the smallest +# repository and config that reproduce the original problem and then asks for +# the page that used to mishandle it. The comment above a case says what the +# page is being defended against, because a request that looks ordinary is +# usually the whole point of the attack. + test_description='Check security fixes and fork-specific behavior' . ./setup.sh -# A repo with an oversized blob, readmes that carry markup, and a directory, -# plus a config that pins a tiny blob limit and groups directories. +# Most of what follows shares one repository and one config, so the fixture +# carries everything they need at once, a blob over the size limit, readmes +# holding markup that must not reach the page as markup, and a subdirectory +# to sort ahead of the files. test_expect_success 'set up security fixtures' ' mkrepo repos/sec 1 && ( @@ -31,9 +40,9 @@ test_expect_success 'set up security fixtures' ' secq() { CGIT_CONFIG="$PWD/seccgitrc" QUERY_STRING="$1" cgit; } -# --- Argument injection through the log id= parameter ----------------------- -# A tip beginning with a dash would be parsed as a git option, and -# id=--output=<path> would create or truncate an arbitrary file. +# A revision beginning with a dash reaches git as an option rather than as a +# tip, so a request for id=--output=<path> could create or truncate any file +# the server is able to write. test_expect_success 'log id=--output does not write a file' ' rm -f pwned && cgit_query "url=foo/log&id=--output=$PWD/pwned" >tmp 2>&1 && @@ -55,7 +64,9 @@ test_expect_success 'a valid id= still renders the log' ' grep -i "commit 5" tmp ' -# --- max-blob-size is enforced before the object is read -------------------- +# max-blob-size is enforced before the object is read, so every view that +# would otherwise inline a file has to turn the same one away rather than +# inflate it first and think better of it afterwards. test_expect_success 'tree view refuses an oversized blob' ' secq "url=sec/tree/big.txt" | grep -iE "exceeds|too large" ' @@ -72,7 +83,9 @@ test_expect_success 'a small blob is still served' ' secq "url=sec/plain/afile" | grep -F "top" ' -# --- Readme rendering escapes untrusted repository content ------------------ +# A readme is repository content, so with no about filter configured it has +# to reach the page escaped instead of as live markup, whatever its name +# suggests about the format. test_expect_success 'markdown readme without a filter is escaped as plain text' ' { echo "virtual-root=/" && @@ -104,9 +117,9 @@ test_expect_success 'non-markdown readme keeps its line structure' ' grep "pre class=.plaintext." tmp ' -# --- Auto-submitting selects carry no inline handlers ------------------------ -# A Content-Security-Policy without unsafe-inline blocks inline onchange -# handlers, so the forms mark their selects and cgit.js wires them up. +# A Content-Security-Policy without unsafe-inline stops an inline onchange +# handler from ever running, so the option forms only mark their selects and +# cgit.js wires the submit up from outside the page. test_expect_success 'diff option selects use the autosubmit marker' ' sha=$(git -C repos/foo rev-parse HEAD) && cgit_query "url=foo/commit&id=$sha" >tmp && @@ -114,7 +127,8 @@ test_expect_success 'diff option selects use the autosubmit marker' ' ! grep "onchange" tmp ' -# --- Fork feature: directories are grouped before files in the tree --------- +# Grouping directories ahead of files is behaviour this fork adds, so nothing +# upstream covers it. test_expect_success 'tree groups directories before files' ' secq "url=sec/tree/" >tmp && dirline=$(grep -n "tree/zsub" tmp | head -1 | cut -d: -f1) && @@ -124,9 +138,9 @@ test_expect_success 'tree groups directories before files' ' test "$dirline" -lt "$fileline" ' -# --- Side-by-side diff percent-encodes a file path into its links ----------- -# A file name is repository content and may contain a quote, which would -# otherwise break out of the href attribute of the line-number links. +# A file name is repository content and may hold a quote, which would break +# out of the href attribute on the line number links of a side by side diff, +# so the path is percent-encoded on its way into them. test_expect_success 'ssdiff percent-encodes a quoted file path' ' mkrepo repos/xss 1 && name=$(printf "x\047y.txt") && @@ -150,7 +164,8 @@ test_expect_success 'ssdiff percent-encodes a quoted file path' ' ! grep "href=.[^>]*x.y.txt.[^>]*>" tmp ' -# --- A commit with no message must not crash the history views -------------- +# git itself will write a commit with an empty message, so the log and the +# summary both have to have something to print where the subject goes. test_expect_success 'a message-less commit renders without crashing' ' mkrepo repos/nomsg 1 && ( @@ -172,12 +187,12 @@ test_expect_success 'a message-less commit renders without crashing' ' grep "no commit message" tmp ' -# --- A branch need not point at a commit ------------------------------------ -# struct refinfo keeps taginfo and commitinfo in a union and fills only the one -# matching the object type. Sorting branches by reading the commit member read -# past the end of the smaller taginfo, and read NULL for a tree, which crashed. -# git update-ref refuses to write these, so the refs go in as loose files: a -# repository is just files on disk and cgit reads whatever is there. +# A branch need not point at a commit. struct refinfo keeps taginfo and +# commitinfo in a union and fills only the member matching the object type, +# so sorting branches through the commit member read past the end of the +# smaller taginfo, and read NULL for a tree, which crashed. git update-ref +# refuses to create such a ref, hence the loose files written by hand below, +# and a repository is only files on disk so cgit meets whatever is there. test_expect_success 'set up a repo whose branches point at odd objects' ' mkrepo repos/oddref 2 && ( @@ -229,10 +244,11 @@ test_expect_success 'name-sorted branches are unaffected' ' grep "</html>" tmp ' -# --- A repository cannot supply a printf format string ---------------------- -# module-link is a template, and scan-path lets a repository set it through its -# own cgitrc. Handing it to printf let a repo owner crash the process, or read -# stack memory into the served page, with surplus conversions. +# module-link is a template and scan-path lets a repository set its own from +# a cgitrc in the tree, so handing that string to printf let the owner of a +# scanned repository crash the process, or read stack memory into the served +# page, merely by adding conversions past the two that are filled. The tests +# after the fixture also pin down the templates that must keep working. test_expect_success 'set up a submodule fixture with a hostile module-link' ' mkrepo repos/modlink 1 && ( diff --git a/tests/t0201-limits.sh b/tests/t0201-limits.sh index 72d8a23..f09c290 100755 --- a/tests/t0201-limits.sh +++ b/tests/t0201-limits.sh @@ -1,5 +1,14 @@ #!/bin/sh +# Checks the ceilings a config can put on a page, that is the caps on refs +# listed, on the lines and the files a diff renders inline, and on the size +# of a blob any view will inflate, along with the clamp on how long an index +# query may be. Crossing one of these has to trim the page or point the +# reader at somewhere better suited, never drop the request, so each case +# watches what survives as closely as what is left out. The last case is the +# same idea applied to a filter, which degrades to escaped text rather than +# failing when its highlighting library is absent. + test_description='Check the ref listing and diff size limits' . ./setup.sh @@ -7,8 +16,10 @@ if [ $CGIT_HAS_LUA -eq 1 ]; then test_set_prereq LUA fi -# A repo with several branches and tags, one commit with an oversized file -# diff beside a small one, and configs that pin tiny limits. +# The limits only show themselves against content that crosses them, so the +# fixture carries more branches and tags than max-ref-count allows and a +# commit whose oversized file diff sits beside a small one, which is what +# tells a per file limit apart from a whole page one. test_expect_success 'set up limit fixtures' ' mkrepo repos/limits 1 && ( @@ -43,10 +54,10 @@ test_expect_success 'set up limit fixtures' ' limitq() { CGIT_CONFIG="$PWD/limitrc" QUERY_STRING="$1" cgit; } -# --- A request cannot ask for an unbounded amount of matching --------------- -# The query is compared against every repository the index lists, so an -# enormous one would multiply out across the whole listing. It is clamped -# rather than rejected, so an ordinary query still narrows the page. +# An index query is compared against every repository the listing holds, so +# an enormous one multiplies out across the whole index. Clamping it rather +# than refusing it keeps the cost bounded without making an ordinary search +# behave any differently. test_expect_success 'an enormous query is clamped, not rejected' ' long=$(awk "BEGIN{s=\"\";for(i=0;i<4000;i++)s=s \"a\"; print s}") && test ${#long} -eq 4000 && @@ -62,7 +73,9 @@ test_expect_success 'an ordinary query still filters the index' ' ! grep ">bar<" tmp ' -# --- The refs page caps each section and links to the category pages -------- +# The combined refs page caps branches and tags separately, and the pages it +# hands the overflow to page on their own, so a limit that leaked between the +# two sections would show up as the wrong link or the wrong count here. test_expect_success 'refs page lists max-ref-count branches and tags' ' limitq "url=limits/refs/" >tmp && test $(grep -c "log/?h=" tmp) -eq 2 && @@ -90,7 +103,10 @@ test_expect_success 'tag page paginates independently' ' ! grep "log/?h=" tmp ' -# --- Oversized file diffs link out instead of rendering inline -------------- +# A file that busts max-diff-lines is replaced by a link to its own page, +# where the reader asked for that one file and the limit no longer applies. +# The rest of the commit still renders, so one huge file cannot hide the +# small change beside it. test_expect_success 'oversized file diff is replaced by a link' ' limitq "url=limits/commit/" >tmp && grep "too large to be rendered inline" tmp && @@ -107,7 +123,9 @@ test_expect_success 'the single-file diff page always renders in full' ' ! grep "too large to be rendered inline" tmp ' -# --- Commits over max-diff-files fall back to the diffstat ------------------ +# max-diff-files counts the files in a commit rather than the lines in one, +# so it drops the whole body back to the diffstat while a request naming a +# single file stays unaffected. test_expect_success 'a commit changing too many files shows stat only' ' CGIT_CONFIG="$PWD/limitfilesrc" QUERY_STRING="url=limits/commit/" cgit >tmp && grep "too large to be rendered inline" tmp && @@ -119,9 +137,9 @@ test_expect_success 'the single-file page is not limited by max-diff-files' ' grep "class=.hunk." tmp ' -# --- max-blob-size also bounds the diff path -------------------------------- -# The diff family must not inflate a blob larger than max-blob-size just to -# render it. Such a file is reported as binary instead of inlined. +# The diff views must not inflate a blob past max-blob-size merely to render +# it, which is the same defence the tree and plain views make and is easy to +# miss on this path. Such a file is reported as binary instead. test_expect_success 'a diff of an oversized blob is not inlined' ' mkrepo repos/blobdiff 1 && ( @@ -145,7 +163,9 @@ test_expect_success 'a diff of an oversized blob is not inlined' ' ! grep "aaaaaaaa" tmp ' -# --- The shipped highlight filter degrades to escaped passthrough ----------- +# A missing scintillua leaves the shipped filter with nothing to highlight +# with, and a filter that failed there would take the whole page down with +# it, so it has to hand the source back escaped instead. test_expect_success LUA 'highlight filter passes text through without scintillua' ' { echo "virtual-root=/" && diff --git a/tests/t0202-stats.sh b/tests/t0202-stats.sh index 1589776..2ada672 100755 --- a/tests/t0202-stats.sh +++ b/tests/t0202-stats.sh @@ -1,9 +1,15 @@ #!/bin/sh +# Checks the per repository statistics page, which stays off until a config +# turns it on and then reports commits per author beside a breakdown of the +# tree by language. A second config caps the period so that what max-stats +# takes out of the period selector is covered as well. + test_description='Check the statistics page' . ./setup.sh -# The default test config carries no enable-stats. +# The shared test config sets no enable-stats, so this is what an untouched +# install does. test_expect_success 'stats are off by default' ' cgit_url "foo/stats" >tmp && grep "Status: 404" tmp && @@ -11,10 +17,11 @@ test_expect_success 'stats are off by default' ' ! grep ">stats</a>" tmp ' -# A repo with one commit dated now, so the authors table has something -# to count beside the 2005-dated fixture commits. The commit graph -# makes the language walk take the graph lookup path, which once -# returned another commit after the history walk released memory. +# The authors table counts only recent periods, so the fixture needs a commit +# dated now to sit beside the 2005 dates the shared repositories carry. The +# commit graph is written on purpose, because it sends the language walk down +# the graph lookup path, which once handed back another commit after the +# history walk had released its memory. test_expect_success 'set up a stats config' ' mkrepo repos/pulse 2 && ( diff --git a/tests/t0203-dates.sh b/tests/t0203-dates.sh index 1bd6253..02f12f1 100755 --- a/tests/t0203-dates.sh +++ b/tests/t0203-dates.sh @@ -1,10 +1,16 @@ #!/bin/sh +# Checks how cgit shows a commit date, which is either an age relative to now +# or a calendar date in the format the config names. The config is rewritten +# between cases so that each one names only the settings it is about and +# leaves the rest at their defaults. + test_description='Check the date display options' . ./setup.sh -# An old commit and a fresh one, so a single log page exercises both the -# relative rendering and the calendar fallback past the two week cutoff. +# One old commit and one fresh one, so a single page shows both the relative +# rendering and the calendar fallback that takes over past the two week +# cutoff. test_expect_success 'set up a dated repo' ' mkrepo repos/dated 1 && ( @@ -18,7 +24,8 @@ test_expect_success 'set up a dated repo' ' ) ' -# Each argument is written as one extra config line. +# Writes the config afresh, one extra line per argument, which is what lets a +# test below name a setting or two and say nothing about the rest. daterc() { { echo "virtual-root=/" |