transaction: Never return Transaction as error
While transaction does implement error, it's not a valid error implementer because it may have bogous values since it's not thread-safe and so we may read the result of Error() when it's into an invalid state As per this never return it as an error, while always return the Status unless when not available, where we still return pam.Error.
This commit is contained in:
@@ -22,7 +22,6 @@ package pam
|
|||||||
import "C"
|
import "C"
|
||||||
|
|
||||||
import (
|
import (
|
||||||
"errors"
|
|
||||||
"fmt"
|
"fmt"
|
||||||
"runtime"
|
"runtime"
|
||||||
"runtime/cgo"
|
"runtime/cgo"
|
||||||
@@ -146,8 +145,8 @@ func transactionFinalizer(t *Transaction) {
|
|||||||
// Allows to call pam functions managing return status
|
// Allows to call pam functions managing return status
|
||||||
func (t *Transaction) handlePamStatus(cStatus C.int) error {
|
func (t *Transaction) handlePamStatus(cStatus C.int) error {
|
||||||
t.lastStatus.Store(int32(cStatus))
|
t.lastStatus.Store(int32(cStatus))
|
||||||
if cStatus != success {
|
if status := Error(cStatus); status != success {
|
||||||
return t
|
return status
|
||||||
}
|
}
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
@@ -213,7 +212,7 @@ func start(service, user string, handler ConversationHandler, confDir string) (*
|
|||||||
err = t.handlePamStatus(C.pam_start_confdir(s, u, t.conv, c, &t.handle))
|
err = t.handlePamStatus(C.pam_start_confdir(s, u, t.conv, c, &t.handle))
|
||||||
}
|
}
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, errors.Join(Error(t.lastStatus.Load()), err)
|
return nil, err
|
||||||
}
|
}
|
||||||
return t, nil
|
return t, nil
|
||||||
}
|
}
|
||||||
@@ -365,7 +364,7 @@ func (t *Transaction) GetEnvList() (map[string]string, error) {
|
|||||||
p := C.pam_getenvlist(t.handle)
|
p := C.pam_getenvlist(t.handle)
|
||||||
if p == nil {
|
if p == nil {
|
||||||
t.lastStatus.Store(int32(ErrBuf))
|
t.lastStatus.Store(int32(ErrBuf))
|
||||||
return nil, t
|
return nil, ErrBuf
|
||||||
}
|
}
|
||||||
t.lastStatus.Store(success)
|
t.lastStatus.Store(success)
|
||||||
for q := p; *q != nil; q = next(q) {
|
for q := p; *q != nil; q = next(q) {
|
||||||
|
|||||||
@@ -167,8 +167,8 @@ func TestPAM_007(t *testing.T) {
|
|||||||
if len(s) == 0 {
|
if len(s) == 0 {
|
||||||
t.Fatalf("error #expected an error message")
|
t.Fatalf("error #expected an error message")
|
||||||
}
|
}
|
||||||
if tx.Error() != ErrAuth.Error() {
|
if !errors.Is(err, ErrAuth) {
|
||||||
t.Fatalf("error #unexpected status %v", tx.Error())
|
t.Fatalf("error #unexpected error %v", err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -255,8 +255,8 @@ func TestPAM_ConfDir_Deny(t *testing.T) {
|
|||||||
if len(s) == 0 {
|
if len(s) == 0 {
|
||||||
t.Fatalf("error #expected an error message")
|
t.Fatalf("error #expected an error message")
|
||||||
}
|
}
|
||||||
if tx.Error() != ErrAuth.Error() {
|
if !errors.Is(err, ErrAuth) {
|
||||||
t.Fatalf("error #unexpected status %v", tx.Error())
|
t.Fatalf("error #unexpected error %v", err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -304,8 +304,8 @@ func TestPAM_ConfDir_WrongUserName(t *testing.T) {
|
|||||||
if len(s) == 0 {
|
if len(s) == 0 {
|
||||||
t.Fatalf("error #expected an error message")
|
t.Fatalf("error #expected an error message")
|
||||||
}
|
}
|
||||||
if tx.Error() != ErrAuth.Error() {
|
if !errors.Is(err, ErrAuth) {
|
||||||
t.Fatalf("error #unexpected status %v", tx.Error())
|
t.Fatalf("error #unexpected error %v", err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -416,7 +416,7 @@ func Test_Error(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
statuses := map[string]error{
|
statuses := map[string]error{
|
||||||
"success": Error(success),
|
"success": nil,
|
||||||
"open_err": ErrOpen,
|
"open_err": ErrOpen,
|
||||||
"symbol_err": ErrSymbol,
|
"symbol_err": ErrSymbol,
|
||||||
"service_err": ErrService,
|
"service_err": ErrService,
|
||||||
@@ -441,7 +441,7 @@ func Test_Error(t *testing.T) {
|
|||||||
"authtok_lock_busy": ErrAuthtokLockBusy,
|
"authtok_lock_busy": ErrAuthtokLockBusy,
|
||||||
"authtok_disable_aging": ErrAuthtokDisableAging,
|
"authtok_disable_aging": ErrAuthtokDisableAging,
|
||||||
"try_again": ErrTryAgain,
|
"try_again": ErrTryAgain,
|
||||||
"ignore": Error(success), /* Ignore can't be returned */
|
"ignore": nil, /* Ignore can't be returned */
|
||||||
"abort": ErrAbort,
|
"abort": ErrAbort,
|
||||||
"authtok_expired": ErrAuthtokExpired,
|
"authtok_expired": ErrAuthtokExpired,
|
||||||
"module_unknown": ErrModuleUnknown,
|
"module_unknown": ErrModuleUnknown,
|
||||||
@@ -504,13 +504,17 @@ func Test_Error(t *testing.T) {
|
|||||||
err = tx.OpenSession(0)
|
err = tx.OpenSession(0)
|
||||||
}
|
}
|
||||||
|
|
||||||
if tx.Error() != expected.Error() {
|
if !errors.Is(err, expected) {
|
||||||
t.Fatalf("error #unexpected status %v", tx.Error())
|
t.Fatalf("error #unexpected status %#v vs %#v", err,
|
||||||
|
expected)
|
||||||
}
|
}
|
||||||
if tx.Error() == Error(success).Error() && err != nil {
|
|
||||||
t.Fatalf("error #unexpected: %v", err)
|
if err != nil {
|
||||||
} else if tx.Error() != Error(success).Error() && err == nil {
|
var status Error
|
||||||
t.Fatalf("error #expected an error message")
|
if !errors.As(err, &status) || err.Error() != status.Error() {
|
||||||
|
t.Fatalf("error #unexpected status %v vs %v", err.Error(),
|
||||||
|
status.Error())
|
||||||
|
}
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user