Skip to content

Implement HalFilters via RI injection#132

Merged
richteer merged 4 commits into
masterfrom
filter-ri-injection
Mar 11, 2019
Merged

Implement HalFilters via RI injection#132
richteer merged 4 commits into
masterfrom
filter-ri-injection

Conversation

@richteer

@richteer richteer commented Mar 5, 2019

Copy link
Copy Markdown
Contributor

In the never ending quest to make Halibot aware of its own spam, here is an implementation of filters that inject themselves into a send path based on a config.json filter table. Filters are implemented as a new HalObject type, and now also have their own filter-instances key in config.json. See below for an example config.

Overview

When a message is sent via .send_to() (and thus also .reply(), .dispatch()), the base HalObject class first checks the filter table in config.json for any filters that may need to be applied. The filter table is a top-level config key filter, that contains two sub keys: inbound and outbound. The inbound key maps a target object to a list of filters that should be applied first. The outbound key maps the sending object to a list of filters. The sending logic first prepends any valid inbound filters, then prepends any outbound filters. Finally, the message is sent to the first item in the RI list.

When a filter receives a message object, it first strips off the first item in the RI list. This preserves the chain, and prevents a loop. When the message exits the filter chain, the target should be the original intended target prior to the filter injection. See example below for how this chain resolves.

Why...

...another HalObject

I think it makes sense that it maintains its own state, including message queue. A filter may want to simply delay sending a message, instead of outright denying it. A filter may want to modify a message and completely change the target. Also, it's much much much easier to steal all the same module/agent loading logic.

...not make this a module?

In this implementation, HalFilters take a special role that makes them quite different from other modules or agents. They aren't intended to process the data themselves, only to slightly tweak some magic in between. They are like a detour when traveling, if received at the same destination, it doesn't particularly matter which road was taken to get there.

They also require a bit of extra special magic that hasn't quite made it into Halibot yet: special routing. This allows us to play with a baby version of a routing scheme we discussed long ago, without completely overhauling the current system.

...do you need #131

Because without it, it would send messages in a loop forever.

There needs to be a way to inject the filter's RIs into the target path somehow, so .send_to seemed to be a good place to put it. Only then did I realize, it is super easy to get caught in a loop that way. So, I needed HalFilter to send a message without any possible accidental filter injection. Thus, .raw_send was born: a way to send a message to a target without any interference.

Example config.json

{
  "filter-instances": {
    "rlfilter0": {
      "of": "RateLimitFilter:Default",
      "rate": 10
    },
    "rlfilter1": {
      "of": "RateLimitFilter:Default",
      "rate": 6000
    }
  },


  "filters": {
    "inbound": {
      "irc0": [ "rlfilter0" ]
    },
    
    "outbound": {
      "quote0": [ "rlfilter1" ]
    }
  }
}

In this example, all messages flowing to agent irc0 would first pass through rlfilter0. Similarly, all messages passing from module quote0 would first pass through rlfilter1.

The admin decided to do this so that the Halibot instance can only send to any irc endpoints once every ten seconds. Furthermore, since quote0 can be a somewhat spammy module, the admin specifically puts that module behind an outbound filter, so quote0 can only reply once every ten minutes.

When a !quote command is received, both filters apply. That is, the reply message for the irc endpoint from quote0 must successfully pass through rlfilter1 and rlfilter0. The target RI generated is thus rlfilter1/rlfilter0/irc0/##somechannel.

Getting into more detail, the chain resolves thusly:

  • quote0 calls a send_to (via reply) targeting irc0/##halibot
  • HalObject.send_to injects rlfilter1 and rlfilter0, so the new RI is rlfilter1/rlfilter0/irc0/##halibot
  • rlfilter1 receives the message, accepts it, and sends to the new RI rlfilter0/irc0/##halibot
  • rlfilter0 receives the message, accepts it, and sends to the new RI irc0/##halibot
  • irc0 receives the message, and does the normal agenty thing
    NOTE: in all cases, msg.origin remains the same: quote0.

Draft Notes

As this is a draft, I am very aware at some lacking features, implementations etc. As of now, here are the tasks still needed to be completed:

  • Add CLI support
  • Implement an example filter
  • Expand testing to support filter chains

Comment thread halibot/halobject.py Outdated
ret = "/".join(ib[name] + [dest]) if name in ib else dest

ob = fl.get("outbound", {})
ret = "/".join(ob[self.name] + [ret]) if self.name in ob else ret

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm growing increasingly displeased with verbatim string matching for RIs. Starting to think it might be worth adding an abstraction to handle RI comparison. Something where a check for irc will pass for both irc and irc/##halibot, with the former referring only to the agent (general case), the latter referring to a specific resource to/from irc.

As it stands, it occurred to me that this doesn't support per-channel filters, which might be useful for limiting output to a specific channel/user, rather than for all resources behind a particular agent. I can hack in support, but there's a deeper issue here I feel.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I agree, we really shouldn't be spliting/joining on "/" everywhere. It definitely should be abstracted somewhere.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should we open a new issue to track this? I think this is fine for now, but we probably want some way to allow per-channel filtering as well.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@richteer richteer mentioned this pull request Mar 5, 2019

@sjrct sjrct left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should add a filters property to ObjectDict in halibot.py too. Overall looks good to go though.

Comment thread halibot/halobject.py Outdated
ret = "/".join(ib[name] + [dest]) if name in ib else dest

ob = fl.get("outbound", {})
ret = "/".join(ob[self.name] + [ret]) if self.name in ob else ret

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I agree, we really shouldn't be spliting/joining on "/" everywhere. It definitely should be abstracted somewhere.

Comment thread halibot/halobject.py Outdated
name = dest.split("/")[0]

ib = fl.get("inbound", {})
ret = "/".join(ib[name] + [dest]) if name in ib else dest

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ret = "/".join(ib.get(name, []) + [dest]) is shorter but maybe more opaque

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ahhhh this was the one liner I knew was in there somewhere. Opague, but cleaner.

@richteer
richteer force-pushed the filter-ri-injection branch from 7b19468 to 867c0c5 Compare March 5, 2019 16:55
@richteer

richteer commented Mar 5, 2019

Copy link
Copy Markdown
Contributor Author

Update: cleaned up the .apply_filter() logic slightly, made the prepend operations a little more terse

@richteer

richteer commented Mar 5, 2019

Copy link
Copy Markdown
Contributor Author

Update: rebased onto #131 , merge that and #136 first.

richteer added 3 commits March 8, 2019 14:16
Functions like .send_to() really should be a helper function that sets the
target of a message for you (and handles multiple targets), but it shouldn't
be the bottom sending function itself.

This commit moves the actual send logic (including copy) out of the .send_to
function, and into its own function called .raw_send(). A `Message` object
parameter MUST be populated with a valid `.target` and `.origin` parameter.
Problem: How can we introduce rate limiting, self-censoring, or other kind of
logic into Halibot instances without adding this to each module or agent?

This patch introduces a new Halibot object type "HalFilter". These filters can
be inserted in between a normal message send path, to silently drop or modify
message content. These filters are NOT meant for large behaviors.

Filters are instantiated similarly to other HalObjects, using the
"filter-instances" config key. To "attach" a filter to an agent or module, a
new filter table needs to be defined in config.json.

Here is an example:

"filters": {
    "inbound": {
      "irc0": [ "rlfilter0" ]
    },

    "outbound": {
      "quote0": [ "rlfilter1" ]
    }
}

In this example, "rlfilter0" and "rlfilter1" are instances of a rate-limiting
filter. The above declaration states that all messages from any source being
sent to irc0 must first pass through rlfilter0. Also, any messages flowing out
from module quote0 must first pass through rlfilter1.

As rlfilter0 and rlfilter1 are different instances, they may have different
tolerances for their rate limiter. So, for example, this could restrict the
amount of spam coming from one particular module more strictly than all
messages flowing to an agent.
@richteer
richteer force-pushed the filter-ri-injection branch from ce7d959 to c48d22a Compare March 8, 2019 20:17
@richteer
richteer marked this pull request as ready for review March 8, 2019 21:12
@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.3%) to 93.35% when pulling c48d22a on filter-ri-injection into fb54463 on master.

@richteer
richteer merged commit 696a05e into master Mar 11, 2019
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.

3 participants