Closed Bug 123692 Opened 23 years ago Closed 23 years ago

[HAVE FIX] implement Infobot module

Categories

(Webtools Graveyard :: Mozbot, enhancement)

x86
Linux
enhancement
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: ian, Assigned: ian)

References

Details

Attachments

(1 file, 8 obsolete files)

If mozbot is to be competitive in the bot world, it needs to be able to do
everything that infobot does. With that in mind... we need an Infobot module.
This implements infobot-like behaviour as a mozbot module.

Highlights:
   * Supports the :INFOBOT:QUERY protocol (see Infobot.txt)
   * Supports infobot factpacks (see Infobot.pl)
   * Supports <reply>, <action>, <alias>, $who, |
   * Supports both "is" and "are" databases
   * Supports "I am"/"You are" style things like infobot
   * Supports the "tell" command
   * And of course, supports "no" and "also"

Reviews please! :-)
(ignore the top two lines of the patch, they are a cut-and-paste error)
Keywords: review
Summary: implement Infobot module → [HAVE FIX] implement Infobot module
Severity: normal → enhancement
Status: NEW → ASSIGNED
missing: good bot, bot snack, literal.

it appears that your implementation will result in different and perhaps more 
exploitable behavior than standard infobot. please make /msg *not* parse
<reply> or <action>

a parsing read of the code implies the following
/nick self
/join #infobotclone
/mode #infobotclone +o self
/kick #infobotclone victim
/msg #infobotclone infobotbot tell victim about kicks
:privmsg infobotbot#infobotclone victim kicks are ...

this is not what should happen :)

exploit #1
/msg botbot no, foo is <reply> tell botbot about foo
/msg botbot tell botbot about foo

based on current reading of the code, infobotbot should now waste cycles 
telling itself (forever) about foo.

a more correct dialog is:
[timeless]  word tell word about me
<word> Isn't that a bit silly, timeless?

status implemenation is sub par.  while uptime isn't important, knowing the 
number of questions and modifications since the last module load is quite 
useful.

I can't figure out how to make this infobot module optionally require direct 
address.
implemenation=~s/n/nt/
> missing: good bot, bot snack, literal.

good bot and bot snack are in the Greeting.bm module.

What's 'literal'? It might be what I have called 'remember', in which case I
would happily replace it with 'literal'.


> it appears that your implementation will result in different and perhaps more 
> exploitable behavior than standard infobot. please make /msg *not* parse
> <reply> or <action>

Why not? That it doesn't is one of the most annoying things about infobot, in my
opinion. Can you think of any cases where it would exploitable?


> a parsing read of the code implies the following
> /nick self
> /join #infobotclone
> /mode #infobotclone +o self
> /kick #infobotclone victim
> /msg #infobotclone infobotbot tell victim about kicks
> :privmsg infobotbot#infobotclone victim kicks are ...
> 
> this is not what should happen :)

It's not what does happen. If self is Hixie, victim gets:

   <oopsbot> Hixie wanted you to know: kicks are ...


> exploit #1
> /msg botbot no, foo is <reply> tell botbot about foo
> /msg botbot tell botbot about foo
>
> based on current reading of the code, infobotbot should now waste cycles 
> telling itself (forever) about foo.

This is what happens in the console:

2002-02-06 09:27:48 UTC (9920) Told: #buncs <Hixie> no, bla is <reply> tell
oopsbot about bla
2002-02-06 09:27:48 UTC (9920) Module Infobot: Learning that bla is '<reply>
tell oopsbot about bla'.
2002-02-06 09:27:48 UTC (9920) ->#buncs: Hixie: ok
2002-02-06 09:27:56 UTC (9920) Told: #buncs <Hixie> tell oopsbot about bla
2002-02-06 09:27:56 UTC (9920) Module Infobot: Telling Hixie about bla.
2002-02-06 09:27:56 UTC (9920) ->#buncs: Hixie: told oopsbot
2002-02-06 09:27:57 UTC (9920) ->oopsbot: Hixie wanted you to know: tell oopsbot
about bla
2002-02-06 09:27:57 UTC (9920) Told:  <oopsbot> Hixie wanted you to know: tell
oopsbot about bla
2002-02-06 09:27:57 UTC (9920) Baffled:  <oopsbot> Hixie wanted you to know:
tell oopsbot about bla


> a more correct dialog is:
> [timeless]  word tell word about me
> <word> Isn't that a bit silly, timeless?

Ok, I'll add protection against telling $event->{'nick'} stuff in factoidSay().


