From daad3a215499422559587666bb2f85ca746d7bab Mon Sep 17 00:00:00 2001
From: Benjamin Wang <wachao@vmware.com>
Date: Mon, 5 Dec 2022 19:53:35 +0800
Subject: [PATCH] etcdserver: fix nil pointer panic for readonly txn

FYI. https://github.com/etcd-io/etcd/issues/14891#issuecomment-1337191993

Signed-off-by: Benjamin Wang <wachao@vmware.com>
---
 server/etcdserver/txn/util.go      |   6 +-
 server/etcdserver/txn/util_test.go | 125 +++++++++++++++++++++++++++++
 2 files changed, 130 insertions(+), 1 deletion(-)
 create mode 100644 server/etcdserver/txn/util_test.go

diff --git a/server/etcdserver/txn/util.go b/server/etcdserver/txn/util.go
index 64e2e01bcb5..7c4b58b46cc 100644
--- a/server/etcdserver/txn/util.go
+++ b/server/etcdserver/txn/util.go
@@ -63,7 +63,11 @@ func WarnOfExpensiveReadOnlyTxnRequest(lg *zap.Logger, warningApplyDuration time
 		for _, r := range txnResponse.Responses {
 			switch op := r.Response.(type) {
 			case *pb.ResponseOp_ResponseRange:
-				resps = append(resps, fmt.Sprintf("range_response_count:%d", len(op.ResponseRange.Kvs)))
+				if op.ResponseRange != nil {
+					resps = append(resps, fmt.Sprintf("range_response_count:%d", len(op.ResponseRange.Kvs)))
+				} else {
+					resps = append(resps, "range_response:nil")
+				}
 			default:
 				// only range responses should be in a read only txn request
 			}
diff --git a/server/etcdserver/txn/util_test.go b/server/etcdserver/txn/util_test.go
new file mode 100644
index 00000000000..205f35e168e
--- /dev/null
+++ b/server/etcdserver/txn/util_test.go
@@ -0,0 +1,125 @@
+// Copyright 2022 The etcd Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package txn
+
+import (
+	"testing"
+	"time"
+
+	pb "go.etcd.io/etcd/api/v3/etcdserverpb"
+	"go.uber.org/zap/zaptest"
+)
+
+// TestWarnOfExpensiveReadOnlyTxnRequest verifies WarnOfExpensiveReadOnlyTxnRequest
+// never panic no matter what data the txnResponse contains.
+func TestWarnOfExpensiveReadOnlyTxnRequest(t *testing.T) {
+	testCases := []struct {
+		name    string
+		txnResp *pb.TxnResponse
+	}{
+		{
+			name: "all readonly responses",
+			txnResp: &pb.TxnResponse{
+				Responses: []*pb.ResponseOp{
+					{
+						Response: &pb.ResponseOp_ResponseRange{
+							ResponseRange: &pb.RangeResponse{},
+						},
+					},
+					{
+						Response: &pb.ResponseOp_ResponseRange{
+							ResponseRange: &pb.RangeResponse{},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "all readonly responses with partial nil responses",
+			txnResp: &pb.TxnResponse{
+				Responses: []*pb.ResponseOp{
+					{
+						Response: &pb.ResponseOp_ResponseRange{
+							ResponseRange: &pb.RangeResponse{},
+						},
+					},
+					{
+						Response: &pb.ResponseOp_ResponseRange{
+							ResponseRange: nil,
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "all readonly responses with all nil responses",
+			txnResp: &pb.TxnResponse{
+				Responses: []*pb.ResponseOp{
+					{
+						Response: &pb.ResponseOp_ResponseRange{
+							ResponseRange: nil,
+						},
+					},
+					{
+						Response: &pb.ResponseOp_ResponseRange{
+							ResponseRange: nil,
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "partial non readonly responses",
+			txnResp: &pb.TxnResponse{
+				Responses: []*pb.ResponseOp{
+					{
+						Response: &pb.ResponseOp_ResponseRange{
+							ResponseRange: nil,
+						},
+					},
+					{
+						Response: &pb.ResponseOp_ResponsePut{},
+					},
+					{
+						Response: &pb.ResponseOp_ResponseDeleteRange{},
+					},
+				},
+			},
+		},
+		{
+			name: "all non readonly responses",
+			txnResp: &pb.TxnResponse{
+				Responses: []*pb.ResponseOp{
+					{
+						Response: &pb.ResponseOp_ResponsePut{},
+					},
+					{
+						Response: &pb.ResponseOp_ResponseDeleteRange{},
+					},
+				},
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			lg := zaptest.NewLogger(t)
+			start := time.Now().Add(-1 * time.Second)
+			// WarnOfExpensiveReadOnlyTxnRequest shouldn't panic.
+			WarnOfExpensiveReadOnlyTxnRequest(lg, 0, start, &pb.TxnRequest{}, tc.txnResp, nil)
+		})
+	}
+}