work on new constructors

.gitignore

@@ -0,0 +1,2 @@
+review.md
+SAMv3.md

Makefile

@@ -0,0 +1,2 @@
+fmt:
+	find . -name '*.go' -exec gofumpt -w -s -extra {} \;

common/SAM.go

@@ -13,49 +13,6 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
-// Creates a new controller for the I2P routers SAM bridge.
-func OldNewSAM(address string) (*SAM, error) {
-	log.WithField("address", address).Debug("Creating new SAM instance")
-	var s SAM
-	// TODO: clean this up by refactoring the connection setup and error handling logic
-	conn, err := net.Dial("tcp", address)
-	if err != nil {
-		log.WithError(err).Error("Failed to dial SAM address")
-		return nil, fmt.Errorf("error dialing to address '%s': %w", address, err)
-	}
-	if _, err := conn.Write(s.SAMEmit.HelloBytes()); err != nil {
-		log.WithError(err).Error("Failed to write hello message")
-		conn.Close()
-		return nil, fmt.Errorf("error writing to address '%s': %w", address, err)
-	}
-	buf := make([]byte, 256)
-	n, err := conn.Read(buf)
-	if err != nil {
-		log.WithError(err).Error("Failed to read SAM response")
-		conn.Close()
-		return nil, fmt.Errorf("error reading onto buffer: %w", err)
-	}
-	if strings.Contains(string(buf[:n]), HELLO_REPLY_OK) {
-		log.Debug("SAM hello successful")
-		s.SAMEmit.I2PConfig.SetSAMAddress(address)
-		s.Conn = conn
-		s.SAMResolver, err = NewSAMResolver(&s)
-		if err != nil {
-			log.WithError(err).Error("Failed to create SAM resolver")
-			return nil, fmt.Errorf("error creating resolver: %w", err)
-		}
-		return &s, nil
-	} else if string(buf[:n]) == HELLO_REPLY_NOVERSION {
-		log.Error("SAM bridge does not support SAMv3")
-		conn.Close()
-		return nil, fmt.Errorf("That SAM bridge does not support SAMv3.")
-	} else {
-		log.WithField("response", string(buf[:n])).Error("Unexpected SAM response")
-		conn.Close()
-		return nil, fmt.Errorf("%s", string(buf[:n]))
-	}
-}
-
 func (sam *SAM) Keys() (k *i2pkeys.I2PKeys) {
 	// TODO: copy them?
 	log.Debug("Retrieving SAM keys")
@@ -71,6 +28,7 @@ func (sam *SAM) ReadKeys(r io.Reader) (err error) {
 	if err == nil {
 		log.Debug("Keys loaded successfully")
 		sam.SAMEmit.I2PConfig.DestinationKeys = &keys
+		return
 	}
 	log.WithError(err).Error("Failed to load keys")
 	return

common/config.go

@@ -38,25 +38,25 @@ func (f *I2PConfig) Sam() string {
 // SetSAMAddress sets the SAM bridge host and port from a combined address string.
 // If no address is provided, it sets default values for the host and port.
 func (f *I2PConfig) SetSAMAddress(addr string) {
-	// Set default values
-	f.SamHost = "127.0.0.1"
-	f.SamPort = 7656
-
-	// Split address into host and port components
-	host, port, err := net.SplitHostPort(addr)
-	if err != nil {
-		// If error occurs, assume only host is provided
-		f.SamHost = addr
-	} else {
-		f.SamHost = host
-		f.SamPort, _ = strconv.Atoi(port)
+	if addr == "" {
+		f.SamHost = "127.0.0.1"
+		f.SamPort = 7656
+		return
 	}
 
-	// Log the configured SAM address
-	log.WithFields(logrus.Fields{
-		"host": f.SamHost,
-		"port": f.SamPort,
-	}).Debug("SAM address set")
+	host, port, err := net.SplitHostPort(addr)
+	if err != nil {
+		// Only set host if it looks valid
+		if net.ParseIP(addr) != nil || !strings.Contains(addr, ":") {
+			f.SamHost = addr
+		}
+		return
+	}
+
+	f.SamHost = host
+	if p, err := strconv.Atoi(port); err == nil && p > 0 && p < 65536 {
+		f.SamPort = p
+	}
 }
 
 // ID returns the tunnel name as a formatted string. If no tunnel name is set,

common/new.go

@@ -0,0 +1,81 @@
+package common
+
+import (
+	"fmt"
+	"net"
+	"strings"
+)
+
+// Creates a new controller for the I2P routers SAM bridge.
+func OldNewSAM(address string) (*SAM, error) {
+	log.WithField("address", address).Debug("Creating new SAM instance")
+	var s SAM
+	// TODO: clean this up by refactoring the connection setup and error handling logic
+	conn, err := net.Dial("tcp", address)
+	if err != nil {
+		log.WithError(err).Error("Failed to dial SAM address")
+		return nil, fmt.Errorf("error dialing to address '%s': %w", address, err)
+	}
+	if _, err := conn.Write(s.SAMEmit.HelloBytes()); err != nil {
+		log.WithError(err).Error("Failed to write hello message")
+		conn.Close()
+		return nil, fmt.Errorf("error writing to address '%s': %w", address, err)
+	}
+	buf := make([]byte, 256)
+	n, err := conn.Read(buf)
+	if err != nil {
+		log.WithError(err).Error("Failed to read SAM response")
+		conn.Close()
+		return nil, fmt.Errorf("error reading onto buffer: %w", err)
+	}
+	if strings.Contains(string(buf[:n]), HELLO_REPLY_OK) {
+		log.Debug("SAM hello successful")
+		s.SAMEmit.I2PConfig.SetSAMAddress(address)
+		s.Conn = conn
+		s.SAMResolver, err = NewSAMResolver(&s)
+		if err != nil {
+			log.WithError(err).Error("Failed to create SAM resolver")
+			return nil, fmt.Errorf("error creating resolver: %w", err)
+		}
+		return &s, nil
+	} else if string(buf[:n]) == HELLO_REPLY_NOVERSION {
+		log.Error("SAM bridge does not support SAMv3")
+		conn.Close()
+		return nil, fmt.Errorf("That SAM bridge does not support SAMv3.")
+	} else {
+		log.WithField("response", string(buf[:n])).Error("Unexpected SAM response")
+		conn.Close()
+		return nil, fmt.Errorf("%s", string(buf[:n]))
+	}
+}
+
+func NewSAM(address string) (*SAM, error) {
+	logger := log.WithField("address", address)
+	logger.Debug("Creating new SAM instance")
+
+	conn, err := connectToSAM(address)
+	if err != nil {
+		return nil, err
+	}
+	defer func() {
+		if err != nil {
+			conn.Close()
+		}
+	}()
+
+	s := &SAM{
+		Conn: conn,
+	}
+
+	if err = sendHelloAndValidate(conn, s); err != nil {
+		return nil, err
+	}
+
+	s.SAMEmit.I2PConfig.SetSAMAddress(address)
+
+	if s.SAMResolver, err = NewSAMResolver(s); err != nil {
+		return nil, fmt.Errorf("failed to create SAM resolver: %w", err)
+	}
+
+	return s, nil
+}

common/types.go

@@ -1,8 +1,10 @@
 package common
 
 import (
+	"context"
 	"fmt"
 	"net"
+	"time"
 
 	"github.com/go-i2p/i2pkeys"
 )
@@ -64,6 +66,11 @@ type SAM struct {
 	SAMEmit
 	*SAMResolver
 	net.Conn
+
+	// Timeout for SAM connections
+	Timeout time.Duration
+	// Context for control of lifecycle
+	Context context.Context
 }
 
 type SAMResolver struct {

common/util.go

@@ -37,11 +37,12 @@ func SplitHostPort(hostport string) (string, string, error) {
 	return host, port, nil
 }
 
+var randSource = rand.NewSource(time.Now().UnixNano())
+var randGen = rand.New(randSource)
+
 func RandPort() string {
 	for {
-		s := rand.NewSource(time.Now().UnixNano())
-		r := rand.New(s)
-		p := r.Intn(55534) + 10000
+		p := randGen.Intn(55534) + 10000
 		port := strconv.Itoa(p)
 		if l, e := net.Listen("tcp", net.JoinHostPort("localhost", port)); e != nil {
 			continue
@@ -54,6 +55,5 @@ func RandPort() string {
 				return strconv.Itoa(l.Addr().(*net.UDPAddr).Port)
 			}
 		}
-
 	}
 }