From b8dab7b1a9c2fdcb4cc287c920eb603d79a158fa Mon Sep 17 00:00:00 2001 From: Anis Eleuch Date: Mon, 2 Dec 2024 14:21:17 +0100 Subject: [PATCH] Set http server read/write timeout from --idle-timeout (#228) (#20715) Golang http.Server will call SetReadDeadline overwriting the previous deadline configuration set after a new connection Accept in the custom listener code. Therefore, --idle-timeout was not correctly respected. Make http.Server read/write timeout similar to --idle-timeout. --- .github/workflows/go.yml | 1 + Makefile | 4 + buildscripts/test-timeout.sh | 137 ++++++++++++++++++++++++++ cmd/server-main.go | 4 +- internal/deadlineconn/deadlineconn.go | 8 +- 5 files changed, 148 insertions(+), 6 deletions(-) create mode 100644 buildscripts/test-timeout.sh diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index b3c4027db3cff..480a18d027768 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -39,3 +39,4 @@ jobs: sudo sysctl net.ipv6.conf.all.disable_ipv6=0 sudo sysctl net.ipv6.conf.default.disable_ipv6=0 make verify + make test-timeout diff --git a/Makefile b/Makefile index 4d65c39c55992..ef6bf84d88300 100644 --- a/Makefile +++ b/Makefile @@ -149,6 +149,10 @@ test-multipart: install-race ## test multipart @echo "Test multipart behavior when part files are missing" @(env bash $(PWD)/buildscripts/multipart-quorum-test.sh) +test-timeout: install-race ## test multipart + @echo "Test server timeout" + @(env bash $(PWD)/buildscripts/test-timeout.sh) + verify: install-race ## verify minio various setups @echo "Verifying build with race" @(env bash $(PWD)/buildscripts/verify-build.sh) diff --git a/buildscripts/test-timeout.sh b/buildscripts/test-timeout.sh new file mode 100644 index 0000000000000..77a248a6432fc --- /dev/null +++ b/buildscripts/test-timeout.sh @@ -0,0 +1,137 @@ +#!/bin/bash + +if [ -n "$TEST_DEBUG" ]; then + set -x +fi + +WORK_DIR="$PWD/.verify-$RANDOM" +MINIO_CONFIG_DIR="$WORK_DIR/.minio" +MINIO=("$PWD/minio" --config-dir "$MINIO_CONFIG_DIR" server) + +if [ ! -x "$PWD/minio" ]; then + echo "minio executable binary not found in current directory" + exit 1 +fi + +if [ ! -x "$PWD/minio" ]; then + echo "minio executable binary not found in current directory" + exit 1 +fi + +trap 'catch $LINENO' ERR + +function purge() { + rm -rf "$1" +} + +# shellcheck disable=SC2120 +catch() { + if [ $# -ne 0 ]; then + echo "error on line $1" + fi + + echo "Cleaning up instances of MinIO" + pkill minio || true + pkill -9 minio || true + purge "$WORK_DIR" + if [ $# -ne 0 ]; then + exit $# + fi +} + +catch + +function gen_put_request() { + hdr_sleep=$1 + body_sleep=$2 + + echo "PUT /testbucket/testobject HTTP/1.1" + sleep $hdr_sleep + echo "Host: foo-header" + echo "User-Agent: curl/8.2.1" + echo "Accept: */*" + echo "Content-Length: 30" + echo "" + + sleep $body_sleep + echo "random line 0" + echo "random line 1" + echo "" + echo "" +} + +function send_put_object_request() { + hdr_timeout=$1 + body_timeout=$2 + + start=$(date +%s) + timeout 5m bash -c "gen_put_request $hdr_timeout $body_timeout | netcat 127.0.0.1 $start_port | read" || return -1 + [ $(($(date +%s) - start)) -gt $((srv_hdr_timeout + srv_idle_timeout + 1)) ] && return -1 + return 0 +} + +function test_minio_with_timeout() { + start_port=$1 + + export MINIO_ROOT_USER=minio + export MINIO_ROOT_PASSWORD=minio123 + export MC_HOST_minio="http://minio:minio123@127.0.0.1:${start_port}/" + export MINIO_CI_CD=1 + + mkdir ${WORK_DIR} + C_PWD=${PWD} + if [ ! -x "$PWD/mc" ]; then + MC_BUILD_DIR="mc-$RANDOM" + if ! git clone --quiet https://github.com/minio/mc "$MC_BUILD_DIR"; then + echo "failed to download https://github.com/minio/mc" + purge "${MC_BUILD_DIR}" + exit 1 + fi + + (cd "${MC_BUILD_DIR}" && go build -o "$C_PWD/mc") + + # remove mc source. + purge "${MC_BUILD_DIR}" + fi + + "${MINIO[@]}" --address ":$start_port" --read-header-timeout ${srv_hdr_timeout}s --idle-timeout ${srv_idle_timeout}s "${WORK_DIR}/disk/" >"${WORK_DIR}/server1.log" 2>&1 & + pid=$! + disown $pid + sleep 1 + + if ! ps -p ${pid} 1>&2 >/dev/null; then + echo "server1 log:" + cat "${WORK_DIR}/server1.log" + echo "FAILED" + purge "$WORK_DIR" + exit 1 + fi + + set -e + + "${PWD}/mc" mb minio/testbucket + "${PWD}/mc" anonymous set public minio/testbucket + + # slow header writing + send_put_object_request 20 0 && exit -1 + "${PWD}/mc" stat minio/testbucket/testobject && exit -1 + + # quick header write and slow bodywrite + send_put_object_request 0 40 && exit -1 + "${PWD}/mc" stat minio/testbucket/testobject && exit -1 + + # quick header and body write + send_put_object_request 1 1 || exit -1 + "${PWD}/mc" stat minio/testbucket/testobject || exit -1 +} + +function main() { + export start_port=$(shuf -i 10000-65000 -n 1) + export srv_hdr_timeout=5 + export srv_idle_timeout=5 + export -f gen_put_request + + test_minio_with_timeout ${start_port} +} + +main "$@" diff --git a/cmd/server-main.go b/cmd/server-main.go index 455853282861f..62014c8469ad8 100644 --- a/cmd/server-main.go +++ b/cmd/server-main.go @@ -879,8 +879,8 @@ func serverMain(ctx *cli.Context) { UseHandler(setCriticalErrorHandler(corsHandler(handler))). UseTLSConfig(newTLSConfig(getCert)). UseIdleTimeout(globalServerCtxt.IdleTimeout). - UseReadTimeout(24 * time.Hour). // (overridden by listener.config.IdleTimeout on requests) - UseWriteTimeout(24 * time.Hour). // (overridden by listener.config.IdleTimeout on requests) + UseReadTimeout(globalServerCtxt.IdleTimeout). + UseWriteTimeout(globalServerCtxt.IdleTimeout). UseReadHeaderTimeout(globalServerCtxt.ReadHeaderTimeout). UseBaseContext(GlobalContext). UseCustomLogger(log.New(io.Discard, "", 0)). // Turn-off random logging by Go stdlib diff --git a/internal/deadlineconn/deadlineconn.go b/internal/deadlineconn/deadlineconn.go index 948aa8f1c0322..95bb43efff772 100644 --- a/internal/deadlineconn/deadlineconn.go +++ b/internal/deadlineconn/deadlineconn.go @@ -114,8 +114,8 @@ func (c *DeadlineConn) SetDeadline(t time.Time) error { c.mu.Lock() defer c.mu.Unlock() - c.readSetAt = time.Now() - c.writeSetAt = time.Now() + c.readSetAt = time.Time{} + c.writeSetAt = time.Time{} c.abortReads.Store(!t.IsZero() && time.Until(t) < 0) c.abortWrites.Store(!t.IsZero() && time.Until(t) < 0) c.infReads.Store(t.IsZero()) @@ -131,7 +131,7 @@ func (c *DeadlineConn) SetReadDeadline(t time.Time) error { defer c.mu.Unlock() c.abortReads.Store(!t.IsZero() && time.Until(t) < 0) c.infReads.Store(t.IsZero()) - c.readSetAt = time.Now() + c.readSetAt = time.Time{} return c.Conn.SetReadDeadline(t) } @@ -145,7 +145,7 @@ func (c *DeadlineConn) SetWriteDeadline(t time.Time) error { defer c.mu.Unlock() c.abortWrites.Store(!t.IsZero() && time.Until(t) < 0) c.infWrites.Store(t.IsZero()) - c.writeSetAt = time.Now() + c.writeSetAt = time.Time{} return c.Conn.SetWriteDeadline(t) }