X Draft autodetection of which encoding NEO server uses

go/neo/client_test.go

@@ -40,6 +40,7 @@ import (
 	"lab.nexedi.com/kirr/go123/xsync"
 
 	"lab.nexedi.com/kirr/neo/go/neo/neonet"
+	"lab.nexedi.com/kirr/neo/go/neo/proto"
 	bsqlite "lab.nexedi.com/kirr/neo/go/neo/storage/sqlite"
 )
 
@@ -489,7 +490,7 @@ func withNEO(t *testing.T, f func(t *testing.T, nsrv NEOSrv, ndrv *Client), optv
 	withNEOSrv(t, func(t *testing.T, nsrv NEOSrv) {
 		t.Helper()
 		X := xtesting.FatalIf(t)
-		// TODO test for enc=(M|N)  (XXX M|N only for NEO/go as NEO/py does not support autodetect)
+
 		ndrv, _, err := neoOpen(nsrv.URL(),
 					&zodb.DriverOptions{ReadOnly: true}); X(err)
 		defer func() {
@@ -501,7 +502,46 @@ func withNEO(t *testing.T, f func(t *testing.T, nsrv NEOSrv, ndrv *Client), optv
 }
 
 
-// XXX TestHandshake ?
+// TestEncAutodetect verifies that client can autodetect server encoding.
+func TestEncAutodetect(t *testing.T) {
+	withNEOSrv(t, func(t *testing.T, nsrv NEOSrv) {
+		// test client with N,M and M,N encoding try orders. This
+		// verifies that encoding autodetection actually works when
+		// NEO/go client connects NEO/py server.
+		//
+		// For example if NEO/py server implments only M encoding,
+		// testing with N,M verifies retry after handshaking with
+		// enc=N.
+		encTryOrder0 := neonet.DialEncTryOrder
+		defer func() {
+			neonet.DialEncTryOrder = encTryOrder0
+		}()
+
+		for i := range encTryOrder0 {
+			var encv []proto.Encoding
+			encv = append(encv, encTryOrder0[i])
+			encv = append(encv, encTryOrder0[:i]...)
+			encv = append(encv, encTryOrder0[i+1:]...)
+
+			encvs := ""
+			for j, enc := range encv {
+				if j > 0 {
+					encvs += ","
+				}
+				encvs += fmt.Sprintf("%c", enc)
+			}
+
+			t.Run(fmt.Sprintf("dialEncTryOrder=%v", encvs), func(t *testing.T) {
+				X := xtesting.FatalIf(t)
+
+				neonet.DialEncTryOrder = encv
+				ndrv, _, err := neoOpen(nsrv.URL(),
+							&zodb.DriverOptions{ReadOnly: true}); X(err)
+				err = ndrv.Close(); X(err)
+			})
+		}
+	})
+}
 
 // XXX connect with wrong clusterName -> rejected
 

go/neo/neonet/newlink.go

@@ -23,6 +23,7 @@ package neonet
 import (
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"net"
@@ -37,35 +38,6 @@ import (
 	"lab.nexedi.com/kirr/neo/go/neo/proto"
 )
 
-// encTryOrder is the order of trials for encoding when establishing a NEO link from client side.
-//
-// NEO/go server autodetects client preferred encoding and adjusts to that, but
-// NEO/py rejects connections if client encoding does not exactly match server.
-//
-// To autodetect what NEO/py server uses DialLink retries dial + handshake with
-// client-preferred encodings in the following order.
-//
-// NOTE tests change this to verify that autodetection of NEO/py server
-// encoding actually works.
-var encTryOrder = []proto.Encoding{'N', 'M'}
-
-/*
-// encDefault is default encoding to use.
-// XXX we don't need this? (just set encDefault = 'M')
-var encDefault = proto.Encoding('N') // XXX = 'M' instead?
-func init() {
-	e := os.Getenv("NEO_ENCODING")
-	switch e {
-	case "":  // not set
-	case "N": fallthrough
-	case "M": encDefault = proto.Encoding(e[0])
-	default:
-		fmt.Fprintf(os.Stderr, "E: $NEO_ENCODING=%q - invalid -> abort", e)
-		os.Exit(1)
-	}
-}
-*/
-
 // ---- Handshake ----
 
 // XXX _Handshake{Client,Server} may be needed to become public in case when we have already
@@ -73,7 +45,7 @@ func init() {
 // do not have such uses.
 
 func _HandshakeClient(ctx context.Context, conn net.Conn) (*NodeLink, error) {
-	return handshakeClient(ctx, conn, proto.Version, encTryOrder[0])
+	return handshakeClient(ctx, conn, proto.Version, DialEncTryOrder[0])
 }
 
 func _HandshakeServer(ctx context.Context, conn net.Conn) (*NodeLink, error) {
@@ -296,17 +268,41 @@ func rxHello(errctx string, rx *fwd.Reader) (enc proto.Encoding, version uint32,
 
 // ---- Dial & Listen at NodeLink level ----
 
+// DialEncTryOrder is the order of trials for encoding when establishing a NEO link from client side.
+//
+// NEO/go server autodetects client preferred encoding and adjusts to that, but
+// NEO/py rejects connections if client encoding does not exactly match server.
+//
+// To autodetect what NEO/py server uses DialLink retries dial + handshake with
+// client-preferred encodings in the order specified by DialEncTryOrder.
+//
+// NOTE tests change this to verify that autodetection of NEO/py server
+// encoding actually works.
+//
+// XXX unexport
+var DialEncTryOrder = []proto.Encoding{'N', 'M'}
+
 // DialLink connects to address on given network, performs NEO protocol
 // handshake and wraps the connection as NodeLink.
-func DialLink(ctx context.Context, net xnet.Networker, addr string) (*NodeLink, error) {
-	peerConn, err := net.Dial(ctx, addr)
-	if err != nil {
-		return nil, err
+func DialLink(ctx context.Context, net xnet.Networker, addr string) (link *NodeLink, err error) {
+	for _, enc := range DialEncTryOrder {
+		peerConn, err := net.Dial(ctx, addr)
+		if err != nil {
+			return nil, err
+		}
+
+		link, err = handshakeClient(ctx, peerConn, proto.Version, enc)
+
+		// NEO/py closes connection if it sees unexpected magic, version, etc.
+		// -> in such case retry with next encoding trying to autodetect and match server.
+		// -> stop trying on success, or on any other error.
+		if err == nil || !errors.Is(err, io.ErrUnexpectedEOF) {
+			break
+		}
 	}
 
-	// TODO if handshake fails with "closed" (= might be unexpected encoding)
-	//      -> try redial and handshaking with different encoding (= autodetect encoding)
-	return handshakeClient(ctx, peerConn, proto.Version, encTryOrder[0])
+	// either link is established or the error indicates we should not retry anymore
+	return link, err
 }
 
 // ListenLink starts listening on laddr for incoming connections and wraps them as NodeLink.