TerminalWitchcraft / actix-ratelimit

Rate limiter framework for Actix web
MIT License
126 stars 25 forks source link

fixed substract with overwlow for in memory store #3

Closed tglman closed 4 years ago

tglman commented 4 years ago

Hi,

while I was running some tests in debug mode i stumbled on a panic:

thread 'actix-rt:worker:0' panicked at 'attempt to subtract with overflow', /home/tglman/.cargo/registry/src/github.com-1ecc6299db9ec823/actix-ratelimit-0.2.1/src/stores/memory.rs:90:21
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.46/src/backtrace/libunwind.rs:86
   1: backtrace::backtrace::trace_unsynchronized
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.46/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print_fmt
             at src/libstd/sys_common/backtrace.rs:78
   3: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
             at src/libstd/sys_common/backtrace.rs:59
   4: core::fmt::write
             at src/libcore/fmt/mod.rs:1069
   5: std::io::Write::write_fmt
             at src/libstd/io/mod.rs:1504
   6: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:62
   7: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:49
   8: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:198
   9: std::panicking::default_hook
             at src/libstd/panicking.rs:218
  10: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:511
  11: rust_begin_unwind
             at src/libstd/panicking.rs:419
  12: core::panicking::panic_fmt
             at src/libcore/panicking.rs:111
  13: core::panicking::panic
             at src/libcore/panicking.rs:54
  14: <actix_ratelimit::stores::memory::MemoryStoreActor as actix::handler::Handler<actix_ratelimit::ActorMessage>>::handle
             at /home/tglman/.cargo/registry/src/github.com-1ecc6299db9ec823/actix-ratelimit-0.2.1/src/stores/memory.rs:90
  15: <actix::address::envelope::SyncEnvelopeProxy<A,M> as actix::address::envelope::EnvelopeProxy>::handle
             at /home/tglman/.cargo/registry/src/github.com-1ecc6299db9ec823/actix-0.9.0/src/address/envelope.rs:112
  16: <actix::address::envelope::Envelope<A> as actix::address::envelope::EnvelopeProxy>::handle
             at /home/tglman/.cargo/registry/src/github.com-1ecc6299db9ec823/actix-0.9.0/src/address/envelope.rs:71
  17: actix::mailbox::Mailbox<A>::poll
             at /home/tglman/.cargo/registry/src/github.com-1ecc6299db9ec823/actix-0.9.0/src/mailbox.rs:101
  18: <actix::contextimpl::ContextFut<A,C> as core::future::future::Future>::poll

I think the correct way here is to never go lower than 0, that actually would cause a really big value and maybe invalidate the rate limit.

This do not have big issue in production because in release mode overflow do not panic.

This fix PR change the code to never get a value less than 0 avoid the overflow

TerminalWitchcraft commented 4 years ago

Thanks for pointing it out!