diff --git a/irc/handlers.go b/irc/handlers.go index 10cb18d5..029fc0c4 100644 --- a/irc/handlers.go +++ b/irc/handlers.go @@ -2842,10 +2842,16 @@ func redactHandler(server *Server, client *Client, msg ircmsg.Message, rb *Respo } err := server.DeleteMessage(target, targetmsgid, accountName) - if err == errNoop { + switch err { + case history.ErrNotFound: rb.Add(nil, server.name, "FAIL", "REDACT", "UNKNOWN_MSGID", utils.SafeErrorParam(target), utils.SafeErrorParam(targetmsgid), client.t("This message does not exist or is too old")) return false - } else if err != nil { + case history.ErrDisallowed: + rb.Add(nil, server.name, "FAIL", "REDACT", "REDACT_FORBIDDEN", utils.SafeErrorParam(target), utils.SafeErrorParam(targetmsgid), client.t("You are not authorized to delete this message")) + return false + case nil: + // OK + default: isOper := client.HasRoleCapabs("history") if isOper { rb.Add(nil, server.name, "FAIL", "REDACT", "REDACT_FORBIDDEN", utils.SafeErrorParam(target), utils.SafeErrorParam(targetmsgid), fmt.Sprintf(client.t("Error deleting message: %v"), err)) @@ -2860,7 +2866,8 @@ func redactHandler(server *Server, client *Client, msg ircmsg.Message, rb *Respo // now we have to remove it from the buffer of the client who sent the REDACT command err := server.DeleteMessage(client.Nick(), targetmsgid, accountName) - if err != nil { + // ErrNotFound is expected if both clients are using persistent history + if err != nil && err != history.ErrNotFound { client.server.logger.Error("internal", fmt.Sprintf("Private message %s is not deletable by %s from their own buffer's even though we just deleted it from %s's. This is a bug, please report it in details.", targetmsgid, client.Nick(), target), client.Nick()) isOper := client.HasRoleCapabs("history") if isOper { diff --git a/irc/history/database.go b/irc/history/database.go index 3ec44ea4..277d2c39 100644 --- a/irc/history/database.go +++ b/irc/history/database.go @@ -4,10 +4,16 @@ package history import ( + "errors" "io" "time" ) +var ( + ErrDisallowed = errors.New("disallowed") + ErrNotFound = errors.New("not found") +) + // Database is an interface for persistent history storage backends. type Database interface { // Close closes the database connection and releases resources. diff --git a/irc/mysql/history.go b/irc/mysql/history.go index 16749347..d123ff29 100644 --- a/irc/mysql/history.go +++ b/irc/mysql/history.go @@ -7,7 +7,6 @@ import ( "context" "database/sql" "encoding/json" - "errors" "fmt" "io" "runtime/debug" @@ -23,10 +22,6 @@ import ( _ "github.com/go-sql-driver/mysql" ) -var ( - ErrDisallowed = errors.New("disallowed") -) - const ( // maximum length in bytes of any message target (nickname or channel name) in its // canonicalized (i.e., casefolded) state: @@ -749,6 +744,9 @@ func (mysql *MySQL) DeleteMsgid(msgid, accountName string) (err error) { _, id, data, err := mysql.lookupMsgid(ctx, msgid, true) if err != nil { + if err == sql.ErrNoRows { + return history.ErrNotFound + } return } @@ -757,7 +755,7 @@ func (mysql *MySQL) DeleteMsgid(msgid, accountName string) (err error) { err = history.UnmarshalItem(data, &item) // delete if the entry is corrupt if err == nil && item.AccountName != accountName { - return ErrDisallowed + return history.ErrDisallowed } } @@ -830,6 +828,9 @@ func (mysql *MySQL) Export(account string, writer io.Writer) { func (mysql *MySQL) lookupMsgid(ctx context.Context, msgid string, includeData bool) (result time.Time, id uint64, data []byte, err error) { decoded, err := utils.DecodeSecretToken(msgid) if err != nil { + // use sql.ErrNoRows internally for consistency, translate to history.ErrNotFound + // at the package boundary if necessary + err = sql.ErrNoRows return } cols := `sequence.nanotime, conversations.nanotime` diff --git a/irc/server.go b/irc/server.go index 94a1021b..49697397 100644 --- a/irc/server.go +++ b/irc/server.go @@ -1242,7 +1242,7 @@ func (server *Server) DeleteMessage(target, msgid, accountName string) (err erro return item.Message.Msgid == msgid && (accountName == "*" || item.AccountName == accountName) }) if count == 0 { - err = errNoop + err = history.ErrNotFound } }