Geta / 404handler

The popular 404 handler for EPiServer, enabling better control over your 404 page in addition to allowing redirects for old urls that no longer works.
Apache License 2.0
55 stars 51 forks source link

question: antispam logic is strange #163

Open lanorkin opened 5 years ago

lanorkin commented 5 years ago

Logic in LogRequests method https://github.com/Geta/404handler/blob/6b0c6a09db559c47f4353585c7f8bbd975925091/src/Core/Logging/RequestLogger.cs#L42 looks really strange to me.

In current state it tries to postpone message saving to database until after average requests per second becomes lower than threshold, BUT if it starts saving, it will save whole queue at once regardless number of messages in queue and buffer size - while still locking all the threads here https://github.com/Geta/404handler/blob/6b0c6a09db559c47f4353585c7f8bbd975925091/src/Core/Logging/RequestLogger.cs#L24

So being under heavy (spam?) load, we sooner or later will lock all the threads (because logic just depends on time, as soon as number of seconds > than buffer size, we'll start saving), and start saving all the spam messages in database.

Is it supposed to work in this way? Or should it instead respect bufferSize during saving to save in batches? Or maybe it supposed to cleanup queue in warning branch?

evest commented 5 years ago

Looks like this has changed a bit since it's inception, so I won't assume too much about the current implementation.

However, the original idea was to handle spam or excessive 404 gracefully, preferably lossy, since one could speculate in filling the database over time, taking the site down. With locking, this seems just as important, if not more.