fix: refactor relay URLs validation

rmb-sdk-go/peer/peer.go

@@ -11,7 +11,6 @@ import (
 	"fmt"
 	"net/url"
 	"slices"
-	"sort"
 	"strings"
 	"time"
 
@@ -145,34 +144,59 @@ func generateSecureKey(identity substrate.Identity) (*secp256k1.PrivateKey, erro
 	priv := secp256k1.PrivKeyFromBytes(keyPair.Seed())
 	return priv, nil
 }
-
-// getRelayConnections tries to connect to all relays and returns only the successful ones
-// getRelayConnections returns InnerConnections for all valid relay URLs
-func getRelayConnections(relayURLs []string, identity substrate.Identity, session string, twinID uint32) ([]string, []InnerConnection, error) {
-	var validRelayURLs []string
-	var connections []InnerConnection
+func validateRelayURLs(relayURLs []string) ([]*url.URL, error) {
+	var validRelayURLs []*url.URL
 
 	for _, relayURL := range relayURLs {
-		parsedURL, err := url.Parse(relayURL)
+		parsedURL, err := url.Parse(strings.ToLower(relayURL))
 		if err != nil {
 			log.Warn().Err(err).Str("url", relayURL).Msg("failed to parse relay URL, skipping")
 			continue
 		}
-		validRelayURLs = append(validRelayURLs, parsedURL.Host)
-		conn := NewConnection(identity, relayURL, session, twinID)
-		connections = append(connections, conn)
+		// make sure it is ws or wss
+		if parsedURL.Scheme != "ws" && parsedURL.Scheme != "wss" {
+			log.Warn().Str("url", relayURL).Msg("relay URL must be ws or wss, skipping")
+			continue
+		}
+		// make sure Hostname is not empty
+		if parsedURL.Hostname() == "" {
+			log.Warn().Str("url", relayURL).Msg("relay URL must have a hostname, skipping")
+			continue
+		}
+		validRelayURLs = append(validRelayURLs, parsedURL)
 	}
 
-	if len(connections) == 0 {
-		return nil, nil, ErrNoValidRelayURLs
+	if len(validRelayURLs) == 0 {
+		return nil, ErrNoValidRelayURLs
 	}
 
-	sort.Slice(validRelayURLs, func(i, j int) bool {
-		return strings.ToLower(validRelayURLs[i]) < strings.ToLower(validRelayURLs[j])
+	slices.SortFunc(validRelayURLs, func(a, b *url.URL) int {
+		return strings.Compare(a.Hostname(), b.Hostname())
+	})
+	validRelayURLs = slices.CompactFunc(validRelayURLs, func(a, b *url.URL) bool {
+		return a.Hostname() == b.Hostname()
 	})
-	validRelayURLs = slices.Compact(validRelayURLs)
+	return validRelayURLs, nil
+}
+
+// getRelayConnections tries to connect to all relays and returns only the successful ones
+// getRelayConnections returns InnerConnections for all valid relay URLs
+func getRelayConnections(relayURLs []string, identity substrate.Identity, session string, twinID uint32) ([]string, []InnerConnection, error) {
+	validRelayURLs, err := validateRelayURLs(relayURLs)
+	if err != nil {
+		return nil, nil, err
+	}
+	connections := make([]InnerConnection, 0, len(validRelayURLs))
+	hosts := make([]string, 0, len(validRelayURLs))
+
+	for _, relayURL := range validRelayURLs {
+		conn := NewConnection(identity, relayURL.String(), session, twinID)
+		connections = append(connections, conn)
+		host := relayURL.Hostname()
+		hosts = append(hosts, host)
+	}
 
-	return validRelayURLs, connections, nil
+	return hosts, connections, nil
 }
 
 func getIdentity(keytype string, mnemonics string) (substrate.Identity, error) {
@@ -266,7 +290,7 @@ func NewPeer(
 		publicKey = privKey.PubKey().SerializeCompressed()
 	}
 
-	relayURLs, conns, err := getRelayConnections(cfg.relayURLs, identity, cfg.session, twin.ID)
+	hosts, conns, err := getRelayConnections(cfg.relayURLs, identity, cfg.session, twin.ID)
 	if err != nil {
 		return nil, err
 	}
@@ -281,7 +305,7 @@ func NewPeer(
 		}
 	}
 
-	joinURLs := strings.Join(relayURLs, "_")
+	joinURLs := strings.Join(hosts, "_")
 	if !bytes.Equal(twin.E2EKey, publicKey) || twin.Relay == nil || joinURLs != *twin.Relay {
 		log.Info().Str("Relay url/s", joinURLs).Msg("twin relay/public key didn't match, updating on chain ...")
 		if _, err = subConn.UpdateTwin(identity, joinURLs, publicKey); err != nil {
@@ -320,7 +344,7 @@ func NewPeer(
 		cons:    cons,
 		handler: handler,
 		encoder: cfg.encoder,
-		relays:  relayURLs,
+		relays:  hosts,
 	}
 
 	go cl.process(ctx)

rmb-sdk-go/peer/relay_urls_test.go

@@ -0,0 +1,119 @@
+package peer
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestValidateRelayURLs_DedupSort(t *testing.T) {
+	inputs := []string{
+		"ws://Relay.Grid.tf",           // same host different case, ws
+		"wss://relay.grid.tf",          // same host wss
+		"wss://relay.grid.tf:443",      // same host with default port -> ignored for dedup
+		"ws://relay.grid.tf/some/path", // same host path -> ignored for dedup
+		"wss://beta.grid.tf",           // distinct host
+		"http://not-allowed.example",   // invalid scheme
+		"://bad://url",                 // unparsable
+		"wss://relay.dev.grid.tf:443",  // another host
+		"ws://relay.dev.grid.tf",       // duplicate host, ws should be ignored in favor of wss entry
+	}
+
+	urls, err := validateRelayURLs(inputs)
+	if err != nil {
+		// should not error; we have valid entries
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// Expect unique hostnames (lowercased) sorted: [beta.grid.tf relay.dev.grid.tf relay.grid.tf]
+	if len(urls) != 3 {
+		t.Fatalf("expected 3 unique hosts, got %d", len(urls))
+	}
+
+	expectedHosts := []string{"beta.grid.tf", "relay.dev.grid.tf", "relay.grid.tf"}
+	gotHosts := []string{urls[0].Hostname(), urls[1].Hostname(), urls[2].Hostname()}
+	if !reflect.DeepEqual(expectedHosts, gotHosts) {
+		t.Fatalf("hosts mismatch\nexpected: %v\n     got: %v", expectedHosts, gotHosts)
+	}
+}
+
+func TestValidateRelayURLs_DedupSort_NonAdjacent(t *testing.T) {
+	inputs := []string{
+		"ws://Relay.Grid.tf",           // same host different case, ws
+		"wss://beta.grid.tf",           // distinct host
+		"http://not-allowed.example",   // invalid scheme
+		"://bad://url",                 // unparsable
+		"wss://relay.dev.grid.tf:443",  // another host
+		"wss://relay.grid.tf",          // same host wss
+		"ws://relay.dev.grid.tf",       // duplicate host, ws should be ignored in favor of wss entry
+		"wss://relay.grid.tf:443",      // same host with default port -> ignored for dedup
+		"ws://relay.grid.tf/some/path", // same host path -> ignored for dedup
+	}
+
+	urls, err := validateRelayURLs(inputs)
+	if err != nil {
+		// should not error; we have valid entries
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// Expect unique hostnames (lowercased) sorted: [beta.grid.tf relay.dev.grid.tf relay.grid.tf]
+	if len(urls) != 3 {
+		t.Fatalf("expected 3 unique hosts, got %d", len(urls))
+	}
+
+	expectedHosts := []string{"beta.grid.tf", "relay.dev.grid.tf", "relay.grid.tf"}
+	gotHosts := []string{urls[0].Hostname(), urls[1].Hostname(), urls[2].Hostname()}
+	if !reflect.DeepEqual(expectedHosts, gotHosts) {
+		t.Fatalf("hosts mismatch\nexpected: %v\n     got: %v", expectedHosts, gotHosts)
+	}
+}
+
+func TestValidateRelayURLs_AllInvalid(t *testing.T) {
+	inputs := []string{"http://a", "ftp://b", "::::"}
+	_, err := validateRelayURLs(inputs)
+	if err == nil {
+		t.Fatalf("expected error for no valid relay URLs")
+	}
+	if err != ErrNoValidRelayURLs {
+		t.Fatalf("expected ErrNoValidRelayURLs, got %v", err)
+	}
+}
+
+func TestGetRelayConnections_HostsNormalizedAndAligned(t *testing.T) {
+	inputs := []string{
+		"ws://relay.grid.tf:443/path", // same host as below, different scheme/port/path
+		"wss://relay.grid.tf",
+		"wss://Relay.Dev.Grid.TF", // case-insensitive host
+	}
+
+	hosts, conns, err := getRelayConnections(inputs, nil, "sess", 42)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if len(hosts) != len(conns) {
+		t.Fatalf("hosts and connections length mismatch: %d vs %d", len(hosts), len(conns))
+	}
+
+	expectedHosts := []string{"relay.dev.grid.tf", "relay.grid.tf"}
+	if !reflect.DeepEqual(expectedHosts, hosts) {
+		t.Fatalf("normalized hosts mismatch\nexpected: %v\n     got: %v", expectedHosts, hosts)
+	}
+}
+
+func TestValidateRelayURLs_DeterministicSort(t *testing.T) {
+	inputs := []string{
+		"wss://relay.grid.tf",
+		"wss://relay.dev.grid.tf",
+		"wss://beta.grid.tf",
+	}
+
+	urls, err := validateRelayURLs(inputs)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	expectedHosts := []string{"beta.grid.tf", "relay.dev.grid.tf", "relay.grid.tf"}
+	gotHosts := []string{urls[0].Hostname(), urls[1].Hostname(), urls[2].Hostname()}
+	if !reflect.DeepEqual(expectedHosts, gotHosts) {
+		t.Fatalf("hosts mismatch\nexpected: %v\n     got: %v", expectedHosts, gotHosts)
+	}
+}