github-vet / github-vet/rangeloop-pointer-findings

toorop/tmail: core/smtp_client.go; 153 LoC

Open
#6,623 0 comments 0 reactions 0 assignees View on GitHub
fresh large
Dominant language
No language data
Stars
0
Forks
0
PR merge metrics
PR metrics pending

Description

Found a possible issue in [toorop/tmail](https://www.github.com/toorop/tmail) at [core/smtp_client.go](https://github.com/toorop/tmail/blob/38218317af63a2f467fbac6cd10931f97be69b40/core/smtp_client.go#L40-L192)

Below is the message reported by the analyzer for this snippet of code. Beware that the analyzer only reports the first
issue it finds, so please do not limit your consideration to the contents of the below message.

> range-loop variable route used in defer or goroutine at line 148

[Click here to see the code in its original context.](https://github.com/toorop/tmail/blob/38218317af63a2f467fbac6cd10931f97be69b40/core/smtp_client.go#L40-L192)

Click here to show the 153 line(s) of Go which triggered the analyzer.

```go
for _, route := range routes {
localIPs := []net.IP{}
remoteAddresses := []net.TCPAddr{}

// If there is no local IP get default (as defined in config)
if route.LocalIp.String == "" {
route.LocalIp = sql.NullString{String: Cfg.GetLocalIps(), Valid: true}
}

// there should be no mix beetween failover and round robin for local IP
failover := strings.Count(route.LocalIp.String, "&") != 0
roundRobin := strings.Count(route.LocalIp.String, "|") != 0
if failover && roundRobin {
return nil, fmt.Errorf("failover and round-robin are mixed in route %d for local IP", route.Id)
}

// Contient les IP sous forme de string
var sIps []string

// On a une seule IP locale
if !failover && !roundRobin {
sIps = []string{route.LocalIp.String}
} else { // multiple locals ips
var sep string
if failover {
sep = "&"
} else {
sep = "|"
}
sIps = strings.Split(route.LocalIp.String, sep)

// if roundRobin we need to shuffle IPs
rSIps := make([]string, len(sIps))
perm := rand.Perm(len(sIps))
for i, v := range perm {
rSIps[v] = sIps[i]
}
sIps = rSIps
rSIps = nil
}

// IP string to net.IP
for _, ipStr := range sIps {
ip := net.ParseIP(ipStr)
if ip == nil {
return nil, errors.New("invalid IP " + ipStr + " found in localIp routes: " + route.LocalIp.String)
}
localIPs = append(localIPs, ip)
}

// remoteAdresses
// Hostname or IP
// IP ?
ip := net.ParseIP(route.RemoteHost)
if ip != nil { // ip
remoteAddresses = append(remoteAddresses, net.TCPAddr{
IP: ip,
Port: int(route.RemotePort.Int64),
})
// hostname
} else {
ips, err := net.LookupIP(route.RemoteHost)
// TODO: no such host -> perm failure
if err != nil {
return nil, err
}
for _, i := range ips {
remoteAddresses = append(remoteAddresses, net.TCPAddr{
IP: i,
Port: int(route.RemotePort.Int64),
})
}
}

// try routes & returns first OK
for _, localIP := range localIPs {
for _, remoteAddr := range remoteAddresses {
// IPv4 <-> IPv4 or IPv6 <-> IPv6
if IsIPV4(localIP.String()) != IsIPV4(remoteAddr.IP.String()) {
continue
}

// If during the last 15 minutes we have fail to connect to this host don't try again
if !isRemoteIPOK(remoteAddr.IP.String()) {
Logger.Info("smtp getclient " + remoteAddr.IP.String() + " is marked as KO. I'll dot not try to reach it.")
continue
}

localAddr, err := net.ResolveTCPAddr("tcp", localIP.String()+":0")
if err != nil {
return nil, errors.New("bad local IP: " + localIP.String() + ". " + err.Error())
}

// Dial timeout
connectTimer := time.NewTimer(time.Duration(timeoutBasePerCmd) * time.Second)
done := make(chan error, 1)
var conn net.Conn
var client *smtpClient
go func() {
conn, err = net.DialTCP("tcp", localAddr, &remoteAddr)
if err != nil {
done <- err
return
}
client = &smtpClient{
conn: conn,
timeoutBasePerCmd: timeoutBasePerCmd,
}
client.route = &route
client.text = textproto.NewConn(conn)
_, _, err = client.text.ReadResponse(220)
done <- err
}()

select {
case err = <-done:
if err == nil {
return client, nil
}

//client.text = textproto.NewConn(conn)
// timeout on response
/*connectTimer.Reset(time.Duration(30) * time.Second)
go func() {

client.text = textproto.NewConn(conn)
_, _, err = client.text.ReadResponse(220)
done <- err
}()
select {
case err = <-done:*/

// Timeout
/*case <-connectTimer.C:
conn.Close()
err = errors.New("timeout")
// todo si c'est un timeout pas la peine d'essayer les autres IP locales
if errBolt := setIPKO(remoteAddr.IP.String()); errBolt != nil {
Logger.Error("Bolt - ", errBolt)
}*/

// Timeout
case <-connectTimer.C:
err = errors.New("timeout")
// todo si c'est un timeout pas la peine d'essayer les autres IP locales
if errBolt := setIPKO(remoteAddr.IP.String()); errBolt != nil {
Logger.Error("Bolt - ", errBolt)
}
}
Logger.Info(fmt.Sprintf("deliverd-remote %s - unable to get a SMTP client for %s->%s:%d - %s ", d.ID, localIP, remoteAddr.IP.String(), remoteAddr.Port, err.Error()))
}
}
}

```

Leave a reaction on this issue to contribute to the project by classifying this instance as a **Bug** :-1:, **Mitigated** :+1:, or **Desirable Behavior** :rocket:
See the descriptions of the classifications [here](https://github.com/github-vet/rangeclosure-findings#how-can-i-help) for more information.

commit ID: 38218317af63a2f467fbac6cd10931f97be69b40

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.