Details
-
Bug
-
Status: Closed
-
Major
-
Resolution: Fixed
-
1.1 RC1
-
None
-
Operating System: All
Platform: All
-
19223
Description
The current implementation uses the following:
HttpSession session = request.getSession(false);
...
synchronized (session) {
There is no guarantee that the object returned from request.getSession will be
the same instance for different threads that are servicing requests within the
same session. A container is free to return a new instance of an object that
implements HttpSession for each call. Therefore, synchronizing on that object
may not have any effect.
Off the top of my head, I can't really think of a decent solution. Essentially,
you either need a per-session lock object (how?) or you need to just synchronize
a block of code on "this" (overkill).
An additional thing to note is that fetching the request parameter does not
need to take place within a synchronized block. So, a non-synchronized version
would look like this:
protected boolean isTokenValid(HttpServletRequest request, boolean reset) {
boolean isValid = false;
String token = request.getParameter(Constants.TOKEN_KEY);
if (token != null) {
HttpSession session = request.getSession(false);
if (session != null) {
// Synchronization should start here...
String saved = (String)session.getAttribute(TRANSACTION_TOKEN_KEY);
if (saved != null) {
if (reset)
isValid = saved.equals(token);
}
// ...and end here
}
}
return isValid;
}
It also seems like resetToken and saveToken should be synchronized in concert
with isTokenValid...