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

server.DeleteMessage catches channel or client==nil #2024

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

FiskFan1999
Copy link
Contributor

Fixes #2020

Calling /msg histserv delete #wrongchan returns spurrious success message. Calling /msg histserv delete returns similar message.

These would lead to the pointer hist == nil, and
server.historyDB.DeleteMsgid() would return nil.

Returns errNoSuchChannel or errInvalidTarget if channel or client == nil

irc/server.go Outdated
@@ -1061,13 +1061,17 @@ func (server *Server) DeleteMessage(target, msgid, accountName string) (err erro
if status, _, _ := channel.historyStatus(config); status == HistoryEphemeral {
hist = &channel.history
}
} else {
return errNoSuchChannel
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm interested in your feedback on what errors should be thrown here. and below.

@slingamn
Copy link
Member

Thanks, these error values seem fine. What is the error message that ultimately gets sent to the operator?

@FiskFan1999
Copy link
Contributor Author

Thanks, these error values seem fine. What is the error message that ultimately gets sent to the operator?

The error that is returned by server.DeleteMessage is sent to the operator (that is, err.Error()).

However, while I was away I realized this PR will need to be refactored, because it breaks a situation in which persistent storage is storing a channel or user which is not currently logged in (which is the intention of the original code). Will get on this.

If hist == nil and mysql database Delete msgid function returns
ErrDBIsNil, we know that the target does not match any channel or user.
Return invalid target error to operator (see ergochat#2020)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

histserv delete returns spurrious success message if incorrect parameters
2 participants