1082501 - LIST fails on some servers during the first minute after connection

Status: RESOLVED FIXED

(Chat Core :: IRC, defect)

Description

[aleth [:aleth]]

e.g. irc.mozilla.org

[14/10/2014 11:58:21] ":fripp.mozilla.org NOTICE aleth-build :*** You cannot list within the first 60 seconds of connecting. Please try again later."
[14/10/2014 11:58:21] ":fripp.mozilla.org 321 aleth-build Channel :Users Name"
[14/10/2014 11:58:21] ":fripp.mozilla.org 323 aleth-build :End of channel list."

So we get a correct 321,323 pair, but that means we won't reattempt LIST for another hour or day or whatever the constant is.

Comment 1

[aleth [:aleth]]

This is indicated by ISUPPORT SECURELIST ("This InspIRCd specific token indicates that secure listing is enabled, and that your initial request for a channel list on connect may be denied until you have been connected for a certain amount of time.")

https://github.com/inspircd/inspircd/blob/master/src/modules/m_securelist.cpp

Comment 2

[aleth [:aleth]]

Attachment: securelist.diff

Comment 3

[Patrick Cloke [:clokep]]

Comment on attachment 8513432 [details] [diff] [review]
securelist.diff

Review of attachment 8513432 [details] [diff] [review]:
-----------------------------------------------------------------

How hard would this be to add a test for?

Let me know about the parseInt comment...

::: chat/protocols/irc/ircNonStandard.jsm
@@ +56,5 @@
> +        // pair, but no list data.
> +        // We fake the last LIST time so that we will retry LIST the next time
> +        // the user requires it after the interval specified.
> +        const kMinute = 60000;
> +        let waittime = (aMessage.params[1].split(" ")[7] * 1000) || kMinute;

Doesn't this need a parseInt or something around it? Nit: waitTime

@@ +57,5 @@
> +        // We fake the last LIST time so that we will retry LIST the next time
> +        // the user requires it after the interval specified.
> +        const kMinute = 60000;
> +        let waittime = (aMessage.params[1].split(" ")[7] * 1000) || kMinute;
> +        this._lastListTime = Date.now() - kListRefreshInterval + waittime;

This is me just being pedantic, but can you do now + waitTime - refreshInterval? It makes more sense in my head. :)

Comment 4

[aleth [:aleth]]

Attachment: securelist.diff v2

(In reply to Patrick Cloke [:clokep] from comment #3)
> How hard would this be to add a test for?

The test we would want here is to detect whether inspircd have decided to change the string they send us, and we can't test for that.

> > +        let waittime = (aMessage.params[1].split(" ")[7] * 1000) || kMinute;
> 
> Doesn't this need a parseInt or something around it? Nit: waitTime

It's not needed as the multiplication enforces type conversion.

Comment 5

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

https://hg.mozilla.org/comm-central/rev/b9bda3682a08