tomcat-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Pete Gould <pete.go...@gmail.com>
Subject Re: CsrfPreventionFilter - LRU cache
Date Fri, 04 Nov 2011 17:06:33 GMT
Hi,

Okay, great. I guess that I should raise a bug for this then.

The reason that I think that add() needs to change is that it used to be:

  cache.put(key, null);

and therefore cache.contains() would return null as it would have to change
to use get(). This is because we can no longer use containsKey(), as that
doesn't count as a structural modification, and therefore the cache would
still be FIFO.

Thanks,

Pete

On Fri, Nov 4, 2011 at 4:58 PM, Christopher Schultz <
chris@christopherschultz.net> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Pete,
>
> On 11/4/11 9:14 AM, Pete Gould wrote:
> > I have recently been using the
> > org.apache.catalina.filters.CsrfPreventionFilter, and I notice that
> > the documentation for setNonceCacheSize states:
> >
> > "Sets the number of previously issued nonces that will be cached on
> > a LRU basis to support parallel requests..."
> >
> > However, looking at the implementation of the cache, it appears to
> > be a FIFO implementation rather than a LRU cache. I'm happy to
> > raise a bug and supply a patch for whichever is the desired
> > implementation, but need to determine what the original intention
> > is first - based on the Javadoc it would suggest that the intention
> > is for the cache to be LRU, could anyone here confirm that?
>
> That's my expectation: LRU means to remove the element that was least
> recently used.
>
> > In order to act as an LRU cache, the LinkedHashMap(int
> > initialCapacity, float loadFactor, boolean accessOrder) constructor
> > would need to be used with accessOrder set to true.
>
> +1
>
> > Also the add and contains methods would need to be altered as
> > follows, as "LinkedHashMap.containsKey" does not act as a
> > structural modification.
> >
> > public void add(T key) { synchronized(cache) { cache.put(key,
> > key); } }
>
>
> I'm not sure add() needs to change.
>
> > public boolean contains(T key) { synchronized(cache) { return
> > cache.get(key) != null; } }
>
> +1
>
> > Either cache implementation will work for the majority of cases,
> > however I came across this issue when issuing Ajax requests which
> > repeatedly use the same nonce string and after 5 requests the value
> > I'm using is ejected from the (FIFO) cache, changing the cache to
> > LRU fixes this (although could potentially result in the same token
> > being used for N requests).
>
> Good catch.
>
> - -chris
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG/MacGPG2 v2.0.17 (Darwin)
> Comment: GPGTools - http://gpgtools.org
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAk60GaMACgkQ9CaO5/Lv0PBQXwCgltBfdKYoJLnCLEHWgnm5ryV4
> s3kAn0K9L4tz2XnzqoHhFSO07EsmINfk
> =ba9N
> -----END PGP SIGNATURE-----
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message