Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: return errors upon panics or receiving unexpected responses #433

Merged
merged 3 commits into from
May 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion add.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ldap

import (
"fmt"
ber "github.com/go-asn1-ber/asn1-ber"
)

Expand Down Expand Up @@ -82,7 +83,7 @@ func (l *Conn) Add(addRequest *AddRequest) error {
return err
}
} else {
logger.Printf("Unexpected Response: %d", packet.Children[1].Tag)
return fmt.Errorf("ldap: unexpected response: %d", packet.Children[1].Tag)
}
return nil
}
3 changes: 2 additions & 1 deletion client.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import (
type Client interface {
Start()
StartTLS(*tls.Config) error
Close()
Close() error
GetLastError() error
IsClosing() bool
SetTimeout(time.Duration)
TLSConnectionState() (tls.ConnectionState, bool)
Expand Down
25 changes: 16 additions & 9 deletions conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ type Conn struct {
wgClose sync.WaitGroup
outstandingRequests uint
messageMutex sync.Mutex

err error
}

var _ Client = &Conn{}
Expand Down Expand Up @@ -277,7 +279,7 @@ func (l *Conn) setClosing() bool {
}

// Close closes the connection.
func (l *Conn) Close() {
func (l *Conn) Close() (err error) {
l.messageMutex.Lock()
defer l.messageMutex.Unlock()

Expand All @@ -301,13 +303,12 @@ func (l *Conn) Close() {
close(l.chanMessage)

l.Debug.Printf("Closing network connection")
if err := l.conn.Close(); err != nil {
logger.Println(err)
}

err = l.conn.Close()
l.wgClose.Done()
}
l.wgClose.Wait()

return err
}

// SetTimeout sets the time after a request is sent that a MessageTimeout triggers
Expand All @@ -323,6 +324,12 @@ func (l *Conn) nextMessageID() int64 {
return 0
}

// GetLastError returns the last recorded error from goroutines like processMessages and reader.
// Only the last recorded error will be returned.
func (l *Conn) GetLastError() error {
return l.err
}

// StartTLS sends the command to start a TLS session and then creates a new TLS Client
func (l *Conn) StartTLS(config *tls.Config) error {
if l.isTLS {
Expand Down Expand Up @@ -471,7 +478,7 @@ func (l *Conn) sendProcessMessage(message *messagePacket) bool {
func (l *Conn) processMessages() {
defer func() {
if err := recover(); err != nil {
logger.Printf("ldap: recovered panic in processMessages: %v", err)
l.err = fmt.Errorf("ldap: recovered panic in processMessages: %v", err)
}
for messageID, msgCtx := range l.messageContexts {
// If we are closing due to an error, inform anyone who
Expand Down Expand Up @@ -520,7 +527,7 @@ func (l *Conn) processMessages() {
timer := time.NewTimer(time.Duration(l.requestTimeout))
defer func() {
if err := recover(); err != nil {
logger.Printf("ldap: recovered panic in RequestTimeout: %v", err)
l.err = fmt.Errorf("ldap: recovered panic in RequestTimeout: %v", err)
}

timer.Stop()
Expand All @@ -542,7 +549,7 @@ func (l *Conn) processMessages() {
if msgCtx, ok := l.messageContexts[message.MessageID]; ok {
msgCtx.sendResponse(&PacketResponse{message.Packet, nil}, time.Duration(l.requestTimeout))
} else {
logger.Printf("Received unexpected message %d, %v", message.MessageID, l.IsClosing())
l.err = fmt.Errorf("ldap: received unexpected message %d, %v", message.MessageID, l.IsClosing())
l.Debug.PrintPacket(message.Packet)
}
case MessageTimeout:
Expand All @@ -569,7 +576,7 @@ func (l *Conn) reader() {
cleanstop := false
defer func() {
if err := recover(); err != nil {
logger.Printf("ldap: recovered panic in reader: %v", err)
l.err = fmt.Errorf("ldap: recovered panic in reader: %v", err)
}
if !cleanstop {
l.Close()
Expand Down
4 changes: 3 additions & 1 deletion del.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ldap

import (
"fmt"
ber "github.com/go-asn1-ber/asn1-ber"
)

Expand Down Expand Up @@ -51,7 +52,8 @@ func (l *Conn) Del(delRequest *DelRequest) error {
return err
}
} else {
logger.Printf("Unexpected Response: %d", packet.Children[1].Tag)
return fmt.Errorf("ldap: unexpected response: %d", packet.Children[1].Tag)
}

return nil
}
4 changes: 3 additions & 1 deletion moddn.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ldap

import (
"fmt"
ber "github.com/go-asn1-ber/asn1-ber"
)

Expand Down Expand Up @@ -94,7 +95,8 @@ func (l *Conn) ModifyDN(m *ModifyDNRequest) error {
return err
}
} else {
logger.Printf("Unexpected Response: %d", packet.Children[1].Tag)
return fmt.Errorf("ldap: unexpected response: %d", packet.Children[1].Tag)
}

return nil
}
4 changes: 3 additions & 1 deletion modify.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package ldap

import (
"errors"
"fmt"

ber "github.com/go-asn1-ber/asn1-ber"
)
Expand Down Expand Up @@ -126,8 +127,9 @@ func (l *Conn) Modify(modifyRequest *ModifyRequest) error {
return err
}
} else {
logger.Printf("Unexpected Response: %d", packet.Children[1].Tag)
return fmt.Errorf("ldap: unexpected response: %d", packet.Children[1].Tag)
}

return nil
}

Expand Down
3 changes: 2 additions & 1 deletion v3/add.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ldap

import (
"fmt"
ber "github.com/go-asn1-ber/asn1-ber"
)

Expand Down Expand Up @@ -82,7 +83,7 @@ func (l *Conn) Add(addRequest *AddRequest) error {
return err
}
} else {
logger.Printf("Unexpected Response: %d", packet.Children[1].Tag)
return fmt.Errorf("ldap: unexpected response: %d", packet.Children[1].Tag)
}
return nil
}
3 changes: 2 additions & 1 deletion v3/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import (
type Client interface {
Start()
StartTLS(*tls.Config) error
Close()
Close() error
GetLastError() error
IsClosing() bool
SetTimeout(time.Duration)
TLSConnectionState() (tls.ConnectionState, bool)
Expand Down
25 changes: 16 additions & 9 deletions v3/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ type Conn struct {
wgClose sync.WaitGroup
outstandingRequests uint
messageMutex sync.Mutex

err error
}

var _ Client = &Conn{}
Expand Down Expand Up @@ -277,7 +279,7 @@ func (l *Conn) setClosing() bool {
}

// Close closes the connection.
func (l *Conn) Close() {
func (l *Conn) Close() (err error) {
l.messageMutex.Lock()
defer l.messageMutex.Unlock()

Expand All @@ -301,13 +303,12 @@ func (l *Conn) Close() {
close(l.chanMessage)

l.Debug.Printf("Closing network connection")
if err := l.conn.Close(); err != nil {
logger.Println(err)
}

err = l.conn.Close()
l.wgClose.Done()
}
l.wgClose.Wait()

return err
}

// SetTimeout sets the time after a request is sent that a MessageTimeout triggers
Expand All @@ -323,6 +324,12 @@ func (l *Conn) nextMessageID() int64 {
return 0
}

// GetLastError returns the last recorded error from goroutines like processMessages and reader.
// // Only the last recorded error will be returned.
func (l *Conn) GetLastError() error {
return l.err
}

// StartTLS sends the command to start a TLS session and then creates a new TLS Client
func (l *Conn) StartTLS(config *tls.Config) error {
if l.isTLS {
Expand Down Expand Up @@ -471,7 +478,7 @@ func (l *Conn) sendProcessMessage(message *messagePacket) bool {
func (l *Conn) processMessages() {
defer func() {
if err := recover(); err != nil {
logger.Printf("ldap: recovered panic in processMessages: %v", err)
l.err = fmt.Errorf("ldap: recovered panic in processMessages: %v", err)
}
for messageID, msgCtx := range l.messageContexts {
// If we are closing due to an error, inform anyone who
Expand Down Expand Up @@ -520,7 +527,7 @@ func (l *Conn) processMessages() {
timer := time.NewTimer(time.Duration(l.requestTimeout))
defer func() {
if err := recover(); err != nil {
logger.Printf("ldap: recovered panic in RequestTimeout: %v", err)
l.err = fmt.Errorf("ldap: recovered panic in RequestTimeout: %v", err)
}

timer.Stop()
Expand All @@ -542,7 +549,7 @@ func (l *Conn) processMessages() {
if msgCtx, ok := l.messageContexts[message.MessageID]; ok {
msgCtx.sendResponse(&PacketResponse{message.Packet, nil}, time.Duration(l.requestTimeout))
} else {
logger.Printf("Received unexpected message %d, %v", message.MessageID, l.IsClosing())
l.err = fmt.Errorf("ldap: received unexpected message %d, %v", message.MessageID, l.IsClosing())
l.Debug.PrintPacket(message.Packet)
}
case MessageTimeout:
Expand All @@ -569,7 +576,7 @@ func (l *Conn) reader() {
cleanstop := false
defer func() {
if err := recover(); err != nil {
logger.Printf("ldap: recovered panic in reader: %v", err)
l.err = fmt.Errorf("ldap: recovered panic in reader: %v", err)
}
if !cleanstop {
l.Close()
Expand Down
4 changes: 3 additions & 1 deletion v3/del.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ldap

import (
"fmt"
ber "github.com/go-asn1-ber/asn1-ber"
)

Expand Down Expand Up @@ -51,7 +52,8 @@ func (l *Conn) Del(delRequest *DelRequest) error {
return err
}
} else {
logger.Printf("Unexpected Response: %d", packet.Children[1].Tag)
return fmt.Errorf("ldap: unexpected response: %d", packet.Children[1].Tag)
}

return nil
}
4 changes: 3 additions & 1 deletion v3/moddn.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ldap

import (
"fmt"
ber "github.com/go-asn1-ber/asn1-ber"
)

Expand Down Expand Up @@ -94,7 +95,8 @@ func (l *Conn) ModifyDN(m *ModifyDNRequest) error {
return err
}
} else {
logger.Printf("Unexpected Response: %d", packet.Children[1].Tag)
return fmt.Errorf("ldap: unexpected response: %d", packet.Children[1].Tag)
}

return nil
}
4 changes: 3 additions & 1 deletion v3/modify.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package ldap

import (
"errors"
"fmt"

ber "github.com/go-asn1-ber/asn1-ber"
)
Expand Down Expand Up @@ -126,8 +127,9 @@ func (l *Conn) Modify(modifyRequest *ModifyRequest) error {
return err
}
} else {
logger.Printf("Unexpected Response: %d", packet.Children[1].Tag)
return fmt.Errorf("ldap: unexpected response: %d", packet.Children[1].Tag)
}

return nil
}

Expand Down