From 0e35eb2df38b047216e11ffbc73ddeba05c95582 Mon Sep 17 00:00:00 2001 From: Call me Phil Date: Wed, 12 Mar 2025 21:39:23 +0000 Subject: [PATCH 1/2] Clean up and fixes - Define and use a few constants for the sake of code maintainability - Change log package name to logger so it better reflects its purpose - Fix depredations - Drop unused private methods and constants - Create an auxiliary func newFromPrimary() to reduce code duplicacy. Also straight away type conversion doesn't seem to type-check. Got to explicitly create a new struct instance. --- common/config.go | 6 +++--- common/const.go | 12 ++++++++++++ common/emit-options.go | 22 +++++++++------------- common/log.go | 2 +- datagram/log.go | 2 +- log.go | 4 ++-- {log => logger}/log.go | 6 +++--- primary.go | 13 ------------- primary/log.go | 2 +- primary/stream.go | 26 ++++++++++++++------------ raw/log.go | 2 +- sam3.go | 12 ------------ stream.go | 12 ------------ stream/log.go | 2 +- 14 files changed, 48 insertions(+), 75 deletions(-) rename {log => logger}/log.go (93%) diff --git a/common/config.go b/common/config.go index 7ed9987..c7d0a3d 100644 --- a/common/config.go +++ b/common/config.go @@ -404,13 +404,13 @@ func (f *I2PConfig) Print() []string { // Accesslisttype returns the I2CP access list configuration string based on the AccessListType setting func (f *I2PConfig) Accesslisttype() string { switch f.AccessListType { - case "whitelist": + case ACCESS_TYPE_WHITELIST: log.Debug("Access list type set to whitelist") return fmt.Sprintf("i2cp.enableAccessList=true") - case "blacklist": + case ACCESS_TYPE_BLACKLIST: log.Debug("Access list type set to blacklist") return fmt.Sprintf("i2cp.enableBlackList=true") - case "none": + case ACCESS_TYPE_NONE: log.Debug("Access list type set to none") return "" default: diff --git a/common/const.go b/common/const.go index 33d25f7..1b65da3 100644 --- a/common/const.go +++ b/common/const.go @@ -32,3 +32,15 @@ const ( HELLO_REPLY_OK = "HELLO REPLY RESULT=OK" HELLO_REPLY_NOVERSION = "HELLO REPLY RESULT=NOVERSION\n" ) + +const ( + SESSION_STYLE_STREAM = "STREAM" + SESSION_STYLE_DATAGRAM = "DATAGRAM" + SESSION_STYLE_RAW = "RAW" +) + +const ( + ACCESS_TYPE_WHITELIST = "whitelist" + ACCESS_TYPE_BLACKLIST = "blacklist" + ACCESS_TYPE_NONE = "none" +) diff --git a/common/emit-options.go b/common/emit-options.go index 973ad06..c65de70 100644 --- a/common/emit-options.go +++ b/common/emit-options.go @@ -14,15 +14,15 @@ type Option func(*SAMEmit) error // SetType sets the type of the forwarder server func SetType(s string) func(*SAMEmit) error { return func(c *SAMEmit) error { - if s == "STREAM" { + if s == SESSION_STYLE_STREAM { c.Style = s log.WithField("style", s).Debug("Set session style") return nil - } else if s == "DATAGRAM" { + } else if s == SESSION_STYLE_DATAGRAM { c.Style = s log.WithField("style", s).Debug("Set session style") return nil - } else if s == "RAW" { + } else if s == SESSION_STYLE_RAW { c.Style = s log.WithField("style", s).Debug("Set session style") return nil @@ -399,24 +399,20 @@ func SetCloseIdleTimeMs(u int) func(*SAMEmit) error { // SetAccessListType tells the system to treat the AccessList as a whitelist func SetAccessListType(s string) func(*SAMEmit) error { return func(c *SAMEmit) error { - if s == "whitelist" { - c.I2PConfig.AccessListType = "whitelist" + if s == ACCESS_TYPE_WHITELIST { + c.I2PConfig.AccessListType = ACCESS_TYPE_WHITELIST log.Debug("Set access list type to whitelist") return nil - } else if s == "blacklist" { - c.I2PConfig.AccessListType = "blacklist" + } else if s == ACCESS_TYPE_BLACKLIST { + c.I2PConfig.AccessListType = ACCESS_TYPE_BLACKLIST log.Debug("Set access list type to blacklist") return nil - } else if s == "none" { - c.I2PConfig.AccessListType = "" - log.Debug("Set access list type to none") - return nil - } else if s == "" { + } else if s == ACCESS_TYPE_NONE || s == "" { c.I2PConfig.AccessListType = "" log.Debug("Set access list type to none") return nil } - return fmt.Errorf("Invalid Access list type(whitelist, blacklist, none)") + return fmt.Errorf("Invalid Access list type (whitelist, blacklist, none)") } } diff --git a/common/log.go b/common/log.go index a08b612..592513f 100644 --- a/common/log.go +++ b/common/log.go @@ -1,6 +1,6 @@ package common -import logger "github.com/go-i2p/go-sam-go/log" +import logger "github.com/go-i2p/go-sam-go/logger" var log = logger.GetSAM3Logger() diff --git a/datagram/log.go b/datagram/log.go index a1ea712..41ba34d 100644 --- a/datagram/log.go +++ b/datagram/log.go @@ -1,6 +1,6 @@ package datagram -import logger "github.com/go-i2p/go-sam-go/log" +import logger "github.com/go-i2p/go-sam-go/logger" var log = logger.GetSAM3Logger() diff --git a/log.go b/log.go index bb8616a..f6b3fe0 100644 --- a/log.go +++ b/log.go @@ -2,7 +2,7 @@ package sam3 import ( - "io/ioutil" + "io" "os" "strings" "sync" @@ -19,7 +19,7 @@ func InitializeSAM3Logger() { once.Do(func() { log = logrus.New() // We do not want to log by default - log.SetOutput(ioutil.Discard) + log.SetOutput(io.Discard) log.SetLevel(logrus.PanicLevel) // Check if DEBUG_I2P is set if logLevel := os.Getenv("DEBUG_I2P"); logLevel != "" { diff --git a/log/log.go b/logger/log.go similarity index 93% rename from log/log.go rename to logger/log.go index b186257..4df2c51 100644 --- a/log/log.go +++ b/logger/log.go @@ -1,7 +1,7 @@ -package log +package logger import ( - "io/ioutil" + "io" "os" "strings" "sync" @@ -18,7 +18,7 @@ func InitializeSAM3Logger() { once.Do(func() { log = logrus.New() // We do not want to log by default - log.SetOutput(ioutil.Discard) + log.SetOutput(io.Discard) log.SetLevel(logrus.PanicLevel) // Check if DEBUG_I2P is set if logLevel := os.Getenv("DEBUG_I2P"); logLevel != "" { diff --git a/primary.go b/primary.go index fcd0f99..5001a77 100644 --- a/primary.go +++ b/primary.go @@ -2,10 +2,6 @@ package sam3 import ( - "math/rand" - "strconv" - "time" - "github.com/go-i2p/go-sam-go/primary" ) @@ -13,15 +9,6 @@ const ( session_ADDOK = "SESSION STATUS RESULT=OK" ) -func randport() string { - s := rand.NewSource(time.Now().UnixNano()) - r := rand.New(s) - p := r.Intn(55534) + 10000 - port := strconv.Itoa(p) - log.WithField("port", port).Debug("Generated random port") - return strconv.Itoa(p) -} - // Represents a primary session. type PrimarySession struct { *primary.PrimarySession diff --git a/primary/log.go b/primary/log.go index ea6a3a0..c4cf94a 100644 --- a/primary/log.go +++ b/primary/log.go @@ -1,6 +1,6 @@ package primary -import logger "github.com/go-i2p/go-sam-go/log" +import logger "github.com/go-i2p/go-sam-go/logger" var log = logger.GetSAM3Logger() diff --git a/primary/stream.go b/primary/stream.go index dc35831..d455cbe 100644 --- a/primary/stream.go +++ b/primary/stream.go @@ -1,6 +1,8 @@ package primary import ( + "net" + "github.com/go-i2p/go-sam-go/common" "github.com/go-i2p/go-sam-go/stream" "github.com/sirupsen/logrus" @@ -15,11 +17,7 @@ func (sam *PrimarySession) NewStreamSubSession(id string) (*stream.StreamSession log.WithError(err).Error("Failed to create new generic sub-session") return nil, err } - streamSession := &stream.StreamSession{ - SAM: (*stream.SAM)(sam.SAM), - } - streamSession.Conn = conn - return streamSession, nil + return newFromPrimary(sam, conn), nil } // Creates a new stream.StreamSession with the I2CP- and streaminglib options as @@ -33,11 +31,7 @@ func (sam *PrimarySession) NewUniqueStreamSubSession(id string) (*stream.StreamS } fromPort, toPort := common.RandPort(), common.RandPort() log.WithFields(logrus.Fields{"fromPort": fromPort, "toPort": toPort}).Debug("Generated random ports") - streamSession := &stream.StreamSession{ - SAM: (*stream.SAM)(sam.SAM), - } - streamSession.Conn = conn - return streamSession, nil + return newFromPrimary(sam, conn), nil } // Creates a new stream.StreamSession with the I2CP- and streaminglib options as @@ -49,9 +43,17 @@ func (sam *PrimarySession) NewStreamSubSessionWithPorts(id, from, to string) (*s log.WithError(err).Error("Failed to create new generic sub-session with signature and ports") return nil, err } + return newFromPrimary(sam, conn), nil +} + +func newFromPrimary(sam *PrimarySession, conn net.Conn) *stream.StreamSession { streamSession := &stream.StreamSession{ - SAM: (*stream.SAM)(sam.SAM), + SAM: &stream.SAM{ + SAM: (*common.SAM)(sam.SAM), + }, } streamSession.Conn = conn - return streamSession, nil + + return streamSession + } diff --git a/raw/log.go b/raw/log.go index cf55c3d..068cee8 100644 --- a/raw/log.go +++ b/raw/log.go @@ -1,6 +1,6 @@ package raw -import logger "github.com/go-i2p/go-sam-go/log" +import logger "github.com/go-i2p/go-sam-go/logger" var log = logger.GetSAM3Logger() diff --git a/sam3.go b/sam3.go index 860a5a0..6c7b125 100644 --- a/sam3.go +++ b/sam3.go @@ -11,10 +11,6 @@ import ( "github.com/go-i2p/i2pkeys" ) -func init() { - InitializeSAM3Logger() -} - // Used for controlling I2Ps SAMv3. type SAM struct { *common.SAM @@ -60,14 +56,6 @@ func (s *SAM) NewPrimarySession(id string, keys i2pkeys.I2PKeys, options []strin return &primarySession, nil } -const ( - session_OK = "SESSION STATUS RESULT=OK DESTINATION=" - session_DUPLICATE_ID = "SESSION STATUS RESULT=DUPLICATED_ID\n" - session_DUPLICATE_DEST = "SESSION STATUS RESULT=DUPLICATED_DEST\n" - session_INVALID_KEY = "SESSION STATUS RESULT=INVALID_KEY\n" - session_I2P_ERROR = "SESSION STATUS RESULT=I2P_ERROR MESSAGE=" -) - const ( Sig_NONE = "SIGNATURE_TYPE=EdDSA_SHA512_Ed25519" Sig_DSA_SHA1 = "SIGNATURE_TYPE=DSA_SHA1" diff --git a/stream.go b/stream.go index 58e7881..dd30506 100644 --- a/stream.go +++ b/stream.go @@ -2,8 +2,6 @@ package sam3 import ( - "time" - "github.com/go-i2p/go-sam-go/stream" ) @@ -18,13 +16,3 @@ func (s *StreamSession) Cancel() chan *StreamSession { ch <- s return ch }*/ - -func minNonzeroTime(a, b time.Time) time.Time { - if a.IsZero() { - return b - } - if b.IsZero() || a.Before(b) { - return a - } - return b -} diff --git a/stream/log.go b/stream/log.go index 01e5219..5f4e8c5 100644 --- a/stream/log.go +++ b/stream/log.go @@ -1,6 +1,6 @@ package stream -import logger "github.com/go-i2p/go-sam-go/log" +import logger "github.com/go-i2p/go-sam-go/logger" var log = logger.GetSAM3Logger() From 477796de71fb1d162071eee0f74b7535cf77353f Mon Sep 17 00:00:00 2001 From: urgentquest Date: Mon, 7 Apr 2025 23:48:02 +0000 Subject: [PATCH 2/2] general code qualitity and readability. Mostlly no functional changes besides fixing potential null dereferece in SAM.go, avoiding potential (again) infitite loop in util.go. --- common/SAM.go | 7 ++++-- common/config.go | 20 ++++++++--------- common/emit-options.go | 12 +++-------- common/util.go | 10 ++++++--- config.go | 6 +++--- log.go | 49 ++++-------------------------------------- primary/stream.go | 2 -- sam3.go | 16 +++----------- suggestedOptions.go | 1 - 9 files changed, 35 insertions(+), 88 deletions(-) diff --git a/common/SAM.go b/common/SAM.go index 81831e4..8f7b7fb 100644 --- a/common/SAM.go +++ b/common/SAM.go @@ -226,6 +226,9 @@ func (sam *SAM) NewGenericSessionWithSignatureAndPorts(style, id, from, to strin // close this sam session func (sam *SAM) Close() error { - log.Debug("Closing SAM session") - return sam.Conn.Close() + if sam.Conn != nil { + log.Debug("Closing SAM session") + return sam.Conn.Close() + } + return nil } diff --git a/common/config.go b/common/config.go index c7d0a3d..80a5ced 100644 --- a/common/config.go +++ b/common/config.go @@ -237,9 +237,9 @@ func (f *I2PConfig) SignatureType() string { // EncryptLease returns the lease set encryption configuration string // Returns "i2cp.encryptLeaseSet=true" if encryption is enabled, empty string otherwise func (f *I2PConfig) EncryptLease() string { - if f.EncryptLeaseSet == true { + if f.EncryptLeaseSet { log.Debug("Lease set encryption enabled") - return fmt.Sprintf(" i2cp.encryptLeaseSet=true ") + return " i2cp.encryptLeaseSet=true " } log.Debug("Lease set encryption not enabled") return "" @@ -262,7 +262,7 @@ func (f *I2PConfig) Reliability() string { // Reduce returns I2CP reduce-on-idle configuration settings as a string if enabled func (f *I2PConfig) Reduce() string { // If reduce idle is enabled, return formatted configuration string - if f.ReduceIdle == true { + if f.ReduceIdle { // Log the reduce idle settings being applied log.WithFields(logrus.Fields{ "reduceIdle": f.ReduceIdle, @@ -287,7 +287,7 @@ func (f *I2PConfig) Reduce() string { // Close returns I2CP close-on-idle configuration settings as a string if enabled func (f *I2PConfig) Close() string { // If close idle is enabled, return formatted configuration string - if f.CloseIdle == true { + if f.CloseIdle { // Log the close idle settings being applied log.WithFields(logrus.Fields{ "closeIdle": f.CloseIdle, @@ -312,17 +312,17 @@ func (f *I2PConfig) DoZero() string { var settings []string // Add inbound zero hop setting if enabled - if f.InAllowZeroHop == true { + if f.InAllowZeroHop { settings = append(settings, fmt.Sprintf("inbound.allowZeroHop=%t", f.InAllowZeroHop)) } // Add outbound zero hop setting if enabled - if f.OutAllowZeroHop == true { + if f.OutAllowZeroHop { settings = append(settings, fmt.Sprintf("outbound.allowZeroHop=%t", f.OutAllowZeroHop)) } // Add fast receive setting if enabled - if f.FastRecieve == true { + if f.FastRecieve { settings = append(settings, fmt.Sprintf("i2cp.fastRecieve=%t", f.FastRecieve)) } @@ -406,10 +406,10 @@ func (f *I2PConfig) Accesslisttype() string { switch f.AccessListType { case ACCESS_TYPE_WHITELIST: log.Debug("Access list type set to whitelist") - return fmt.Sprintf("i2cp.enableAccessList=true") + return "i2cp.enableAccessList=true" case ACCESS_TYPE_BLACKLIST: log.Debug("Access list type set to blacklist") - return fmt.Sprintf("i2cp.enableBlackList=true") + return "i2cp.enableBlackList=true" case ACCESS_TYPE_NONE: log.Debug("Access list type set to none") return "" @@ -469,7 +469,7 @@ func NewConfig(opts ...func(*I2PConfig) error) (*I2PConfig, error) { config.SamMax = DEFAULT_SAM_MAX config.TunName = "" config.TunType = "server" - config.Style = "STREAM" + config.Style = SESSION_STYLE_STREAM config.InLength = 3 config.OutLength = 3 config.InQuantity = 2 diff --git a/common/emit-options.go b/common/emit-options.go index c65de70..ee9f6f8 100644 --- a/common/emit-options.go +++ b/common/emit-options.go @@ -14,15 +14,9 @@ type Option func(*SAMEmit) error // SetType sets the type of the forwarder server func SetType(s string) func(*SAMEmit) error { return func(c *SAMEmit) error { - if s == SESSION_STYLE_STREAM { - c.Style = s - log.WithField("style", s).Debug("Set session style") - return nil - } else if s == SESSION_STYLE_DATAGRAM { - c.Style = s - log.WithField("style", s).Debug("Set session style") - return nil - } else if s == SESSION_STYLE_RAW { + if s == SESSION_STYLE_STREAM || + s == SESSION_STYLE_DATAGRAM || + s == SESSION_STYLE_RAW { c.Style = s log.WithField("style", s).Debug("Set session style") return nil diff --git a/common/util.go b/common/util.go index 18aba08..e82d183 100644 --- a/common/util.go +++ b/common/util.go @@ -1,6 +1,7 @@ package common import ( + "fmt" "math/rand" "net" "strconv" @@ -76,8 +77,9 @@ var ( randGen = rand.New(randSource) ) -func RandPort() string { - for { +func RandPort() (portNumber string, err error) { + maxAttempts := 30 + for range maxAttempts { p := randGen.Intn(55534) + 10000 port := strconv.Itoa(p) if l, e := net.Listen("tcp", net.JoinHostPort("localhost", port)); e != nil { @@ -88,8 +90,10 @@ func RandPort() string { continue } else { defer l.Close() - return strconv.Itoa(l.Addr().(*net.UDPAddr).Port) + return strconv.Itoa(l.Addr().(*net.UDPAddr).Port), nil } } } + + return "", fmt.Errorf("unable to find a pair of available tcp and udp ports in %v attempts", maxAttempts) } diff --git a/config.go b/config.go index 6a2854c..9bc4627 100644 --- a/config.go +++ b/config.go @@ -16,11 +16,11 @@ func NewConfig(opts ...func(*I2PConfig) error) (*I2PConfig, error) { var config I2PConfig config.SamHost = "127.0.0.1" config.SamPort = 7656 - config.SamMin = "3.0" - config.SamMax = "3.2" + config.SamMin = common.DEFAULT_SAM_MIN + config.SamMax = common.DEFAULT_SAM_MAX config.TunName = "" config.TunType = "server" - config.Style = "STREAM" + config.Style = common.SESSION_STYLE_STREAM config.InLength = 3 config.OutLength = 3 config.InQuantity = 2 diff --git a/log.go b/log.go index f6b3fe0..66ce83f 100644 --- a/log.go +++ b/log.go @@ -1,52 +1,11 @@ // package sam3 wraps the original sam3 API from github.com/go-i2p/sam3 package sam3 -import ( - "io" - "os" - "strings" - "sync" +import "github.com/go-i2p/go-sam-go/logger" - "github.com/sirupsen/logrus" -) - -var ( - log *logrus.Logger - once sync.Once -) - -func InitializeSAM3Logger() { - once.Do(func() { - log = logrus.New() - // We do not want to log by default - log.SetOutput(io.Discard) - log.SetLevel(logrus.PanicLevel) - // Check if DEBUG_I2P is set - if logLevel := os.Getenv("DEBUG_I2P"); logLevel != "" { - log.SetOutput(os.Stdout) - switch strings.ToLower(logLevel) { - case "debug": - log.SetLevel(logrus.DebugLevel) - case "warn": - log.SetLevel(logrus.WarnLevel) - case "error": - log.SetLevel(logrus.ErrorLevel) - default: - log.SetLevel(logrus.DebugLevel) - } - log.WithField("level", log.GetLevel()).Debug("Logging enabled.") - } - }) -} - -// GetSAM3Logger returns the initialized logger -func GetSAM3Logger() *logrus.Logger { - if log == nil { - InitializeSAM3Logger() - } - return log -} +var log = logger.GetSAM3Logger() func init() { - InitializeSAM3Logger() + logger.InitializeSAM3Logger() + log = logger.GetSAM3Logger() } diff --git a/primary/stream.go b/primary/stream.go index d455cbe..214e7f7 100644 --- a/primary/stream.go +++ b/primary/stream.go @@ -29,8 +29,6 @@ func (sam *PrimarySession) NewUniqueStreamSubSession(id string) (*stream.StreamS log.WithError(err).Error("Failed to create new generic sub-session") return nil, err } - fromPort, toPort := common.RandPort(), common.RandPort() - log.WithFields(logrus.Fields{"fromPort": fromPort, "toPort": toPort}).Debug("Generated random ports") return newFromPrimary(sam, conn), nil } diff --git a/sam3.go b/sam3.go index 6c7b125..c56c081 100644 --- a/sam3.go +++ b/sam3.go @@ -17,11 +17,11 @@ type SAM struct { } // Creates a new stream session by wrapping stream.NewStreamSession -func (s *SAM) NewStreamSession(param1 string, keys i2pkeys.I2PKeys, param3 []string) (*StreamSession, error) { +func (s *SAM) NewStreamSession(id string, keys i2pkeys.I2PKeys, options []string) (*StreamSession, error) { sam := &stream.SAM{ SAM: s.SAM, } - ss, err := sam.NewStreamSession(param1, keys, param3) + ss, err := sam.NewStreamSession(id, keys, options) if err != nil { return nil, err } @@ -56,18 +56,8 @@ func (s *SAM) NewPrimarySession(id string, keys i2pkeys.I2PKeys, options []strin return &primarySession, nil } -const ( - Sig_NONE = "SIGNATURE_TYPE=EdDSA_SHA512_Ed25519" - Sig_DSA_SHA1 = "SIGNATURE_TYPE=DSA_SHA1" - Sig_ECDSA_SHA256_P256 = "SIGNATURE_TYPE=ECDSA_SHA256_P256" - Sig_ECDSA_SHA384_P384 = "SIGNATURE_TYPE=ECDSA_SHA384_P384" - Sig_ECDSA_SHA512_P521 = "SIGNATURE_TYPE=ECDSA_SHA512_P521" - Sig_EdDSA_SHA512_Ed25519 = "SIGNATURE_TYPE=EdDSA_SHA512_Ed25519" -) - -var letters = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") - func RandString() string { + letters := []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") n := 4 b := make([]rune, n) for i := range b { diff --git a/suggestedOptions.go b/suggestedOptions.go index 21ae6a9..792dddd 100644 --- a/suggestedOptions.go +++ b/suggestedOptions.go @@ -76,7 +76,6 @@ var ( ) func getEnv(key, fallback string) string { - InitializeSAM3Logger() value, ok := os.LookupEnv(key) if !ok { log.WithFields(logrus.Fields{