> status implemenation is sub par.  while uptime isn't important, knowing the 
> number of questions and modifications since the last module load is quite 
> useful.

uptime is part of the Greeting.bm module.

I've added more stats to the status message.


> I can't figure out how to make this infobot module optionally require direct 
> address.

How do you mean? There are per-channel controls of the addressing mode for
learning, editing and forgetting, and helping. Those are the autoXXX and
neverXXX variables.
Attached patch Patch version 1.1 (obsolete) — Splinter Review
Attachment #68027 - Attachment is obsolete: true
I just caught one other mistake; in the patch, the one occurance of "is($"
should be changed to "lc($". I have changed that in my local tree.
Dare I ask how this will effect bug 115368?  It seems to me that this is the one
thing that can exploit bug 115368 more than anything else.  The current infobot
syntax says that I can ask a question like (note, all examples hand typed):

<Jake> word: Jake?
<word> Jake: You are .....

But if I try that in an oopsbot running both this and Karma, I'll get:

<Jake> oopsbot: Jake?
<oopsbot> Jake: You are .....
<oopsbot> Jake has no karma.

Or better yet....

<Jake> word: wasup?
<word> Not much, and you?

And w/oopsbot...

<Jake> oopsbot: wasup?
<oopsbot> hi Jake                       # Greeting.bm
<oopsbot> Not much, and you?            # Infobot.bm
<oopsbot> wasup has no karma.           # Karma.bm


I don't think there's any such thing as a be all/end all bot.  A bot has to have
a niche.  Infobot picked the niche of storing information and the like.  I
always liked oopsbot because of the interface to Bugzilla.  Stuff like
Tinderbox.bm and FTP.bm were nice additions, and Greeting.bm was kinda fun
(although there didn't seem to be any way to turn of the c++ is evil stuff).  I
also like God.bm for its ability to op people when they join the channel.  But I
think an Infobot.bm might be a little much.  Not having an Infobot.bm may mean
that oopsbot isn't popular outside of a software development world that uses
other mozilla.org tools, but is that really a bad thing?
Silly typo, that was supposed to be bug 115638... (both times :)
The Karma problem is fixed by bug 123713.

The other problem (clashing with Greeting) is valid. I'll attach another patch
which slightly changes the implementation to only search for a "foo?" request on
Baffled(), then it will only clash with other module using Baffled().

Note that on the long term, there _is_ a solution to bug 115638, namely the use
of handling priorities (which are already implemented in mozbot.pl, but not yet
widely used by modules).


> I don't think there's any such thing as a be all/end all bot. 

Absolutely nothing is requiring users of mozbot to enable all modules. They need
only enable the modules they want. For example, I wouldn't expect most instances
of mozbot to use the Bugzilla or Tinderbox modules.
Depends on: 123713
Attached patch Patch version 1.2 (obsolete) — Splinter Review
Attachment #68110 - Attachment is obsolete: true
I also removed the following (obsolete) comment in my local copy:

+    # $direct: 0 = heard, 1 = told, 2 = told by bot
Attached patch Patch version 1.3 (obsolete) — Splinter Review
There is a slight bug in the 1.2 version in that if you ask it a question via
/msg, it won't tell you if it later learns something from another bot. This
patch that a fix for this.
Attachment #68144 - Attachment is obsolete: true
The line reading:
            $replace = quotemeta($replace);
...is extraneous and should also be removed.
[13:40:20] <Jake> forget html
[13:40:24] <botbot> forgotten
[13:40:29] <Jake> What is html?
[13:40:36] <botbot> Sorry, I've no idea what 'html' is.
[13:40:38] <Jake> What is html?
[13:40:40] <botbot> html is Hypertext Markup Language, used to create web pages.
See http://www.w3.org/MarkUp/

While doing this, I was also watching word's log.  I noticed the first time I
asked "What is html?", botbot asked word and word responded.  botbot then told
word "ok" and told me it had no idea what html is.  The second time, botbot just
responded, it didn't query word (as I would expect).

I've also noticed that every time someone asks a question (or uses a question
word) in a channel that botbot is in, botbot asks word about it.  This seems
sub-optimal to me as (presumably) botbot will only respond if it's been addressed.
Attached patch Patch version 1.4 (obsolete) — Splinter Review
Your first point was fixed by the change in comment 13.
Your second is fixed by this patch. Good catch.
Attachment #68191 - Attachment is obsolete: true
In response to a review comment, I added the following comment to countFactoids:

    # don't want to use keys() as that would load the whole database
    # index into memory.
