From 8f65285faf473f91ad0c8c5d099821147c30caa8 Mon Sep 17 00:00:00 2001
From: Bjorn Svensson <bjorn.a.svensson@est.tech>
Date: Wed, 26 Apr 2023 16:00:01 +0200
Subject: [PATCH] Add test of redirects with hostname (#150)

The new testcase redirect-with-hostname-test-async is run depending of
hiredis version used. It is disabled for hiredis v1.1.0 due to a hiredis bug
or if the hiredis version is unknown.
---
 .github/workflows/ci.yml                     | 10 +-
 tests/CMakeLists.txt                         | 13 +++
 tests/scripts/redirect-with-hostname-test.sh | 96 ++++++++++++++++++++
 3 files changed, 116 insertions(+), 3 deletions(-)
 create mode 100755 tests/scripts/redirect-with-hostname-test.sh

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 3dd4bb3..6a15994 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -45,10 +45,14 @@ jobs:
       with:
         packages: libevent-dev libuv1-dev libev-dev libglib2.0-dev ${{ matrix.compiler }}
         version: 1.0
-    - name: Install hiredis
+    - name: Install hiredis (pre-v1.1.1)
+      env:
+        SHA: d13c091e9f09fe4ad1858cd54ed3a842ac00b8e6
       run: |
-        curl -L https://github.com/redis/hiredis/archive/refs/tags/v1.1.0.tar.gz | tar -xz
-        cmake -S hiredis-1.1.0 -B hiredis-build -DENABLE_SSL=ON
+        # When v1.1.1 is released update to:
+        # curl -L https://github.com/redis/hiredis/archive/refs/tags/v1.1.1.tar.gz | tar -xz
+        curl -L https://github.com/redis/hiredis/archive/${SHA}.tar.gz | tar -xz
+        cmake -S hiredis-${SHA} -B hiredis-build -DENABLE_SSL=ON
         sudo make -C hiredis-build install
     - name: Setup cmake
       uses: jwlawson/actions-setup-cmake@v1
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 7cbe7d2..63e27f4 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -216,3 +216,16 @@ add_test(NAME connection-error-test
          COMMAND "${CMAKE_SOURCE_DIR}/tests/scripts/connection-error-test.sh"
                  "$<TARGET_FILE:clusterclient_async_sequence>"
          WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}/tests/scripts/")
+add_test(NAME redirect-with-hostname-test
+         COMMAND "${CMAKE_SOURCE_DIR}/tests/scripts/redirect-with-hostname-test.sh"
+                 "$<TARGET_FILE:clusterclient>"
+         WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}/tests/scripts/")
+# This test can't be run on hiredis v1.1.0 due to hiredis issue #1171.
+# Disabling the testcase if hiredis contains the issue or if the version is unknown.
+add_test(NAME redirect-with-hostname-test-async
+         COMMAND "${CMAKE_SOURCE_DIR}/tests/scripts/redirect-with-hostname-test.sh"
+                 "$<TARGET_FILE:clusterclient_async_sequence>"
+         WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}/tests/scripts/")
+if(hiredis_VERSION VERSION_EQUAL "1.1.0" OR hiredis_VERSION VERSION_EQUAL "0")
+  set_tests_properties(redirect-with-hostname-test-async PROPERTIES DISABLED True)
+endif()
diff --git a/tests/scripts/redirect-with-hostname-test.sh b/tests/scripts/redirect-with-hostname-test.sh
new file mode 100755
index 0000000..29d2f74
--- /dev/null
+++ b/tests/scripts/redirect-with-hostname-test.sh
@@ -0,0 +1,96 @@
+#!/bin/sh
+#
+# Verify that redirects with hostname are handled.
+#
+# Redis 7.0 introduced the config `cluster-preferred-endpoint-type` which
+# controls how the endpoint is returned in ASK/MOVED redirects and in
+# CLUSTER SLOTS. This testcase verifies correct handling when Redis
+# announce a hostname 'localhost' instead of an IP.
+#
+# Redis 7.0 also adds additional metadata in the CLUSTER SLOTS response
+# and this testcase uses a black hole address to make sure this is accepted
+# but not used.
+#
+# Usage: $0 /path/to/clusterclient-binary
+
+clientprog=${1:-./clusterclient}
+testname=redirect-with-hostname-test
+
+# Sync processes waiting for CONT signals.
+perl -we 'use sigtrap "handler", sub{exit}, "CONT"; sleep 1; die "timeout"' &
+syncpid1=$!;
+perl -we 'use sigtrap "handler", sub{exit}, "CONT"; sleep 1; die "timeout"' &
+syncpid2=$!;
+
+# Start simulated redis node #1
+timeout 5s ./simulated-redis.pl -p 7401 -d --sigcont $syncpid1 <<'EOF' &
+# Inital slotmap update
+EXPECT CONNECT
+EXPECT ["CLUSTER", "SLOTS"]
+SEND [[0, 16383, ["localhost", 7401, "nodeid1", ["ip", "192.168.254.254"]]]]
+EXPECT CLOSE
+
+# Verify ASK redirect
+EXPECT CONNECT
+EXPECT ["GET", "foo"]
+SEND -ASK 12182 localhost:7402
+
+# Verify MOVED redirect
+EXPECT ["GET", "foo"]
+SEND -MOVED 12182 localhost:7402
+
+# Slotmap updated due to MOVED
+EXPECT CONNECT
+EXPECT ["CLUSTER", "SLOTS"]
+SEND [[0, 16383, ["localhost", 7402, "nodeid2", ["ip", "192.168.254.254"]]]]
+EXPECT CLOSE
+EXPECT CLOSE
+EOF
+server1=$!
+
+# Start simulated redis node #2
+timeout 5s ./simulated-redis.pl -p 7402 -d --sigcont $syncpid2 <<'EOF' &
+EXPECT CONNECT
+EXPECT ["ASKING"]
+SEND +OK
+EXPECT ["GET", "foo"]
+SEND "bar"
+EXPECT ["GET", "foo"]
+SEND "bar"
+EXPECT CLOSE
+EOF
+server2=$!
+
+# Wait until both nodes are ready to accept client connections
+wait $syncpid1 $syncpid2;
+
+# Run client
+timeout 3s "$clientprog" localhost:7401 > "$testname.out" <<'EOF'
+GET foo
+GET foo
+EOF
+clientexit=$?
+
+# Wait for servers to exit
+wait $server1; server1exit=$?
+wait $server2; server2exit=$?
+
+# Check exit statuses
+if [ $server1exit -ne 0 ]; then
+    echo "Simulated server #1 exited with status $server1exit"
+    exit $server1exit
+fi
+if [ $server2exit -ne 0 ]; then
+    echo "Simulated server #2 exited with status $server2exit"
+    exit $server2exit
+fi
+if [ $clientexit -ne 0 ]; then
+    echo "$clientprog exited with status $clientexit"
+    exit $clientexit
+fi
+
+# Check the output from clusterclient
+printf 'bar\nbar\n' | cmp "$testname.out" - || exit 99
+
+# Clean up
+rm "$testname.out"