On Thu, 2010-11-25 at 16:33 +0000, Mark Thomas wrote: > I wouldn't call it bad. It doesn't do any harm (apart from adding a very > small amount of overhead), and it would help if the random source > selected ended up not being that random. > > I thought the trade-off of protection against bad choices of random > sources was worth the minimal overhead added. I'm not against removing > it entirely, if there is consensus to do so.
MD5 is now known as a bad hash (it was fine at the time the code was written), so does it actually improve anything ? Also, isn't SecureRandom always available now ? This is 10+ years old code, probably. > For SecureRandom, yes. I did test this locally and it achieves > thread-safety by using an internal sync and it did create a significant > bottleneck. That is why I went the parallel route. Reading from a stream > has a similar sync so again that is why I went the parallel route. Ok. The internal lock is a much smaller sync than the old sync block, so it would be a bit better. It is possible it is noticeable, though. The question is if this yields a good enough session creation rate. > How about this as an approach to reduce the complexity: > 1. Remove the MD5 code (optional) > 2. Default to /dev/urandom then SecureRandom. Don't fall back to Random. > 3. Provide a class that implements Random that reads data from a file > 4. If randomFile is specified, use the the class from 3 as the Random source > > That should reduce the current 3 Queues to one. I doubt it will improve > performance much but it should make the code clearer. > > Thoughts? I don't know what the best solution is. /dev/urandom could also only be used as seed only to a SecureRandom, this is more Javaish. So about the MD5, I don't think it is useful anymore. Rémy --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
