Closed GoogleCodeExporter closed 9 years ago
This is a solid patch, I'll review it before 2.0 final, thank you sir.
Original comment by manico.james@gmail.com
on 1 Nov 2010 at 4:26
This patch is Java 1.6 (LinkedBlockingDeque) and ESAPI is 1.5 based. Can you
suggest a 1.5 patch? This is great code - I'd like to get this fix in before
2.0 GA. Any help is appreciated!
Thanks kindly,
Jim
Original comment by manico.james@gmail.com
on 1 Nov 2010 at 4:42
Original comment by manico.james@gmail.com
on 2 Nov 2010 at 8:06
The patch from Lee is 1.6, but we could make this work if we include the JSR166
packport
http://sourceforge.net/projects/backport-jsr166/files/backport-jsr166/3.1/backpo
rt-util-concurrent-Java50-3.1.zip/download
Original comment by manico.james@gmail.com
on 3 Nov 2010 at 5:46
It looks like the basic idea here is that we have a blocking deque that is
being used essentially as a Stack.
So in other words, a request comes in – we get the existing deque off of the
session (or create a new one if there isn’t one yet)
We then push the time of the current request (in ms) onto the head of the
stack.
We then examine the tail of the stack looking for requests older than whatever
the maximum threshold is and remove them from the tail of the stack.
Once we are done maintaining the stack, we check the size of the stack and if
it exceeds the maximum threshold of requests/time then we error the request.
Does this sound like the basic idea?
We could easily do this (using a Queue) in Java 5 without adding the additional
dependencies using a LinkedBlockingQueue
Request comes in, do same logic – check session for existing queue, if absent
create a new one
Push the time in MS onto the tail of the queue
Peek and Remove from the head of the queue for anything older than the time
threshold
Check size of queue and if it exceeds the max threshold of requests/time then
error the request...
This achieves the same logic, with a minimal hit to performance and keeps it
thread-safe without having to add external synchronization or locking. Notice
that basically all we did was flip the Stack from Lee’s implementation into a
Queue so it is still a FIFO collection, just the head and tail are reversed
from the Deque implementation. Also – generally speaking, since you are not
retrieving and pushing onto both ends of the Collection – I’m not sure it
makes sense, and could possibly even be a bit of a performance hit to use a
Deque in this case (although that is largely speculation on my part – it may
be a NIL effect on performance, but definitely seems odd to use a double-ended
collection if you are only working one-way)
The only real benefit I see to using the LinkedBlockingDeque is this statement
from the API Docs:
“Most operations run in constant time (ignoring time spent blocking).”
However, I think that this would be a negligable performance difference due to
the environment – containers are multi-threaded but since we are dealing with
information local to a specific session, I think it is pretty much an
impossibility that you would be able to generate enough simultaneous traffic on
a single session that this would become an issue.
Thoughts?
Original comment by chrisisbeef
on 6 Nov 2010 at 7:54
Original comment by manico.james@gmail.com
on 20 Nov 2010 at 11:30
We can't make the assumption about setting cache-control headers in this
filter. This should be a implementation concern.
Original comment by chrisisbeef
on 18 Sep 2014 at 8:51
Now that we require at least JDK 1.6, we can do this.
The cache related headers can be driven via Java properties; either a new
property in ESAPI.properties or a new property file, depending on the
flexibility required. Given that most of this is already provided, this may
even make an ideal first bug candidate.
Original comment by kevin.w.wall@gmail.com
on 23 Sep 2014 at 1:39
Original issue reported on code.google.com by
blue.re...@gmail.com
on 25 Aug 2010 at 9:37Attachments: