From: Mikhail Subject: Re: gotd crash while requesting non-existent commit id To: gameoftrees@openbsd.org Date: Sun, 4 Dec 2022 15:15:58 +0300 On Fri, Nov 04, 2022 at 01:47:43PM +0100, Stefan Sperling wrote: > On Fri, Nov 04, 2022 at 03:37:12PM +0300, Mikhail wrote: > > While doing some kind of "manual stress testing" with gotd it crashed > > for me. Crashes are repeatable. > > Thanks, there was a simple coding error. > Should be fixed by commit 36c7cfbb2a9b646bfb1658fca4e34bc63a46ec42 Here is a test case for this bug. Can it be useful? diff /home/misha/work/got commit - 24b7de1c04072bf25db2df8acbf93a7ba7bbabfd path + /home/misha/work/got blob - cd20ee9bf8631fcd757767edf78df5f410e56161 file + regress/gotd/Makefile --- regress/gotd/Makefile +++ regress/gotd/Makefile @@ -1,7 +1,7 @@ REGRESS_TARGETS=test_repo_read test_repo_read_group \ test_repo_read_denied_user test_repo_read_denied_group \ test_repo_read_bad_user test_repo_read_bad_group \ - test_repo_write test_repo_write_empty + test_repo_write test_repo_write_empty repo_req_wrong_commit NOOBJ=Yes .PHONY: ensure_root prepare_test_repo check_test_repo start_gotd @@ -28,6 +28,7 @@ GOTD_TEST_ENV=GOTD_TEST_ROOT=$(GOTD_TEST_ROOT) \ GOTD_TEST_REPO_URL=$(GOTD_TEST_REPO_URL) \ GOTD_TEST_REPO=$(GOTD_TEST_REPO) \ GOTD_SOCK=$(GOTD_SOCK) \ + GOTD_STOP_CMD="$(GOTD_STOP_CMD)" \ GOTD_DEVUSER=$(GOTD_DEVUSER) \ HOME=$(GOTD_TEST_USER_HOME) \ PATH=$(GOTD_TEST_USER_HOME)/bin:$(PATH) @@ -177,4 +178,8 @@ test_repo_write_empty: prepare_test_repo_empty start_g @$(GOTD_STOP_CMD) 2>/dev/null @su -m ${GOTD_USER} -c 'env $(GOTD_TEST_ENV) sh ./check_test_repo.sh' +repo_req_wrong_commit: prepare_test_repo_empty start_gotd_ro + @-$(GOTD_TRAP); su ${GOTD_TEST_USER} -c \ + 'env $(GOTD_TEST_ENV) sh ./repo_req_wrong_commit.sh' + .include blob - /dev/null file + regress/gotd/repo_req_wrong_commit.sh (mode 644) --- /dev/null +++ regress/gotd/repo_req_wrong_commit.sh @@ -0,0 +1,47 @@ +#!/bin/sh +# +# Copyright (c) 2022 Mikhail Pchelin +# +# Permission to use, copy, modify, and distribute this software for any +# purpose with or without fee is hereby granted, provided that the above +# copyright notice and this permission notice appear in all copies. +# +# THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES +# WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF +# MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR +# ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES +# WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN +# ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF +# OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + +. ../cmdline/common.sh +. ./common.sh + +test_wrong_commit() { + local testroot=`test_init wrong_commit` + + echo "0054want aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa multi_ack \ +side-band-64k ofs-delta" | ssh ${GOTD_DEVUSER}@127.0.0.1 \ + git-upload-pack '/test-repo' > $testroot/stdout + + echo -n "0041ERR object aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa \ +not found" > $testroot/stdout.expected + + cmp -s $testroot/stdout $testroot/stdout.expected 112 0 + ret=$? + if [ $ret -ne 0 ]; then + echo "comparison doesn't match" >&2 + test_done "$testroot" "1" + return 1 + fi + + # Before 36c7cfbb2a gotd was crashing after invalid commit in "want" + # line, here we check that we can successfully stop working gotd. + $GOTD_STOP_CMD + ret=$? + + test_done "$testroot" "$ret" +} + +test_parseargs "$@" +run_test test_wrong_commit