Ticket #1462 (closed defect: fixed)

Opened 10 years ago

Last modified 10 years ago

isSpam() in class.dc.filter.iplookup.php shouldn't ping all DNSBL providers if it has a match

Reported by: pascalc Owned by: team
Priority: normal Milestone: 2.5.3
Component: module:plugins Version: 2.5
Severity: normal Keywords:


Looking at this function:

It seems that we do an IP look up to all of the DNSBL providers and if there is one match of the IP processed after that, we return True.

That seems suboptimal to me, we should just return false at the first match we get.

Change History

comment:1 Changed 10 years ago by pascalc

Also, it seems strange to me that we return true/null

For a method called isSomething, I would expect that we always return a boolean value (true/false), is there a reason to return null when it is not a spam?

comment:2 Changed 10 years ago by JcDenis

Je m'étais posé la même question. D'un coté si c'est un spam peu importe si le script prend du temps vu que c'est un spammer, D'un autre coté cela n'apporte rien puisque l'addition des rapport de spam est tronqué. Je suis donc plutôt pour arrêter la boucle en effet.

comment:3 Changed 10 years ago by pascalc

The method can actually return 'null' (not a valid IP), 'true' (comment is spam) or just nothing if this is not spam. I am going to propose a patch that makes it return 'false' if it not spam and that will return true at the first match against DNSBL servers.

comment:4 Changed 10 years ago by pascalc

ok, I found out in another file why it doesn't return false:

This method should return if a comment is a spam or not. If it returns true or false, execution of next filters will be stoped. If should return nothing to let next filters apply.

I am going to update my PR to not return false.

comment:5 Changed 10 years ago by JcDenis

TRUE = spam , FALSE = not spam , NULL = I don't know
Il ne faut pas changer ses valeurs, elle sont commune à tous les filtres antispam ! Le retour NULL indique que le filtre n'est pas compétent pour trancher et laisse au filtre suivant le choix.

comment:6 Changed 10 years ago by franck

  • Milestone changed from A definir to 2.5.3

comment:7 Changed 10 years ago by franck

  • Status changed from new to closed
  • Resolution set to fixed
Note: See TracTickets for help on using tickets.

Sites map