From: Omar Polo Subject: Re: gotd: wrong requests test To: Mikhail Cc: Stefan Sperling , gameoftrees@openbsd.org Date: Mon, 26 Dec 2022 12:00:09 +0100 On 2022/12/25 18:16:06 +0300, Mikhail wrote: > On Sun, Dec 25, 2022 at 11:11:36AM +0100, Stefan Sperling wrote: > > and name the script file request_bad.sh just a thought, doesn't bad_request (both as filename and prefix) reads better? will stop the bikeshedding here :) > > As minor tweaks above, say "capabilities" instead of "caps" (to avoid > > confusion with "capital letters") and "repo" instead of "rep" (we use > > "repo" consistently in our code as abbreviation of "repository"). > > Done, next version: some comments inline. However I can make the changes as a follow-up commit. I'm ok with the diff. > diff /home/misha/work/got > commit - 1abb18e1777172a9f4149a0f50c4cecfd024f02c > path + /home/misha/work/got > [...] > blob - /dev/null > file + regress/gotd/request_bad.sh (mode 644) > --- /dev/null > +++ regress/gotd/request_bad.sh > @@ -0,0 +1,290 @@ > +#!/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 > + > +# Non-existent commit > +test_request_bad_commit() { > + local testroot=`test_init request_bad_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 \ > + 2>$testroot/stderr to cut down the length of the lines we could define the "aaaa..." string in a variable. dummy_commit="aaaaaaaa" ... echo "0054want $dummy_commit multi_ack side-band-64 ofs-delta" \ | ssh ... (and the same in the other cases) > + > + echo -n "0041ERR object aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa \ > +not found" > $testroot/stdout.expected > + > + echo "gotsh: object aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa \ > +not found" > $testroot/stderr.expected > + > + # We use OpenBSD cmp(1) offset extension The regress suite should ideally work on non-OpenBSD. Fortunately, this extension of cmp(1) seem to be quite popular (it's also present at least on FreeBSD cmp and GNU cmp) so I think we should just drop the comment and see how things go in -portable. should it be a issue we can always play with dd i guess. (and the same in the other cases) > + cmp -s $testroot/stdout $testroot/stdout.expected 112 0 > + ret=$? > + if [ $ret -ne 0 ]; then > + echo "unexpected stdout" >&2 > + test_done "$testroot" "1" > + return 1 > + fi > + > + cmp -s $testroot/stderr $testroot/stderr.expected > + ret=$? > + if [ $ret -ne 0 ]; then not an issue, but if you want to cut some typing (and possibly making the reading easier) you could also rewrite it as + if ! cmp -s $testroot/stderr $testroot/stderr.expected; then admittedly not widespread in the regress suite, but when we're not interested in the exit code I think it's cleaner. (wooops, did I say i would stop with the bikeshedding? :P) > + echo "unexpected stderr" >&2 > + test_done "$testroot" "1" > + return 1 > + fi > + test_done "$testroot" "$ret" > +} > + > +# Zero pkt-len (as flush packet with payload) > +test_request_bad_length_zero() { > + local testroot=`test_init test_request_bad_length_zero` > + > + echo "0000want aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa multi_ack \ > +side-band-64k ofs-delta" | ssh ${GOTD_DEVUSER}@127.0.0.1 \ > + git-upload-pack '/test-repo' > $testroot/stdout \ > + 2>$testroot/stderr > + > + echo -n "00000028ERR unexpected flush packet received" \ > + > $testroot/stdout.expected > + > + echo "gotsh: unexpected flush packet received" \ > + > $testroot/stderr.expected > + > + # We use OpenBSD cmp(1) offset extension > + cmp -s $testroot/stdout $testroot/stdout.expected 108 0 > + ret=$? > + if [ $ret -ne 0 ]; then > + echo "unexpected stdout" >&2 > + test_done "$testroot" "1" > + return 1 > + fi > + > + cmp -s $testroot/stderr $testroot/stderr.expected > + ret=$? > + if [ $ret -ne 0 ]; then > + echo "unexpected stderr" >&2 > + test_done "$testroot" "1" > + return 1 > + fi > + test_done "$testroot" "$ret" > +} > + > +# 0004 (empty) > +test_request_bad_length_empty() { > + local testroot=`test_init test_request_bad_length_empty` > + > + echo "0004want aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa multi_ack \ > +side-band-64k ofs-delta" | ssh ${GOTD_DEVUSER}@127.0.0.1 \ > + git-upload-pack '/test-repo' > $testroot/stdout \ > + 2>$testroot/stderr > + > + echo -n "00000008NAK\n0021ERR read: Bad file descriptor" \ > + > $testroot/stdout.expected can we rely on echo(1) expanding "\n" to the newline character? I think it would be better to just use printf(1) here and similar cases below. > + echo "gotsh: read: Bad file descriptor" > $testroot/stderr.expected > + > + # We use OpenBSD cmp(1) offset extension > + cmp -s $testroot/stdout $testroot/stdout.expected 108 0 > + ret=$? > + if [ $ret -ne 0 ]; then > + echo "unexpected stdout" >&2 > + test_done "$testroot" "1" > + return 1 > + fi > + > + cmp -s $testroot/stderr $testroot/stderr.expected > + ret=$? > + if [ $ret -ne 0 ]; then > + echo "unexpected stderr" >&2 > + test_done "$testroot" "1" > + return 1 > + fi > + test_done "$testroot" "$ret" > +} > + > +# Pkt-len too small > +test_request_bad_length_small() { > + local testroot=`test_init test_request_bad_length_small` > + > + echo "0002want aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa multi_ack \ > +side-band-64k ofs-delta" | ssh ${GOTD_DEVUSER}@127.0.0.1 \ > + git-upload-pack '/test-repo' > $testroot/stdout \ > + 2>$testroot/stderr > + > + echo -n "00000008NAK\n0021ERR read: Bad file descriptor" \ > + > $testroot/stdout.expected > + > + echo "gotsh: read: Bad file descriptor" > $testroot/stderr.expected > + > + # We use OpenBSD cmp(1) offset extension > + cmp -s $testroot/stdout $testroot/stdout.expected 108 0 > + ret=$? > + if [ $ret -ne 0 ]; then > + echo "unexpected stdout" >&2 > + test_done "$testroot" "1" > + return 1 > + fi > + > + cmp -s $testroot/stderr $testroot/stderr.expected > + ret=$? > + if [ $ret -ne 0 ]; then > + echo "unexpected stderr" >&2 > + test_done "$testroot" "1" > + return 1 > + fi > + test_done "$testroot" "$ret" > +} missign empty line > +# Pkt-len too large > +test_request_bad_length_large() { > + local testroot=`test_init test_request_bad_length_large` > + > + echo "ffffwant aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa multi_ack \ > +side-band-64k ofs-delta" | ssh ${GOTD_DEVUSER}@127.0.0.1 \ > + git-upload-pack '/test-repo' > $testroot/stdout \ > + 2>$testroot/stderr > + > + echo -n "00000008NAK\n0021ERR read: Bad file descriptor" \ > + > $testroot/stdout.expected > + > + echo "gotsh: read: Bad file descriptor" > $testroot/stderr.expected > + > + # We use OpenBSD cmp(1) offset extension > + cmp -s $testroot/stdout $testroot/stdout.expected 108 0 > + ret=$? > + if [ $ret -ne 0 ]; then > + echo "unexpected stdout" >&2 > + test_done "$testroot" "1" > + return 1 > + fi > + > + cmp -s $testroot/stderr $testroot/stderr.expected > + ret=$? > + if [ $ret -ne 0 ]; then > + echo "unexpected stderr" >&2 > + test_done "$testroot" "1" > + return 1 > + fi > + test_done "$testroot" "$ret" > +} > + > +# Unknown feature > +test_request_bad_capabilities() { > + local testroot=`test_init test_request_bad_capabilities` > + > + echo "0054want aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaa \ > +bbbbbbbbbbbbb ccccccccc" | ssh ${GOTD_DEVUSER}@127.0.0.1 \ > + git-upload-pack '/test-repo' > $testroot/stdout \ > + 2>$testroot/stderr > + > + echo -n "00000025ERR unexpected want-line received" \ > + > $testroot/stdout.expected > + > + echo "gotsh: unexpected want-line received" > $testroot/stderr.expected > + > + # We use OpenBSD cmp(1) offset extension > + cmp -s $testroot/stdout $testroot/stdout.expected 108 0 > + ret=$? > + if [ $ret -ne 0 ]; then > + echo "unexpected stdout" >&2 > + test_done "$testroot" "1" > + return 1 > + fi > + > + cmp -s $testroot/stderr $testroot/stderr.expected > + ret=$? > + if [ $ret -ne 0 ]; then > + echo "unexpected stderr" >&2 > + test_done "$testroot" "1" > + return 1 > + fi > + test_done "$testroot" "$ret" > +} > + > +# Unknown repository > +test_request_bad_repository() { > + local testroot=`test_init test_request_bad_repository` > + > + echo "0054want aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaa \ > +bbbbbbbbbbbbb ccccccccc" | ssh ${GOTD_DEVUSER}@127.0.0.1 \ > + git-upload-pack '/XXXX-XXXX' > $testroot/stdout \ > + 2>$testroot/stderr > + > + echo -n "001fERR no git repository found" > $testroot/stdout.expected > + > + echo "gotsh: no git repository found" > $testroot/stderr.expected > + > + cmp -s $testroot/stdout $testroot/stdout.expected > + ret=$? > + if [ $ret -ne 0 ]; then > + echo "unexpected stdout" >&2 > + test_done "$testroot" "1" > + return 1 > + fi > + > + cmp -s $testroot/stderr $testroot/stderr.expected > + ret=$? > + if [ $ret -ne 0 ]; then > + echo "unexpected stderr" >&2 > + test_done "$testroot" "1" > + return 1 > + fi > + test_done "$testroot" "$ret" > + > +} > + > +# Repository with name of 255 symbols > +test_request_bad_large_repo_name() { > + local testroot=`test_init test_request_bad_large_repo_name` > + > + echo "0054want aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaa \ > +bbbbbbbbbbbbb ccccccccc" | ssh ${GOTD_DEVUSER}@127.0.0.1 \ > + git-upload-pack '/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA' > $testroot/stdout \ > + 2>$testroot/stderr > + > + echo -n "0018ERR buffer too small" > $testroot/stdout.expected > + > + echo "gotsh: buffer too small" > $testroot/stderr.expected > + > + cmp -s $testroot/stdout $testroot/stdout.expected > + ret=$? > + if [ $ret -ne 0 ]; then > + echo "unexpected stdout" >&2 > + test_done "$testroot" "1" > + return 1 > + fi > + > + cmp -s $testroot/stderr $testroot/stderr.expected > + ret=$? > + if [ $ret -ne 0 ]; then > + echo "unexpected stderr" >&2 > + test_done "$testroot" "1" > + return 1 > + fi > + test_done "$testroot" "$ret" > + > +} missing empty line > +test_parseargs "$@" > +run_test test_request_bad_commit > +run_test test_request_bad_length_zero > +run_test test_request_bad_length_empty > +run_test test_request_bad_length_small > +run_test test_request_bad_length_large > +run_test test_request_bad_capabilities > +run_test test_request_bad_repository > +run_test test_request_bad_large_repo_name