svn commit: r1073289 - /tomcat/trunk/java/org/apache/catalina/realm/JNDIRealm.java
Author: markt Date: Tue Feb 22 10:46:02 2011 New Revision: 1073289 URL: http://svn.apache.org/viewvc?rev=1073289&view=rev Log: Refactoring to remove duplicate code Modified: tomcat/trunk/java/org/apache/catalina/realm/JNDIRealm.java Modified: tomcat/trunk/java/org/apache/catalina/realm/JNDIRealm.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/realm/JNDIRealm.java?rev=1073289&r1=1073288&r2=1073289&view=diff == --- tomcat/trunk/java/org/apache/catalina/realm/JNDIRealm.java (original) +++ tomcat/trunk/java/org/apache/catalina/realm/JNDIRealm.java Tue Feb 22 10:46:02 2011 @@ -1312,27 +1312,11 @@ public class JNDIRealm extends RealmBase // If the getUserByPattern() call fails, try it again with the // credentials of the user that we're searching for try { -// Set up security environment to bind as the user -context.addToEnvironment(Context.SECURITY_PRINCIPAL, dn); -context.addToEnvironment(Context.SECURITY_CREDENTIALS, credentials); +userCredentialsAdd(context, dn, credentials); user = getUserByPattern(context, username, attrIds, dn); } finally { -// Restore the original security environment -if (connectionName != null) { -context.addToEnvironment(Context.SECURITY_PRINCIPAL, - connectionName); -} else { -context.removeFromEnvironment(Context.SECURITY_PRINCIPAL); -} - -if (connectionPassword != null) { -context.addToEnvironment(Context.SECURITY_CREDENTIALS, - connectionPassword); -} -else { - context.removeFromEnvironment(Context.SECURITY_CREDENTIALS); -} +userCredentialsRemove(context); } } return user; @@ -1590,9 +1574,7 @@ public class JNDIRealm extends RealmBase containerLog.trace(" validating credentials by binding as the user"); } -// Set up security environment to bind as the user -context.addToEnvironment(Context.SECURITY_PRINCIPAL, dn); -context.addToEnvironment(Context.SECURITY_CREDENTIALS, credentials); +userCredentialsAdd(context, dn, credentials); // Elicit an LDAP bind operation boolean validated = false; @@ -1609,6 +1591,35 @@ public class JNDIRealm extends RealmBase } } +userCredentialsRemove(context); + +return (validated); +} + + /** + * Configure the context to use the provided credentials for + * authentication. + * + * @param context DirContext to configure + * @param dn Distinguished name of user + * @param credentials Credentials of user + */ +private void userCredentialsAdd(DirContext context, String dn, +String credentials) throws NamingException { +// Set up security environment to bind as the user +context.addToEnvironment(Context.SECURITY_PRINCIPAL, dn); +context.addToEnvironment(Context.SECURITY_CREDENTIALS, credentials); +} + +/** + * Configure the context to use {@link #connectionName} and + * {@link #connectionPassword} if specified or an anonymous connection if + * those attributes are not specified. + * + * @param context DirContext to configure + */ +private void userCredentialsRemove(DirContext context) +throws NamingException { // Restore the original security environment if (connectionName != null) { context.addToEnvironment(Context.SECURITY_PRINCIPAL, @@ -1624,9 +1635,7 @@ public class JNDIRealm extends RealmBase else { context.removeFromEnvironment(Context.SECURITY_CREDENTIALS); } - -return (validated); - } +} /** * Return a List of roles associated with the given User. Any - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
svn commit: r1073296 - in /tomcat/trunk: java/org/apache/catalina/realm/JNDIRealm.java webapps/docs/changelog.xml webapps/docs/config/realm.xml
Author: markt Date: Tue Feb 22 11:08:42 2011 New Revision: 1073296 URL: http://svn.apache.org/viewvc?rev=1073296&view=rev Log: Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=19444 Add an option to the JNDI realm to allow role searches to be performed by the authenticated user. Modified: tomcat/trunk/java/org/apache/catalina/realm/JNDIRealm.java tomcat/trunk/webapps/docs/changelog.xml tomcat/trunk/webapps/docs/config/realm.xml Modified: tomcat/trunk/java/org/apache/catalina/realm/JNDIRealm.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/realm/JNDIRealm.java?rev=1073296&r1=1073295&r2=1073296&view=diff == --- tomcat/trunk/java/org/apache/catalina/realm/JNDIRealm.java (original) +++ tomcat/trunk/java/org/apache/catalina/realm/JNDIRealm.java Tue Feb 22 11:08:42 2011 @@ -365,7 +365,14 @@ public class JNDIRealm extends RealmBase */ protected boolean roleNested = false; - +/** + * When searching for user roles, should the search be performed as the user + * currently being authenticated? If false, {@link #connectionName} and + * {@link #connectionPassword} will be used if specified, else an anonymous + * connection will be used. + */ +protected boolean roleSearchAsUser = false; + /** * An alternate URL, to which, we should connect if connectionURL fails. */ @@ -1692,8 +1699,18 @@ public class JNDIRealm extends RealmBase controls.setReturningAttributes(new String[] {roleName}); // Perform the configured search and process the results -NamingEnumeration results = -context.search(roleBase, filter, controls); +NamingEnumeration results = null; +try { +if (roleSearchAsUser) { +userCredentialsAdd(context, dn, user.getPassword()); +} +results = context.search(roleBase, filter, controls); +} finally { +if (roleSearchAsUser) { +userCredentialsRemove(context); +} +} + if (results == null) return (list); // Should never happen, but just in case ... Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1073296&r1=1073295&r2=1073296&view=diff == --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Tue Feb 22 11:08:42 2011 @@ -45,6 +45,10 @@ + +19444: Add an option to the JNDI realm to allow role searches +to be performed by the authenticated user. (markt) + 48863: Better logging when specifying an invalid directory for a class loader. Based on a patch by Ralf Hauser. (markt) Modified: tomcat/trunk/webapps/docs/config/realm.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/config/realm.xml?rev=1073296&r1=1073295&r2=1073296&view=diff == --- tomcat/trunk/webapps/docs/config/realm.xml (original) +++ tomcat/trunk/webapps/docs/config/realm.xml Tue Feb 22 11:08:42 2011 @@ -412,6 +412,14 @@ property. + + When searching for user roles, should the search be performed as the +user currently being authenticated? If false, +connectionName} and connectionPassword will be +used if specified, else an anonymous. If not specified, the default +value of false is used. + + Set to true if you want to search the entire subtree of the element specified by the roleBase - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 19444] JNDI Authentication roles must be anonymous accessible
https://issues.apache.org/bugzilla/show_bug.cgi?id=19444 Mark Thomas changed: What|Removed |Added Status|NEW |RESOLVED Resolution||FIXED --- Comment #6 from Mark Thomas 2011-02-22 06:10:26 EST --- The code has moved on somewhat since this enhancement request was made so the patch does not apply cleanly. I have added a new JNDIRealm option in Tomcat 7.0.x to optionally allow role searches as the user being authenticated rather than using using an anonymous user or using connectionName/connectionPassword. The new option will be included in 7.0.9 onwards. -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
svn commit: r1073301 - in /tomcat/trunk: java/org/apache/catalina/realm/JNDIRealm.java webapps/docs/changelog.xml webapps/docs/config/realm.xml
Author: markt Date: Tue Feb 22 11:48:09 2011 New Revision: 1073301 URL: http://svn.apache.org/viewvc?rev=1073301&view=rev Log: Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=21669 Add the ability to specify the roleBase for the JNDI Realm as relative to the users DN. Based on a patch by Art W. Modified: tomcat/trunk/java/org/apache/catalina/realm/JNDIRealm.java tomcat/trunk/webapps/docs/changelog.xml tomcat/trunk/webapps/docs/config/realm.xml Modified: tomcat/trunk/java/org/apache/catalina/realm/JNDIRealm.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/realm/JNDIRealm.java?rev=1073301&r1=1073300&r2=1073301&view=diff == --- tomcat/trunk/java/org/apache/catalina/realm/JNDIRealm.java (original) +++ tomcat/trunk/java/org/apache/catalina/realm/JNDIRealm.java Tue Feb 22 11:48:09 2011 @@ -330,6 +330,13 @@ public class JNDIRealm extends RealmBase /** * The MessageFormat object associated with the current + * roleBase. + */ +protected MessageFormat roleBaseFormat = null; + + +/** + * The MessageFormat object associated with the current * roleSearch. */ protected MessageFormat roleFormat = null; @@ -697,6 +704,10 @@ public class JNDIRealm extends RealmBase public void setRoleBase(String roleBase) { this.roleBase = roleBase; +if (roleBase == null) +roleBaseFormat = null; +else +roleBaseFormat = new MessageFormat(roleBase); } @@ -1698,13 +1709,24 @@ public class JNDIRealm extends RealmBase controls.setSearchScope(SearchControls.ONELEVEL_SCOPE); controls.setReturningAttributes(new String[] {roleName}); +String base = null; +if (roleBaseFormat != null) { +NameParser np = context.getNameParser(""); +Name name = np.parse(dn); +String nameParts[] = new String[name.size()]; +for (int i = 0; i < name.size(); i++) { +nameParts[i] = name.get(i); +} +base = roleBaseFormat.format(nameParts); +} + // Perform the configured search and process the results NamingEnumeration results = null; try { if (roleSearchAsUser) { userCredentialsAdd(context, dn, user.getPassword()); } -results = context.search(roleBase, filter, controls); +results = context.search(base, filter, controls); } finally { if (roleSearchAsUser) { userCredentialsRemove(context); Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1073301&r1=1073300&r2=1073301&view=diff == --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Tue Feb 22 11:48:09 2011 @@ -49,6 +49,10 @@ 19444: Add an option to the JNDI realm to allow role searches to be performed by the authenticated user. (markt) + +21669: Add the ability to specify the roleBase for the JNDI +Realm as relative to the users DN. Based on a patch by Art W. (markt) + 48863: Better logging when specifying an invalid directory for a class loader. Based on a patch by Ralf Hauser. (markt) Modified: tomcat/trunk/webapps/docs/config/realm.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/config/realm.xml?rev=1073301&r1=1073300&r2=1073301&view=diff == --- tomcat/trunk/webapps/docs/config/realm.xml (original) +++ tomcat/trunk/webapps/docs/config/realm.xml Tue Feb 22 11:48:09 2011 @@ -387,9 +387,12 @@ -The base directory entry for performing role searches. If -not specified the top-level element in the directory context -will be used. +The base directory entry for performing role searches. If not +specified the top-level element in the directory context will be used. +If specified it may optionally include pattern replacements +"{0}".."{n}" corrosponding to the name parts of the +user's distinguished name (as returned by +javax.naming.Name.get()). - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 21669] JNDIRealm roleBase pattern enahncement
https://issues.apache.org/bugzilla/show_bug.cgi?id=21669 Mark Thomas changed: What|Removed |Added Status|NEW |RESOLVED Resolution||FIXED --- Comment #1 from Mark Thomas 2011-02-22 06:48:41 EST --- Thanks for the suggestion. Sorry it took so long to get to. I have added this feature to 7.0.x and it will be included in 7.0.9 onwards. -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
svn commit: r1073320 - /tomcat/trunk/java/org/apache/jasper/runtime/PageContextImpl.java
Author: markt Date: Tue Feb 22 12:55:56 2011 New Revision: 1073320 URL: http://svn.apache.org/viewvc?rev=1073320&view=rev Log: Remove unused code, fix FindBugs warning Modified: tomcat/trunk/java/org/apache/jasper/runtime/PageContextImpl.java Modified: tomcat/trunk/java/org/apache/jasper/runtime/PageContextImpl.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/runtime/PageContextImpl.java?rev=1073320&r1=1073319&r2=1073320&view=diff == --- tomcat/trunk/java/org/apache/jasper/runtime/PageContextImpl.java (original) +++ tomcat/trunk/java/org/apache/jasper/runtime/PageContextImpl.java Tue Feb 22 12:55:56 2011 @@ -424,7 +424,7 @@ public class PageContextImpl extends Pag .doPrivileged(new PrivilegedAction() { @Override public Integer run() { -return new Integer(doGetAttributeScope(name)); +return Integer.valueOf(doGetAttributeScope(name)); } })).intValue(); } else { @@ -587,10 +587,6 @@ public class PageContextImpl extends Pag return session; } -public Servlet getServlet() { -return servlet; -} - @Override public ServletConfig getServletConfig() { return config; - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 22755] say which variable is at fault! ("Cannot compare null variable to value true")
https://issues.apache.org/bugzilla/show_bug.cgi?id=22755 Mark Thomas changed: What|Removed |Added Status|NEW |RESOLVED Resolution||WONTFIX --- Comment #1 from Mark Thomas 2011-02-22 08:11:47 EST --- Jasper is (and was back in 4.1.24) doing everything it can in terms of propagating the exception and correct exception chaining. I can't find anything that suggests "Cannot compare..." is an error message generated by a Tomcat component. Neither can I reproduce this based on guessing what the root cause might have been. Without a reproducible test case, it looks like some third party component (a taglib maybe?) is generating this message. -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
svn commit: r1073393 - in /tomcat/trunk/java/org/apache/jasper: compiler/Compiler.java resources/LocalStrings.properties
Author: markt Date: Tue Feb 22 16:38:39 2011 New Revision: 1073393 URL: http://svn.apache.org/viewvc?rev=1073393&view=rev Log: Test before generating debug log messages Fix FindBugs issues for File.delete() return value Modified: tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java tomcat/trunk/java/org/apache/jasper/resources/LocalStrings.properties Modified: tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java?rev=1073393&r1=1073392&r2=1073393&view=diff == --- tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java (original) +++ tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java Tue Feb 22 16:38:39 2011 @@ -271,7 +271,14 @@ public abstract class Compiler { } } // Remove the generated .java file -new File(javaFileName).delete(); +File file = new File(javaFileName); +if (file.exists()) { +if (!file.delete()) { +log.warn(Localizer.getMessage( +"jsp.warning.compiler.javafile.delete.fail", +file.getAbsolutePath())); +} +} throw e; } finally { if (writer != null) { @@ -451,7 +458,8 @@ public abstract class Compiler { } uc.getInputStream().close(); } catch (Exception e) { -log.debug("Problem accessing resource. Treat as outdated.", e); +if (log.isDebugEnabled()) +log.debug("Problem accessing resource. Treat as outdated.", e); return true; } @@ -514,7 +522,9 @@ public abstract class Compiler { return true; } } catch (Exception e) { -log.debug("Problem accessing resource. Treat as outdated.", e); +if (log.isDebugEnabled()) +log.debug("Problem accessing resource. Treat as outdated.", +e); return true; } } @@ -551,7 +561,13 @@ public abstract class Compiler { File classFile = new File(classFileName); if (log.isDebugEnabled()) log.debug("Deleting " + classFile); -classFile.delete(); +if (classFile.exists()) { +if (!classFile.delete()) { +log.warn(Localizer.getMessage( +"jsp.warning.compiler.classfile.delete.fail", +classFile.getAbsolutePath())); +} +} } } catch (Exception e) { // Remove as much as possible, ignore possible exceptions @@ -562,7 +578,13 @@ public abstract class Compiler { File javaFile = new File(javaFileName); if (log.isDebugEnabled()) log.debug("Deleting " + javaFile); -javaFile.delete(); +if (javaFile.exists()) { +if (!javaFile.delete()) { +log.warn(Localizer.getMessage( +"jsp.warning.compiler.javafile.delete.fail", +javaFile.getAbsolutePath())); +} +} } } catch (Exception e) { // Remove as much as possible, ignore possible exceptions @@ -576,7 +598,13 @@ public abstract class Compiler { File classFile = new File(classFileName); if (log.isDebugEnabled()) log.debug("Deleting " + classFile); -classFile.delete(); +if (classFile.exists()) { +if (!classFile.delete()) { +log.warn(Localizer.getMessage( +"jsp.warning.compiler.classfile.delete.fail", +classFile.getAbsolutePath())); +} +} } } catch (Exception e) { // Remove as much as possible, ignore possible exceptions Modified: tomcat/trunk/java/org/apache/jasper/resources/LocalStrings.properties URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/resources/LocalStrings.properties?rev=1073393&r1=1073392&r2=1073393&view=diff == --- tomcat/trunk/java/org/apache/jasper/resources/LocalStrings.properties (original) +++ tomcat/trunk/java/org/apache/jasper/resources/LocalStrings.properties Tue Feb 22 16:38:39 2011 @@ -227,6 +227,8 @@ jsp.error.bad_string_char=Cannot extract jsp.warning.compiler.class.cantcreate=Can't create an instance of specified compiler plugin class {0} due to {1}.
DO NOT REPLY [Bug 24739] Control of secure flag when establishing sessions through https using cookies
https://issues.apache.org/bugzilla/show_bug.cgi?id=24739 --- Comment #4 from Andrew Mottaz 2011-02-22 11:58:55 EST --- How can you say there are no valid use cases? Virtually EVERY ecommerce site on the internet supports this behavior. Amazon.com, Apple.com, Dell.com. Basically - whether a session sticks after secure access is based solely on the whim of your first access method? Just give developers control. It can still default to secure - make it programatic to explicitly use insecure. Right now, hundreds of sites have to do a redirect to an insecure page to establish the session. -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 24739] Control of secure flag when establishing sessions through https using cookies
https://issues.apache.org/bugzilla/show_bug.cgi?id=24739 --- Comment #5 from Andrew Mottaz 2011-02-22 11:59:24 EST --- How can you say there are no valid use cases? Virtually EVERY ecommerce site on the internet supports this behavior. Amazon.com, Apple.com, Dell.com. Basically - whether a session sticks after secure access is based solely on the whim of your first access method? Just give developers control. It can still default to secure - make it programatic to explicitly use insecure. Right now, hundreds of sites have to do a redirect to an insecure page to establish the session. -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 24739] Control of secure flag when establishing sessions through https using cookies
https://issues.apache.org/bugzilla/show_bug.cgi?id=24739 --- Comment #6 from Mark Thomas 2011-02-22 12:46:02 EST --- (In reply to comment #5) > How can you say there are no valid use cases? Virtually EVERY ecommerce site > on the internet supports this behavior. No they don't. At least the securely written ones don't. I thoroughly recommend taking a detailed look at how Amazon does this - or at least as much as can be deduced from looking at the HTTP headers from the client side. There is more to it than a single session. Amazon has multiple cookies. I see 5 for that don't have the secure flag set and one that does. The non-secure cookies are what allows Amazon to determine who you are when you connect over http but you can't access any security sensitive information (past orders, addresses, credit card details etc). For that you have to use https and that requires authentication or the presence of a valid secure cookie. The Amazon application is using a far more sophisticated model than the single session with a single cookie model provided by the Servlet specification. If you want that sort of model as used by Amazon and others then you'll need to either code it yourself or use a framework that provides it. With respect to this particular bug the primary concern of the Tomcat committers is security. If a session is created over https then it must remain over https in order to remain secure. As I have said previously, if a valid use case for creating a non-secure session cookie over https that does not compromise security is presented then this will be re-considered but until such time it remains WONTFIX. -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 24739] Control of secure flag when establishing sessions through https using cookies
https://issues.apache.org/bugzilla/show_bug.cgi?id=24739 --- Comment #7 from Andrew Mottaz 2011-02-22 13:04:11 EST --- You actually made my point SOME of the cookies are not secure. My point is not that you should never have secure session cookies. It's that sometimes you don't want them secure. So - if it is appropriate for the session cookie to NOT be secure ( i.e. - I'm identified but don't have any special privileges ), then I have to make sure that they hit a non-secure page first, instead of a secure log-in page. Look at facebook. You actually make the security issue worse because the security level of the cookie is seemingly arbitrary and undocumented: i.e. -- if you hit a secure page first its secure. If you hit a non-secure page first its not secure. Why not just make it an explicit setting -- Session Cookie secure or not secure. Then the developer decides explictly. As it is now, it is confusing and arbirtary, and requires that you control every access to the site -- which has a greater likelihood - that someone allows sessions to accidentally be created on a non-secure page, or that someone sets a value EXPLICITLY to non-secure when they really meant that it be secure. It's been 8 years - I don't really care about this to the extent that there are workarounds, but the proper solution IMHO is to make the security of the session cookie explicit. This improves both cases - Its MORE secure, and I can allow insecure session cookies from a secured first page log-in. -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Reaching ulimit values for open files can generate huge log files
Wouldn't you build this into the logging framework, instead of one specific component? best Filip On 02/21/2011 08:21 AM, Mark Thomas wrote: The ASF Sonar installation managed to generate 46GB of identical log messages [1] today in the 8 hours it took to notice it was down. While better monitoring would/should have identified the problem sooner, it does demonstrate a problem with the acceptor threads in all three endpoints. If there is a system-level issue that causes the accept() call to always fail (such as hitting the ulimit) then the endpoint essentially enters a loop where it logs an error message for every iteration of the loop. This will result in many log messages per second. I'd like to do something about this. I was thinking of something along the lines of the following for each endpoint. Index: java/org/apache/tomcat/util/net/JIoEndpoint.java === --- java/org/apache/tomcat/util/net/JIoEndpoint.java(revision 1072939) +++ java/org/apache/tomcat/util/net/JIoEndpoint.java(working copy) @@ -183,9 +183,19 @@ @Override public void run() { +int errorDelay = 0; + // Loop until we receive a shutdown command while (running) { +if (errorDelay> 0) { +try { +Thread.sleep(errorDelay); +} catch (InterruptedException e) { +// Ignore +} +} + // Loop if endpoint is paused while (paused&& running) { try { @@ -225,9 +235,15 @@ // Ignore } } +errorDelay = 0; } catch (IOException x) { if (running) { log.error(sm.getString("endpoint.accept.fail"), x); +if (errorDelay == 0) { +errorDelay = 50; +} else if (errorDelay< 1600) { +errorDelay = errorDelay * 2; +} } } catch (NullPointerException npe) { if (running) { Thoughts / comments? Mark [1] http://pastebin.com/CrsujeW4 - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Reaching ulimit values for open files can generate huge log files
On 22/02/2011 19:34, Filip Hanik - Dev Lists wrote: > Wouldn't you build this into the logging framework, instead of one > specific component? You could, if you can find an efficient way to spot duplicate log messages and then differentiate correctly between when you want the duplicates and when you don't. I'm not sure how easy that would be. Suggestions welcome. The other issue with the current code is that it is likely to chew up a large chunk of CPU time. Fixing the log mechanism won't fix that. Of course, the real solution is fix the ulimit issue in the first place but Tomcat's behaviour in this error condition seems sufficiently bad to justify some changes to handle it more gracefully. Mark > > best > Filip > > On 02/21/2011 08:21 AM, Mark Thomas wrote: >> The ASF Sonar installation managed to generate 46GB of identical log >> messages [1] today in the 8 hours it took to notice it was down. >> >> While better monitoring would/should have identified the problem sooner, >> it does demonstrate a problem with the acceptor threads in all three >> endpoints. If there is a system-level issue that causes the accept() >> call to always fail (such as hitting the ulimit) then the endpoint >> essentially enters a loop where it logs an error message for every >> iteration of the loop. This will result in many log messages per second. >> >> I'd like to do something about this. I was thinking of something along >> the lines of the following for each endpoint. >> >> Index: java/org/apache/tomcat/util/net/JIoEndpoint.java >> === >> --- java/org/apache/tomcat/util/net/JIoEndpoint.java(revision >> 1072939) >> +++ java/org/apache/tomcat/util/net/JIoEndpoint.java(working copy) >> @@ -183,9 +183,19 @@ >> @Override >> public void run() { >> >> +int errorDelay = 0; >> + >> // Loop until we receive a shutdown command >> while (running) { >> >> +if (errorDelay> 0) { >> +try { >> +Thread.sleep(errorDelay); >> +} catch (InterruptedException e) { >> +// Ignore >> +} >> +} >> + >> // Loop if endpoint is paused >> while (paused&& running) { >> try { >> @@ -225,9 +235,15 @@ >> // Ignore >> } >> } >> +errorDelay = 0; >> } catch (IOException x) { >> if (running) { >> >> log.error(sm.getString("endpoint.accept.fail"), x); >> +if (errorDelay == 0) { >> +errorDelay = 50; >> +} else if (errorDelay< 1600) { >> +errorDelay = errorDelay * 2; >> +} >> } >> } catch (NullPointerException npe) { >> if (running) { >> >> >> >> Thoughts / comments? >> >> Mark >> >> >> [1] http://pastebin.com/CrsujeW4 >> >> - >> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org >> For additional commands, e-mail: dev-h...@tomcat.apache.org >> >> >> > > > - > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Connection draining when upload to large
All, On 2/17/2011 5:41 AM, Mark Thomas wrote: > On 17/02/2011 10:30, Rainer Jung wrote: >> On 10.02.2011 18:44, Mark Thomas wrote: >> Any hints about where to add stuff to the docs? Are people fine with >> making it controllable via the request attribute? > > I'm not particularly comfortable with this. I'm having a hard time > coming up with a use case where some requests are swallowed and some are > not. +1 > I think I'd prefer a per Connector or per Context attribute. +1 -chris signature.asc Description: OpenPGP digital signature
DO NOT REPLY [Bug 23406] Jasper should use file size as well as date for recompile check
https://issues.apache.org/bugzilla/show_bug.cgi?id=23406 Mark Thomas changed: What|Removed |Added Status|NEW |RESOLVED Resolution||WONTFIX --- Comment #4 from Mark Thomas 2011-02-22 15:41:04 EST --- I've been reviewing this one again. The bit that confuses me is that the menu_jsp.class file contains the latest version but clearly the version loaded by the class loader is an older version. The timestamp checks that take place are in milliseconds and a quick text shows that there is reasonably fine granularity with these. It does look like a race condition but one that I can't figure out given the current code. There have been changes to Jasper post 4.1.x (increased synchronisation, switch to the Eclipse compiler, improved dependency checking) and I don't recall any similar issues in 5.5.x onwards. Given the above, it isn't very satisfactory but I am going to resolve this as WONTFIX an the assumption that this was a 4.1.x bug that was fixed in 5.5.x onwards by the various changes. -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Reaching ulimit values for open files can generate huge log files
On 02/22/2011 12:59 PM, Mark Thomas wrote: On 22/02/2011 19:34, Filip Hanik - Dev Lists wrote: Wouldn't you build this into the logging framework, instead of one specific component? You could, if you can find an efficient way to spot duplicate log messages and then differentiate correctly between when you want the duplicates and when you don't. I'm not sure how easy that would be. Suggestions welcome. I'd probably wouldn't care so much about duplication as I would care about logging frequency/amount per thread and have ways to adjust that. The other issue with the current code is that it is likely to chew up a large chunk of CPU time. Fixing the log mechanism won't fix that. the maxConnections setting however, would fix this issue, as that forces the system to back off. So, I would have no problem adding a delay for that error. It makes the product more stable, it would have to be added to all end points, as I suspect they all do the same thing. Filip Of course, the real solution is fix the ulimit issue in the first place but Tomcat's behaviour in this error condition seems sufficiently bad to justify some changes to handle it more gracefully. Mark best Filip On 02/21/2011 08:21 AM, Mark Thomas wrote: The ASF Sonar installation managed to generate 46GB of identical log messages [1] today in the 8 hours it took to notice it was down. While better monitoring would/should have identified the problem sooner, it does demonstrate a problem with the acceptor threads in all three endpoints. If there is a system-level issue that causes the accept() call to always fail (such as hitting the ulimit) then the endpoint essentially enters a loop where it logs an error message for every iteration of the loop. This will result in many log messages per second. I'd like to do something about this. I was thinking of something along the lines of the following for each endpoint. Index: java/org/apache/tomcat/util/net/JIoEndpoint.java === --- java/org/apache/tomcat/util/net/JIoEndpoint.java(revision 1072939) +++ java/org/apache/tomcat/util/net/JIoEndpoint.java(working copy) @@ -183,9 +183,19 @@ @Override public void run() { +int errorDelay = 0; + // Loop until we receive a shutdown command while (running) { +if (errorDelay> 0) { +try { +Thread.sleep(errorDelay); +} catch (InterruptedException e) { +// Ignore +} +} + // Loop if endpoint is paused while (paused&& running) { try { @@ -225,9 +235,15 @@ // Ignore } } +errorDelay = 0; } catch (IOException x) { if (running) { log.error(sm.getString("endpoint.accept.fail"), x); +if (errorDelay == 0) { +errorDelay = 50; +} else if (errorDelay< 1600) { +errorDelay = errorDelay * 2; +} } } catch (NullPointerException npe) { if (running) { Thoughts / comments? Mark [1] http://pastebin.com/CrsujeW4 - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Reaching ulimit values for open files can generate huge log files
Mark, On 2/21/2011 10:21 AM, Mark Thomas wrote: > The ASF Sonar installation managed to generate 46GB of identical log > messages [1] today in the 8 hours it took to notice it was down. Ugh. Yeah, that sucks. Hitting the ulimit doesn't necessarily mean disaster and the process could still recover. For instance, an out-of-control request might try to open files in a loop and tie up all the fds for a minute or two before the request dies and the GC cleans everything back up. In the meantime, some request processors might suffer but eventually recover. > While better monitoring would/should have identified the problem sooner, > it does demonstrate a problem with the acceptor threads in all three > endpoints. If there is a system-level issue that causes the accept() > call to always fail (such as hitting the ulimit) then the endpoint > essentially enters a loop where it logs an error message for every > iteration of the loop. This will result in many log messages per second. I like this idea. Does that mean that you'll get once-per-second (or whatever) errors across all processor threads or once-per-second errors for each processor thread? > +if (errorDelay == 0) { > +errorDelay = 50; How about making this initial delay configurable? > +} else if (errorDelay < 1600) { How about making this maximum delay configurable? > +errorDelay = errorDelay * 2; Geometric growth seems reasonable... I don't see a reason to make this multiplier configurable. -chris signature.asc Description: OpenPGP digital signature
Re: Reaching ulimit values for open files can generate huge log files
Mark, On 2/21/2011 10:21 AM, Mark Thomas wrote: > The ASF Sonar installation managed to generate 46GB of identical log > messages [1] today in the 8 hours it took to notice it was down. I've not really looked at how Tomcat does its logging, but do we have an opportunity to do what, say, syslogd does when it emits messages like "Last message repeated X times"? If the logger could keep a reference to the last log message and do comparisons, this would be possible. On the other hand, reducing the performance of logging isn't a popular thing to do. -chris signature.asc Description: OpenPGP digital signature
DO NOT REPLY [Bug 23745] "jsp.error.unterminated.tag" received when doing struts tags - improve error messages for misbuilt tags!
https://issues.apache.org/bugzilla/show_bug.cgi?id=23745 Mark Thomas changed: What|Removed |Added Status|REOPENED|RESOLVED Resolution||FIXED --- Comment #11 from Mark Thomas 2011-02-22 16:30:46 EST --- This is fixed in later versions. The name of the tag as well as the location in the file are included in the error message. -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 23841] More specific error message to "org.apache.jasper.JasperException: Exception thrown by getter for property ..."
https://issues.apache.org/bugzilla/show_bug.cgi?id=23841 Mark Thomas changed: What|Removed |Added Status|NEW |RESOLVED Resolution||FIXED --- Comment #1 from Mark Thomas 2011-02-22 16:40:19 EST --- This has been fixed in later versions. The root cause is correctly displayed. -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 24650] name class of bean when throwing java.lang.NoSuchMethodException
https://issues.apache.org/bugzilla/show_bug.cgi?id=24650 Mark Thomas changed: What|Removed |Added Status|NEW |RESOLVED Resolution||FIXED --- Comment #3 from Mark Thomas 2011-02-22 16:51:41 EST --- This has been fixed in later versions. A class cast exception is reported with both class names. -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 24772] say where in the .jsp file it happens: "Cannot find bean xyz in scope null"
https://issues.apache.org/bugzilla/show_bug.cgi?id=24772 Mark Thomas changed: What|Removed |Added Status|NEW |RESOLVED Resolution||FIXED --- Comment #1 from Mark Thomas 2011-02-22 16:58:02 EST --- As per JSP.5.3 (JSP 2.2 spec) Tomcat follows the recommendation that a translation time error is raised if the bean has not been introduced. This includes the location in the JSP file and the name of the bean. -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 24773] say which scope is null in "Cannot find bean xyz in scope null" request/session/application/... ?
https://issues.apache.org/bugzilla/show_bug.cgi?id=24773 Mark Thomas changed: What|Removed |Added Status|NEW |RESOLVED Resolution||DUPLICATE --- Comment #1 from Mark Thomas 2011-02-22 16:58:57 EST --- The fix for 24772 also fixes this issue. *** This bug has been marked as a duplicate of bug 24772 *** -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 24772] say where in the .jsp file it happens: "Cannot find bean xyz in scope null"
https://issues.apache.org/bugzilla/show_bug.cgi?id=24772 --- Comment #2 from Mark Thomas 2011-02-22 16:58:57 EST --- *** Bug 24773 has been marked as a duplicate of this bug. *** -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
svn commit: r1073531 - in /tomcat/trunk/modules/jdbc-pool: java/org/apache/tomcat/jdbc/pool/ConnectionPool.java test/org/apache/tomcat/jdbc/test/Bug50805.java
Author: fhanik Date: Tue Feb 22 22:42:44 2011 New Revision: 1073531 URL: http://svn.apache.org/viewvc?rev=1073531&view=rev Log: https://issues.apache.org/bugzilla/show_bug.cgi?id=50805 Make sure we only call borrowConnection once per connection per checkout Added: tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/Bug50805.java (with props) Modified: tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java Modified: tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java URL: http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java?rev=1073531&r1=1073530&r2=1073531&view=diff == --- tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java (original) +++ tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java Tue Feb 22 22:42:44 2011 @@ -142,6 +142,7 @@ public class ConnectionPool { public Future getConnectionAsync() throws SQLException { PooledConnection pc = this.borrowConnection(0, null, null); if (pc!=null) { + return new ConnectionFuture(pc); } //we can only retrieve a future if the underlying queue supports it. @@ -1054,8 +1055,10 @@ public class ConnectionPool { this.pcFuture = pcf; } -public ConnectionFuture(PooledConnection pc) { +public ConnectionFuture(PooledConnection pc) throws SQLException { this.pc = pc; +result = ConnectionPool.this.setupConnection(pc); +configured.set(true); } /** * {@inheritDoc} Added: tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/Bug50805.java URL: http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/Bug50805.java?rev=1073531&view=auto == --- tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/Bug50805.java (added) +++ tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/Bug50805.java Tue Feb 22 22:42:44 2011 @@ -0,0 +1,38 @@ +package org.apache.tomcat.jdbc.test; + +import java.sql.Connection; +import java.util.concurrent.Future; + +public class Bug50805 extends DefaultTestCase { +public Bug50805(String name) { +super(name); +} + +public void test50805() throws Exception { +init(); +this.datasource.setInitialSize(0); +this.datasource.setMaxActive(10); +this.datasource.setMinIdle(1); + +assertEquals("Current size should be 0.", 0, this.datasource.getSize()); + +this.datasource.getConnection().close(); + +assertEquals("Current size should be 1.", 1, this.datasource.getSize()); +assertEquals("Idle size should be 1.", 1, this.datasource.getIdle()); +assertEquals("Busy size should be 0.", 0, this.datasource.getActive()); + +Future fc = this.datasource.getConnectionAsync(); + +Connection con = fc.get(); + +assertEquals("Current size should be 1.", 1, this.datasource.getSize()); +assertEquals("Idle size should be 0.", 0, this.datasource.getIdle()); +assertEquals("Busy size should be 1.", 1, this.datasource.getActive()); + +con.close(); +assertEquals("Current size should be 1.", 1, this.datasource.getSize()); +assertEquals("Idle size should be 1.", 1, this.datasource.getIdle()); +assertEquals("Busy size should be 0.", 0, this.datasource.getActive()); +} +} Propchange: tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/Bug50805.java -- svn:eol-style = native - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
svn commit: r1073532 - /tomcat/trunk/modules/jdbc-pool/doc/changelog.xml
Author: fhanik Date: Tue Feb 22 22:45:43 2011 New Revision: 1073532 URL: http://svn.apache.org/viewvc?rev=1073532&view=rev Log: doco update bug 50805 Modified: tomcat/trunk/modules/jdbc-pool/doc/changelog.xml Modified: tomcat/trunk/modules/jdbc-pool/doc/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/doc/changelog.xml?rev=1073532&r1=1073531&r2=1073532&view=diff == --- tomcat/trunk/modules/jdbc-pool/doc/changelog.xml (original) +++ tomcat/trunk/modules/jdbc-pool/doc/changelog.xml Tue Feb 22 22:45:43 2011 @@ -28,6 +28,14 @@ + + + + 1073531 50805 Only initialize connections once when async (fhanik) + + + + - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 50805] Leak & Performance issue of getConnectionAsync()
https://issues.apache.org/bugzilla/show_bug.cgi?id=50805 Filip Hanik changed: What|Removed |Added Status|NEEDINFO|RESOLVED Resolution||FIXED --- Comment #3 from Filip Hanik 2011-02-22 17:46:07 EST --- Fixed in r1073531 Thank you for the report! -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 50805] Leak & Performance issue of getConnectionAsync()
https://issues.apache.org/bugzilla/show_bug.cgi?id=50805 --- Comment #4 from Eiji Takahashi 2011-02-22 21:07:29 EST --- Sorry for not replying sooner. The performance issue was solved by the latest revision. Many thanks! -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org