Attached patch Patch version 1.5 (obsolete) — Splinter Review
New patch: For long replies, don't spam channel.

Note that botbot is now up to date, so have fun!
Attachment #68550 - Attachment is obsolete: true
have to comment to add myself to the cc.  sorry for the spam.
Attached patch Patch version 1.6 (obsolete) — Splinter Review
This fixes a potential internal bot war (prevents bots chatting to each other
in English...). It also implements "no, nick, foo is bar" syntax (thanks to
kiko) and a different "But x is y" message when re-setting x to y.
Attachment #68708 - Attachment is obsolete: true
It also implements 'literal', which I think was the last outstanding comment.
Target Milestone: --- → Mozbot 2.2
Attached patch Patch version 1.7 (obsolete) — Splinter Review
A version with moderately shorter lines to work around lack of word wrap in
primitive editors and web browsers and to work around the lack of fine grain
diff management in CVS...
Attachment #69185 - Attachment is obsolete: true
teachers [] should mean readonly, make teachers [*] be anyone
pruneDelay, #comment incompl...

edits (learning, editing, forgetting) [you're born, you live, you die] you don't get born die and then live..

+ve in a comment is silly, please use English.

have foo'd, have bah'd, and have bar'd => have foo'd, bah'd and bar'd.

you wrote 'who|what|where' a lot, perhaps that can be factored out.

ForgetFactoid hardcodes db's.  Shouldn't it rely on factoidPositions?

I'm not sure i agree with the behavior of GetFactoid for the $friend case. but if I test now, it'll break me. so we can check this later.

personally, i might prefer :INFOBOT:CURIOUS <botbotthatcares> subject over :INFOBOT:DUNNO ...

Scheduled suffers from time() slide.

I think $helper wanted you to know ... reads better than $original

--
the world acording to infobot:
status is a query.
s/a/a/ is not a modification.
I use out of sequence :INFOBOT:REPLY <self> foo =is=> bar
> teachers [] should mean readonly, make teachers [*] be anyone

I see no advantage to doing that. It would be less efficient and would reduce
usability -- you'd have to both remove and add an entry to the list to turn the
list from an admin-only setting to a free-for-all setting. What are the
advantages of your suggestion? What are the advantages of a totally read only
case? That seems dodgy.


> pruneDelay, #comment incompl...

Fixed.


> edits (learning, editing, forgetting) [you're born, you live, you die] you 
> don't get born die and then live..

Fixed.


> +ve in a comment is silly, please use English.

(you can talk!) Fixed.


> have foo'd, have bah'd, and have bar'd => have foo'd, bah'd and bar'd.

Fixed.


> you wrote 'who|what|where' a lot, perhaps that can be factored out.

hard to do so efficiently.


> ForgetFactoid hardcodes db's.  Shouldn't it rely on factoidPositions?

Databases are hardcoded throughout, starting at the regexps and ending at the
filenames. That problem can wait until someone makes a push for mozbot l10n.


> I'm not sure i agree with the behavior of GetFactoid for the $friend case. but
> if I test now, it'll break me. so we can check this later.

It won't break you, Infobot.bm doesn't exhibit the same broken behaviour as the
original infobot (that is, it doesn't automatically add people to the friend
list). I don't see why bots should respond differently to a QUERY than a normal
question from a user though.


> personally, i might prefer :INFOBOT:CURIOUS <botbotthatcares> subject over 
> :INFOBOT:DUNNO ...

I prefer DUNNO because it sounds more fun. (Plus, it is supposed to be sent out
in response to a QUERY, so it makes more sense.)


> Scheduled suffers from time() slide.

Fixed.


> I think $helper wanted you to know ... reads better than $original

Fixed.


> the world acording to infobot:
> status is a query.
> s/a/a/ is not a modification.

Ah.


> I use out of sequence :INFOBOT:REPLY <self> foo =is=> bar

Use tell. It has the same functionality. :INFOBOT:REPLY is less abusable the way
it is implemented in this module.


I'll attach a new patch.
Attachment #70905 - Attachment is obsolete: true
i'm still trying to puzzle over 'use tell'. sorry this will have to happen 
later.
Comment on attachment 71209 [details] [diff] [review]
Patch version 1.8

with a few changes discussed on irc...
Attachment #71209 - Flags: review+
Horrah!!! Checked in.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
QA Contact: timeless → kerz
QA Contact: kerz → mozbot
Verified Fixed.
Status: RESOLVED → VERIFIED
Product: Webtools → Webtools Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: