search

chrismou / phergie-irc-plugin-react-dice

Phergie plugin for provides randomly generated numbers in response to dice rolling requests (https://github.com/phergie/phergie-irc-bot-react)
BSD 2-Clause "Simplified" License
0 stars 1 forks source link

Large inputs can exceed memory_limit #2

Closed elazar closed 9 years ago

elazar commented 9 years ago

Issuing this command against the Phergie v3 instance on Freenode...

!dice 10000000 10000000

... causes this error...

[30-Apr-2015 16:25:04 America/Chicago] PHP Fatal error:  Allowed memory size of 134217728 bytes exhausted (tried to allocate 32 bytes) in /home/mturland/phergie-freenode/vendor/chrismou/phergie-irc-plugin-react-dice/src/Plugin.php on line 105

Here is the cited line. Plugin->handleCommand() should probably sanity check its roll count by comparing the return value of memory_get_usage() against the return value of ini_get('memory_limit') on each loop iteration to ensure that the current roll won't cause memory_limit to be exceeded.

chrismou commented 9 years ago

Do you know if irc has a max character limit for a message?

elazar commented 9 years ago

It does, but it's difficult to account for how much of that any given message is going to use because of prefixes and such. After further discussion, it may just be safer to have some reasonable upper bounds (e.g. 100 or 1000), perhaps that can be overridden by configuration.

chrismou commented 9 years ago

That was my thinking. I'll get this sorted this weekend, cheers for the heads up.

chrismou commented 9 years ago

I've added a few limits that can be overridden via config. The numbers are low enough that by default it should be OK.

I just need to update a few tests and I'll get it tagged

chrismou commented 9 years ago

Tag 1.0.2 released

elazar commented 9 years ago

Think you added Pong and AutoJoin for testing and forgot to take them out.

chrismou commented 9 years ago

You're right - though that was on the AudioScrobbler plugin!

elazar commented 9 years ago

Appears it was on both. :p I need to come up with a better recommended setup for plugin developers.

chrismou commented 9 years ago

Am I going crazy? I don't see it on this repo

elazar commented 9 years ago

Ah, OK, looks like you removed them in a subsequent commit.