Merge branch 'hashicorp:master' into master

hashicorp · Jun 12, 2022 · a01d1da · a01d1da
2 parents c97f621 + 7c00845
commit a01d1da
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 8 deletions.
diff --git a/serf/internal_query.go b/serf/internal_query.go
@@ -163,6 +163,13 @@ func (s *serfQueries) handleConflict(q *Query) {
 func (s *serfQueries) keyListResponseWithCorrectSize(q *Query, resp *nodeKeyResponse) ([]byte, messageQueryResponse, error) {
 	maxListKeys := q.serf.config.QueryResponseSizeLimit / minEncodedKeyLength
 	actual := len(resp.Keys)
+
+	// if the provided list of keys is smaller then the max allowed, just iterate over it
+	// to avoid an out of bound access when truncating
+	if maxListKeys > actual {
+		maxListKeys = actual
+	}
+
 	for i := maxListKeys; i >= 0; i-- {
 		buf, err := encodeMessage(messageKeyResponseType, resp)
 		if err != nil {

diff --git a/serf/internal_query_test.go b/serf/internal_query_test.go
@@ -143,6 +143,35 @@ func TestSerfQueries_keyListResponseWithCorrectSize(t *testing.T) {
 			"KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=",
 			"KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=",
 		}}},
+		// this test when the list of keys length is less than the max allowed, in this test case 1024/25 = 40
+		{expected: 18, hasMsg: true, resp: nodeKeyResponse{Keys: []string{
+			"KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=",
+			"KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=",
+			"KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=",
+			"KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=",
+			"KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=",
+			"KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=",
+			"KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=",
+			"KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=",
+		}}},
+		// this test when the list of keys length is equal the max allowed, in this test case 1024/25 = 40
+		{expected: 18, hasMsg: true, resp: nodeKeyResponse{Keys: []string{
+			"KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=",
+			"KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=",
+			"KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=",
+			"KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=",
+			"KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=",
+			"KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=",
+			"KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=",
+			"KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=",
+		}}},
+		// this test when the list of keys length is equal the max allowed, in this test case 1024/25 = 40
+		{expected: 18, hasMsg: false, resp: nodeKeyResponse{Keys: []string{
+			"KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=",
+			"KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=",
+			"KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=",
+			"KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=", "KfCPZAKdgHUOdb202afZfE8EbdZqj4+ReTbfJUkfKsg=",
+		}}},
 	}
 	for _, c := range cases {
 		r := c.resp

diff --git a/serf/query.go b/serf/query.go
@@ -185,6 +185,22 @@ func (r *QueryResponse) sendResponse(nr NodeResponse) error {
 	return nil
 }
 
+// sendResponse sends a response on the response channel ensuring the channel is not closed.
+func (r *QueryResponse) sendAck(nr *messageQueryResponse) error {
+	r.closeLock.Lock()
+	defer r.closeLock.Unlock()
+	if r.closed {
+		return nil
+	}
+	select {
+	case r.ackCh <- nr.From:
+		r.acks[nr.From] = struct{}{}
+	default:
+		return errors.New("serf: Failed to deliver query response, dropping")
+	}
+	return nil
+}
+
 // NodeResponse is used to represent a single response from a node
 type NodeResponse struct {
 	From    string

diff --git a/serf/serf.go b/serf/serf.go
@@ -930,7 +930,7 @@ func (s *Serf) handleNodeJoin(n *memberlist.Node) {
 		member = &memberState{
 			Member: Member{
 				Name:   n.Name,
-				Addr:   net.IP(n.Addr),
+				Addr:   n.Addr,
 				Port:   n.Port,
 				Tags:   s.decodeTags(n.Meta),
 				Status: StatusAlive,
@@ -1056,7 +1056,7 @@ func (s *Serf) handleNodeUpdate(n *memberlist.Node) {
 	}
 
 	// Update the member attributes
-	member.Addr = net.IP(n.Addr)
+	member.Addr = n.Addr
 	member.Port = n.Port
 	member.Tags = s.decodeTags(n.Meta)
 
@@ -1424,11 +1424,9 @@ func (s *Serf) handleQueryResponse(resp *messageQueryResponse) {
 		}
 
 		metrics.IncrCounter([]string{"serf", "query_acks"}, 1)
-		select {
-		case query.ackCh <- resp.From:
-			query.acks[resp.From] = struct{}{}
-		default:
-			s.logger.Printf("[WARN] serf: Failed to deliver query ack, dropping")
+		err := query.sendAck(resp)
+		if err != nil {
+			s.logger.Printf("[WARN] %v", err)
 		}
 	} else {
 		// Exit early if this is a duplicate response

diff --git a/website/source/docs/internals/coordinates.html.markdown b/website/source/docs/internals/coordinates.html.markdown
@@ -74,7 +74,7 @@ a way that direct pings between all possible pairs of nodes never would.
 Guided by the follow-on papers cited above, there were several enhancements
 added to Serf's implementation on top of the core Vivaldi algorithm:
 
-* A non-Euclidean "height" term was added to the network coordinates in order to
+* A non-Euclidean "height" term was added to the network coordinates in order
   to help model real-world situations. In particular, height helps model access
   links that nodes have to the rest of the network, such as the fixed latency
   overhead of a virtual machine through a hypervisor to a network card.