From 48e5220d0024834fa2cb054aeedc5a70d22fa755 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran <105669860+msotheeswaran@users.noreply.github.com> Date: Thu, 29 Dec 2022 14:47:06 -0500 Subject: [PATCH] TLS test fix from redis (#524) * Fix TLS tests on newer tcl-tls/OpenSSL. (#10910) Before this commit, TLS tests on Ubuntu 22.04 would fail as dropped connections result with an ECONNABORTED error thrown instead of an empty read. * multithread for ci tests * multithread cluster tests * clients to 1 in ci Co-authored-by: Yossi Gottlieb --- .github/workflows/ci.yml | 4 ++-- tests/support/keydb.tcl | 41 +++++++++++++++++++++++++++++++++----- tests/unit/obuf-limits.tcl | 5 ++--- 3 files changed, 40 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2be17b176..76b18cdbe 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,10 +21,10 @@ jobs: - name: test-tls run: | sudo apt-get -y install tcl tcl-tls - ./runtest --clients 2 --verbose --tls + ./runtest --clients 1 --verbose --tls --config server-threads 3 - name: cluster-test run: | - ./runtest-cluster --tls + ./runtest-cluster --tls --config server-threads 3 - name: sentinel test run: | ./runtest-sentinel diff --git a/tests/support/keydb.tcl b/tests/support/keydb.tcl index 978163e98..0cfd2a3e2 100644 --- a/tests/support/keydb.tcl +++ b/tests/support/keydb.tcl @@ -66,6 +66,33 @@ proc redis {{server 127.0.0.1} {port 6379} {defer 0} {tls 0} {tlsoptions {}} {re interp alias {} ::redis::redisHandle$id {} ::redis::__dispatch__ $id } +# On recent versions of tcl-tls/OpenSSL, reading from a dropped connection +# results with an error we need to catch and mimic the old behavior. +proc ::redis::redis_safe_read {fd len} { + if {$len == -1} { + set err [catch {set val [read $fd]} msg] + } else { + set err [catch {set val [read $fd $len]} msg] + } + if {!$err} { + return $val + } + if {[string match "*connection abort*" $msg]} { + return {} + } + error $msg +} + +proc ::redis::redis_safe_gets {fd} { + if {[catch {set val [gets $fd]} msg]} { + if {[string match "*connection abort*" $msg]} { + return {} + } + error $msg + } + return $val +} + # This is a wrapper to the actual dispatching procedure that handles # reconnection if needed. proc ::redis::__dispatch__ {id method args} { @@ -146,6 +173,10 @@ proc ::redis::__method__read {id fd} { ::redis::redis_read_reply $id $fd } +proc ::redis::__method__rawread {id fd {len -1}} { + return [redis_safe_read $fd $len] +} + proc ::redis::__method__write {id fd buf} { ::redis::redis_write $fd $buf } @@ -192,8 +223,8 @@ proc ::redis::redis_writenl {fd buf} { } proc ::redis::redis_readnl {fd len} { - set buf [read $fd $len] - read $fd 2 ; # discard CR LF + set buf [redis_safe_read $fd $len] + redis_safe_read $fd 2 ; # discard CR LF return $buf } @@ -239,11 +270,11 @@ proc ::redis::redis_read_map {id fd} { } proc ::redis::redis_read_line fd { - string trim [gets $fd] + string trim [redis_safe_gets $fd] } proc ::redis::redis_read_null fd { - gets $fd + redis_safe_gets $fd return {} } @@ -260,7 +291,7 @@ proc ::redis::redis_read_reply {id fd} { } while {1} { - set type [read $fd 1] + set type [redis_safe_read $fd 1] switch -exact -- $type { _ {return [redis_read_null $fd]} : - diff --git a/tests/unit/obuf-limits.tcl b/tests/unit/obuf-limits.tcl index bbb9fcbf6..77c8e0bec 100644 --- a/tests/unit/obuf-limits.tcl +++ b/tests/unit/obuf-limits.tcl @@ -111,7 +111,7 @@ start_server {tags {"obuf-limits"} overrides { server-threads 1 }} { # Read nothing set fd [$rd channel] - assert_equal {} [read $fd] + assert_equal {} [$rd rawread] } # Note: This test assumes that what's written with one write, will be read by redis in one read. @@ -151,8 +151,7 @@ start_server {tags {"obuf-limits"} overrides { server-threads 1 }} { assert_equal "PONG" [r ping] set clients [r client list] assert_no_match "*name=multicommands*" $clients - set fd [$rd2 channel] - assert_equal {} [read $fd] + assert_equal {} [$rd2 rawread] } test {Execute transactions completely even if client output buffer limit is enforced} {