You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by cr...@apache.org on 2001/04/11 03:46:10 UTC
cvs commit: jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/realm GenericPrincipal.java JDBCRealm.java LocalStrings.properties MemoryRealm.java RealmBase.java
craigmcc 01/04/10 18:46:10
Modified: catalina/src/share/org/apache/catalina/realm
GenericPrincipal.java JDBCRealm.java
LocalStrings.properties MemoryRealm.java
RealmBase.java
Log:
General refactoring and cleanup of the org.apache.catalina.realm package:
* Migrated digest processing into the base class so that it can be
used by MemoryRealm as well.
* Migrated standard hasRole() into the base class so that Realm
implementations using GenericPrincipal do not need to do anything.
* Modified JDBCRealm to use GenericPrincipal.
* Clean up code in JDBCRealm for opening and closing the database
connection, reporting errors in start() and stop(), and creating
prepared statements. You can now subclass and specialize if needed.
* Add a "commit()" call inside authenticate() in case the underlying
database does not like a long-running transaction.
* Cosmetic cleanup to code and Javadocs to be consistent with the
rest of Catalina.
There should be no functionality change, except for the new ability to use
digest-encoded passwords in MemoryRealm.
Updates to the config documentation will be done later tonight.
Revision Changes Path
1.2 +8 -9 jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/realm/GenericPrincipal.java
Index: GenericPrincipal.java
===================================================================
RCS file: /home/cvs/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/realm/GenericPrincipal.java,v
retrieving revision 1.1
retrieving revision 1.2
diff -u -r1.1 -r1.2
--- GenericPrincipal.java 2001/04/01 02:29:46 1.1
+++ GenericPrincipal.java 2001/04/11 01:46:09 1.2
@@ -1,7 +1,7 @@
/*
- * $Header: /home/cvs/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/realm/GenericPrincipal.java,v 1.1 2001/04/01 02:29:46 craigmcc Exp $
- * $Revision: 1.1 $
- * $Date: 2001/04/01 02:29:46 $
+ * $Header: /home/cvs/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/realm/GenericPrincipal.java,v 1.2 2001/04/11 01:46:09 craigmcc Exp $
+ * $Revision: 1.2 $
+ * $Date: 2001/04/11 01:46:09 $
*
* ====================================================================
*
@@ -66,6 +66,7 @@
import java.security.Principal;
+import java.util.Arrays;
import java.util.List;
import org.apache.catalina.Realm;
@@ -77,7 +78,7 @@
* private to avoid interference from applications.
*
* @author Craig R. McClanahan
- * @version $Revision: 1.1 $ $Date: 2001/04/01 02:29:46 $
+ * @version $Revision: 1.2 $ $Date: 2001/04/11 01:46:09 $
*/
class GenericPrincipal implements Principal {
@@ -120,6 +121,8 @@
if (roles != null) {
this.roles = new String[roles.size()];
this.roles = (String[]) roles.toArray(this.roles);
+ if (this.roles.length > 0)
+ Arrays.sort(this.roles);
}
}
@@ -198,11 +201,7 @@
if (role == null)
return (false);
- for (int i = 0; i < roles.length; i++) {
- if (role.equals(roles[i]))
- return (true);
- }
- return (false);
+ return (Arrays.binarySearch(roles, role) >= 0);
}
1.12 +257 -406 jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/realm/JDBCRealm.java
Index: JDBCRealm.java
===================================================================
RCS file: /home/cvs/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/realm/JDBCRealm.java,v
retrieving revision 1.11
retrieving revision 1.12
diff -u -r1.11 -r1.12
--- JDBCRealm.java 2001/04/10 09:06:49 1.11
+++ JDBCRealm.java 2001/04/11 01:46:09 1.12
@@ -59,10 +59,15 @@
package org.apache.catalina.realm;
-import java.beans.PropertyChangeListener;
-import java.beans.PropertyChangeSupport;
-import java.security.Principal;
import java.io.File;
+import java.security.Principal;
+import java.sql.Connection;
+import java.sql.Driver;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Properties;
import org.apache.catalina.Container;
import org.apache.catalina.Lifecycle;
import org.apache.catalina.LifecycleEvent;
@@ -72,17 +77,9 @@
import org.apache.catalina.Realm;
import org.apache.catalina.util.LifecycleSupport;
import org.apache.catalina.util.StringManager;
-//import org.apache.catalina.util.xml.SaxContext;
-//import org.apache.catalina.util.xml.XmlAction;
-//import org.apache.catalina.util.xml.XmlMapper;
-import org.xml.sax.AttributeList;
import org.apache.catalina.util.Base64;
-import org.apache.catalina.util.HexUtils;
-import java.security.*;
-import java.sql.*;
-
/**
*
* Implmentation of <b>Realm</b> that works with any JDBC supported database.
@@ -90,11 +87,13 @@
* for configuration options.
*
* TODO:
- * - Work on authentication with non-plaintext passwords
* - Make sure no bad chars can get in and trick the auth and hasrole
+ * - Use a database connection pool for faster simultaneous access
*
* @author Craig R. McClanahan
* @author Carson McDonald
+ * @author Ignacio Ortega
+ * @version $Revision: 1.12 $ $Date: 2001/04/11 01:46:09 $
*/
public class JDBCRealm
@@ -104,113 +103,126 @@
// ----------------------------------------------------- Instance Variables
+ /**
+ * The connection username to use when trying to connect to the database.
+ */
+ protected String connectionName = null;
+
+
/**
- * The connection URL to use when trying to connect to the databse
+ * The connection URL to use when trying to connect to the database.
*/
- private String connectionURL = null;
+ protected String connectionPassword = null;
/**
+ * The connection URL to use when trying to connect to the database.
+ */
+ protected String connectionURL = null;
+
+
+ /**
* The connection to the database.
+ */
+ protected Connection dbConnection = null;
+
+
+ /**
+ * Instance of the JDBC Driver class we use as a connection factory.
*/
- private Connection dbConnection = null;
+ protected Driver driver = null;
/**
* The JDBC driver to use.
*/
- private String driverName = null;
+ protected String driverName = null;
/**
* Descriptive information about this Realm implementation.
*/
+ protected static final String info =
+ "org.apache.catalina.realm.JDBCRealm/1.0";
- protected static final String info = "org.apache.catalina.realm.JDBCRealm/1.0";
/**
* Descriptive information about this Realm implementation.
*/
+ protected static final String name = "JDBCRealm";
- private static final String name = "JDBCRealm";
-
/**
* The PreparedStatement to use for authenticating users.
*/
- private PreparedStatement preparedAuthenticate = null;
+ protected PreparedStatement preparedCredentials = null;
/**
* The PreparedStatement to use for identifying the roles for
* a specified user.
*/
- private PreparedStatement preparedRoles = null;
+ protected PreparedStatement preparedRoles = null;
/**
* The column in the user role table that names a role
*/
- private String roleNameCol = null;
+ protected String roleNameCol = null;
/**
* The string manager for this package.
*/
- private static final StringManager sm =
+ protected static final StringManager sm =
StringManager.getManager(Constants.Package);
/**
- * Has this component been started?
- */
- private boolean started = false;
-
-
- /**
* The column in the user table that holds the user's credintials
*/
- private String userCredCol = null;
+ protected String userCredCol = null;
/**
* The column in the user table that holds the user's name
*/
- private String userNameCol = null;
+ protected String userNameCol = null;
/**
* The table that holds the relation between user's and roles
*/
- private String userRoleTable = null;
+ protected String userRoleTable = null;
/**
* The table that holds user data.
*/
- private String userTable = null;
+ protected String userTable = null;
- /**
- * The connection URL to use when trying to connect to the databse
- */
- private String connectionName = null;
- /**
- * The connection URL to use when trying to connect to the databse
- */
- private String connectionPassword = null;
+ // ------------------------------------------------------------- Properties
- /**
- *
- * Digest algorithm used in passwords thit is same values
- * accepted by MessageDigest for algorithm
- * plus "No" ( no encode ) that is the default
+
+ /**
+ * Set the username to use to connect to the database.
*
+ * @param connectionName Username
*/
+ public void setConnectionName(String connectionName) {
+ this.connectionName = connectionName;
+ }
- private String digest="";
- // ------------------------------------------------------------- Properties
+ /**
+ * Set the password to use to connect to the database.
+ *
+ * @param connectionPassword User password
+ */
+ public void setConnectionPassword(String connectionPassword) {
+ this.connectionPassword = connectionPassword;
+ }
/**
@@ -234,9 +246,9 @@
/**
- * Set the column in the user role table that names a role
+ * Set the column in the user role table that names a role.
*
- * @param userRoleNameCol The column name
+ * @param roleNameCol The column name
*/
public void setRoleNameCol( String roleNameCol ) {
this.roleNameCol = roleNameCol;
@@ -244,7 +256,7 @@
/**
- * Set the column in the user table that holds the user's credintials
+ * Set the column in the user table that holds the user's credentials.
*
* @param userCredCol The column name
*/
@@ -254,7 +266,7 @@
/**
- * Set the column in the user table that holds the user's name
+ * Set the column in the user table that holds the user's name.
*
* @param userNameCol The column name
*/
@@ -264,7 +276,7 @@
/**
- * Set the table that holds the relation between user's and roles
+ * Set the table that holds the relation between user's and roles.
*
* @param userRoleTable The table name
*/
@@ -282,85 +294,22 @@
this.userTable = userTable;
}
- /**
- * Set the name to use to connect to the database.
- *
- * @param connectionName User name
- */
- public void setConnectionName(String connectionName) {
- this.connectionName = connectionName;
- }
-
- /**
- * Set the password to use to connect to the database.
- *
- * @param connectionPassword User password
- */
- public void setConnectionPassword(String connectionPassword) {
- this.connectionPassword = connectionPassword;
- }
-
-
- /**
- * Gets the digest algorithm used for credentials in the database
- * could be the same that MessageDigest accepts vor algorithm
- * and "No" that is the Default
- *
- */
-
- public String getDigest() {
- return digest;
- }
-
- /**
- * Gets the digest algorithm used for credentials in the database
- * could be the same that MessageDigest accepts vor algorithm
- * and "No" that is the Default
- *
- * @param algorithm the Encode type
- */
-
- public void setDigest(String algorithm) {
- digest = algorithm;
- }
-
-
- /**
- * Return descriptive information about this Realm implementation and
- * the corresponding version number, in the format
- * <code><description>/<version></code>.
- */
- public String getInfo() {
-
- return this.info;
-
- }
-
- /**
- * Return short name of this Realm implementation
- */
- public String getName() {
-
- return name;
- }
-
// --------------------------------------------------------- Public Methods
/**
- *
* Return the Principal associated with the specified username and
* credentials, if there is one; otherwise return <code>null</code>.
*
* If there are any errors with the JDBC connection, executing
* the query or anything we return null (don't authenticate). This
- * event is also logged.
+ * event is also logged, and the connection will be closed so that
+ * a subsequent request will automatically re-open it.
*
- * If there is some SQL exception the connection is set to null.
- * This will allow a retry on the next auth attempt. This might not
- * be the best thing to do but it will keep Catalina from needing a
- * restart if the database goes down.
+ * <strong>IMPLEMENTATION NOTE</strong> - This method is synchronized
+ * because we are sharing a single connection (and its associated
+ * prepared statements) across all request threads.
*
* @param username Username of the Principal to look up
* @param credentials Password or other credentials to use in
@@ -370,97 +319,53 @@
String credentials) {
try {
+
+ // Ensure that our database connection is open
+ open();
- if (!checkConnection()) return null;
- // Create the authentication search prepared statement if necessary
- if (preparedAuthenticate == null) {
- String sql = "SELECT " + userCredCol + " FROM " + userTable +
- " WHERE " + userNameCol + " = ?";
- if (debug >= 1)
- log("JDBCRealm.authenticate: " + sql);
- preparedAuthenticate = dbConnection.prepareStatement(sql);
- }
-
- // Create the roles search prepared statement if necessary
- if (preparedRoles == null) {
- String sql = "SELECT " + roleNameCol + " FROM " +
- userRoleTable + " WHERE " + userNameCol + " = ?";
- if (debug >= 1)
- log("JDBCRealm.roles: " + sql);
- preparedRoles = dbConnection.prepareStatement(sql);
- }
-
- // Perform the authentication search
- preparedAuthenticate.setString(1, username);
- ResultSet rs1 = preparedAuthenticate.executeQuery();
- boolean found = false;
- if (rs1.next()) {
- String dbCredentials=rs1.getString(1).trim();
- if( digest.equals("") || digest.equalsIgnoreCase("No")){
- if(credentials.equals(dbCredentials)) {
- if(debug >= 2)
- log(sm.getString("jdbcRealm.authenticateSuccess",
- username));
- found = true;
- }
- } else{
- if (Digest(credentials,digest).equals(dbCredentials)) {
- if (debug >= 2)
- log(sm.getString("jdbcRealm.authenticateSuccess",
+ // Look up the user's credentials
+ String dbCredentials = null;
+ PreparedStatement stmt = credentials(username);
+ ResultSet rs = stmt.executeQuery();
+ while (rs.next()) {
+ dbCredentials = rs.getString(1).trim();
+ }
+ rs.close();
+ if (dbCredentials == null)
+ return (null);
+
+ // Validate the user's credentials
+ if (digest(credentials).equals(dbCredentials)) {
+ if (debug >= 2)
+ log(sm.getString("jdbcRealm.authenticateSuccess",
+ username));
+ } else {
+ if (debug >= 2)
+ log(sm.getString("jdbcRealm.authenticateFailure",
username));
- found = true;
- }
- }
- }
- rs1.close();
- if (!found) {
- if (debug >= 2)
- log(sm.getString("jdbcRealm.authenticateFailure",
- username));
- return (null);
- }
-
- // Prepare and return a suitable Principal to be returned
- JDBCRealmPrincipal principal =
- new JDBCRealmPrincipal(username, credentials);
- preparedRoles.setString(1, username);
- ResultSet rs2 = preparedRoles.executeQuery();
- while (rs2.next()) {
- principal.addRole(rs2.getString(1).trim());
- }
- rs2.close();
- return (principal);
+ return (null);
+ }
- } catch( SQLException ex ) {
+ // Accumulate the user's roles
+ ArrayList list = new ArrayList();
+ stmt = roles(username);
+ rs = stmt.executeQuery();
+ while (rs.next()) {
+ list.add(rs.getString(1).trim());
+ }
+ rs.close();
+ dbConnection.commit();
+
+ // Create and return a suitable Principal for this user
+ return (new GenericPrincipal(this, username, credentials, list));
+
+ } catch (SQLException e) {
// Log the problem for posterity
- log("JDBCRealm.authenticate", ex);
+ log(sm.getString("jdbcRealm.exception"), e);
- // Clean up the JDBC objects so that they get recreated next time
- if (preparedRoles != null) {
- try {
- preparedRoles.close();
- } catch (Throwable t) {
- ;
- }
- preparedRoles = null;
- }
- if (preparedAuthenticate != null) {
- try {
- preparedAuthenticate.close();
- } catch (Throwable t) {
- ;
- }
- preparedAuthenticate = null;
- }
- if (dbConnection != null) {
- try {
- dbConnection.close();
- } catch (Throwable t) {
- ;
- }
- dbConnection = null;
- }
+ // Close the connection so that it gets reopened next time
+ close();
// Return "not authenticated" for this request
return (null);
@@ -470,268 +375,214 @@
}
+ // -------------------------------------------------------- Package Methods
+
+
+ // ------------------------------------------------------ Protected Methods
+
+
/**
+ * Close any database connection that is currently open.
+ */
+ protected void close() {
+
+ // Do nothing if the database connection is already closed
+ if (dbConnection == null)
+ return;
+
+ // Close our prepared statements (if any)
+ try {
+ preparedCredentials.close();
+ } catch (Throwable f) {
+ ;
+ }
+ try {
+ preparedRoles.close();
+ } catch (Throwable f) {
+ ;
+ }
+
+ // Close this database connection, and log any errors
+ try {
+ dbConnection.close();
+ } catch (SQLException e) {
+ log(sm.getString("jdbcRealm.close"), e); // Just log it here
+ }
+
+ // Release resources associated with the closed connection
+ dbConnection = null;
+ preparedCredentials = null;
+ preparedRoles = null;
+
+ }
+
+
+ /**
+ * Return a PreparedStatement configured to perform the SELECT required
+ * to retrieve user credentials for the specified username.
+ *
+ * @param username Username for which credentials should be retrieved
*
- * Return <code>true</code> if the specified Principal has the specified
- * security role, within the context of this Realm; otherwise return
- * <code>false</code>.
- *
- * If there are any errors with the JDBC connection, executing
- * the query or anything we return false (not in role set). This
- * event is also logged.
- *
- * If there is some SQL exception the connection is set to null.
- * This will allow a retry on the next auth attempt. This might not
- * be the best thing to do but it will keep Catalina from needing a
- * restart if the database goes down.
- *
- * @param principal Principal for whom the role is to be checked
- * @param role Security role to be checked
- */
- public boolean hasRole(Principal principal, String role) {
- String username = principal.getName();
-
- // Is the specified Principal one that we created?
- if (!(principal instanceof JDBCRealmPrincipal))
- return (false);
+ * @exception SQLException if a database error occurs
+ */
+ protected PreparedStatement credentials(String username)
+ throws SQLException {
- // Ask this Principal for the answer
- return (((JDBCRealmPrincipal) principal).hasRole(role));
+ if (preparedCredentials == null) {
+ StringBuffer sb = new StringBuffer("SELECT ");
+ sb.append(userCredCol);
+ sb.append(" FROM ");
+ sb.append(userTable);
+ sb.append(" WHERE ");
+ sb.append(userNameCol);
+ sb.append(" = ?");
+ preparedCredentials =
+ dbConnection.prepareStatement(sb.toString());
+ }
+
+ preparedCredentials.setString(1, username);
+ return (preparedCredentials);
}
- // -------------------------------------------------------- Package Methods
+ /**
+ * Return a short name for this Realm implementation.
+ */
+ protected String getName() {
+ return (this.name);
- // ------------------------------------------------------ Protected Methods
+ }
+
/**
* Return the password associated with the given principal's user name.
*/
protected String getPassword(String username) {
+
return (null);
+
}
+
/**
* Return the Principal associated with the given user name.
*/
protected Principal getPrincipal(String username) {
+
return (null);
+
}
/**
- *
- * Prepare for active use of the public methods of this Component.
- *
- * The DriverManager is initiated here. The initial database connection
- * is also formed.
+ * Open a database connection for use by this Realm.
*
- * @exception IllegalStateException if this component has already been
- * started
- * @exception LifecycleException if this component detects a fatal error
- * that prevents it from being started
+ * @exception SQLException if a database error occurs
*/
- public synchronized void start() throws LifecycleException {
- // Validate and update our current component state
- if (started) {
- throw new LifecycleException (sm.getString("jdbcRealm.alreadyStarted"));
- }
- lifecycle.fireLifecycleEvent(START_EVENT, null);
- started = true;
- if(!checkConnection()) {
- throw new LifecycleException (sm.getString("jdbcRealm.alreadyStarted"));
+ protected void open() throws SQLException {
+
+ // Do nothing if there is a database connection already open
+ if (dbConnection != null)
+ return;
+
+ // Instantiate our database driver if necessary
+ if (driver == null) {
+ try {
+ Class clazz = Class.forName(driverName);
+ driver = (Driver) clazz.newInstance();
+ } catch (Throwable e) {
+ throw new SQLException(e.getMessage());
+ }
}
+ // Open a new connection
+ Properties props = new Properties();
+ if (connectionName != null)
+ props.put("user", connectionName);
+ if (connectionPassword != null)
+ props.put("password", connectionPassword);
+ dbConnection = driver.connect(connectionURL, props);
+
}
/**
- *
- * Gracefully shut down active use of the public methods of this Component.
+ * Return a PreparedStatement configured to perform the SELECT required
+ * to retrieve user roles for the specified username.
*
- * If there is a connection it is closed.
+ * @param username Username for which roles should be retrieved
*
- * @exception IllegalStateException if this component has not been started
- * @exception LifecycleException if this component detects a fatal error
- * that needs to be reported
+ * @exception SQLException if a database error occurs
*/
- public synchronized void stop() throws LifecycleException {
- // Validate and update our current component state
- if (!started) {
- throw new LifecycleException (sm.getString("jdbcRealm.notStarted"));
- }
- lifecycle.fireLifecycleEvent(STOP_EVENT, null);
- started = false;
+ protected PreparedStatement roles(String username)
+ throws SQLException {
- // Close any open DB connection
- if( dbConnection != null ) {
- try {
- dbConnection.close();
- }
- catch( SQLException ex ) {
- // XXX: Don't know if this is the best thing to do. Maybe just ignore.
- throw new LifecycleException (sm.getString("jdbcRealm.notStarted"));
- }
+ if (preparedRoles == null) {
+ StringBuffer sb = new StringBuffer("SELECT ");
+ sb.append(roleNameCol);
+ sb.append(" FROM ");
+ sb.append(userRoleTable);
+ sb.append(" WHERE ");
+ sb.append(userNameCol);
+ sb.append(" = ?");
+ preparedRoles =
+ dbConnection.prepareStatement(sb.toString());
}
- }
- /**
- * Digest password using the algorithm especificied and
- * convert the result to a corresponding hex string.
- * If exception, the plain credentials string is returned
- *
- * @param credentials Password or other credentials to use in
- * authenticating this username
- *
- * @param algorithm Algorithm used to do th digest
- *
- */
- final public static String Digest(String credentials,String algorithm) {
- try {
- // Obtain a new message digest with "digest" encryption
- MessageDigest md = (MessageDigest)MessageDigest.getInstance(algorithm).clone();
- // encode the credentials
- md.update( credentials.getBytes() );
- // obtain the byte array from the digest
- byte[] dig = md.digest();
- // convert the byte array to hex string
- return HexUtils.convert(dig);
-
- } catch( Exception ex ) {
- ex.printStackTrace();
- return credentials;
- }
- }
+ preparedRoles.setString(1, username);
+ return (preparedRoles);
- private boolean checkConnection(){
- try {
- if( (dbConnection == null) || dbConnection.isClosed() ) {
- Class.forName(driverName);
- log(sm.getString("jdbcRealm.checkConnectionDBClosed"));
- if ((connectionName == null || connectionName.equals("")) ||
- (connectionPassword == null || connectionPassword.equals(""))) {
- dbConnection = DriverManager.getConnection(connectionURL);
- } else {
- dbConnection = DriverManager.getConnection(connectionURL,
- connectionName,
- connectionPassword);
- }
- if( dbConnection == null || dbConnection.isClosed() ) {
- log(sm.getString("jdbcRealm.checkConnectionDBReOpenFail"));
- return false;
- }
- }
- return true;
- }catch (SQLException ex){
- log(sm.getString("jdbcRealm.checkConnectionSQLException"),ex);
- return false;
- }
- catch( ClassNotFoundException ex ) {
- log(sm.getString("jdbcRealm.checkConnectionClassNotFoundException"),ex);
- return false;
- }
}
- public static void main(String args[] ) {
- if (args.length >= 2) {
- if( args[0].equalsIgnoreCase("-a")){
- for( int i=2; i < args.length ; i++){
- System.out.print(args[i]+":");
- System.out.println(Digest(args[i],args[1]));
- }
- }
- }
-
- }
-}
+ // -------------------------------------------------------- Private Methods
-/**
- * Private class representing an individual user's Principal object.
- */
-final class JDBCRealmPrincipal implements Principal {
+ // ------------------------------------------------------ Lifecycle Methods
/**
- * The password for this Principal.
- */
- private String password = null;
-
-
- /**
- * The role names possessed by this Principal.
+ *
+ * Prepare for active use of the public methods of this Component.
+ *
+ * @exception IllegalStateException if this component has already been
+ * started
+ * @exception LifecycleException if this component detects a fatal error
+ * that prevents it from being started
*/
- private String roles[] = new String[0];
-
+ public void start() throws LifecycleException {
- /**
- * The username for this Principal.
- */
- private String username = null;
+ // Validate that we can open our connection
+ try {
+ open();
+ } catch (SQLException e) {
+ throw new LifecycleException(sm.getString("jdbcRealm.open"), e);
+ }
+ // Perform normal superclass initialization
+ super.start();
- /**
- * Construct a new JDBCRealmPrincipal instance.
- *
- * @param username The username for this Principal
- * @param password The password for this Principal
- */
- public JDBCRealmPrincipal(String username, String password) {
- this.username = username;
- this.password = password;
}
/**
- * Add a new role assigned to this Principal.
- * @param role The new role to be assigned
+ * Gracefully shut down active use of the public methods of this Component.
+ *
+ * @exception IllegalStateException if this component has not been started
+ * @exception LifecycleException if this component detects a fatal error
+ * that needs to be reported
*/
- void addRole(String role) {
- if (role == null)
- return;
- for (int i = 0; i < roles.length; i++) {
- if (role.equals(roles[i]))
- return;
- }
- String results[] = new String[roles.length + 1];
- for (int i = 0; i < roles.length; i++)
- results[i] = roles[i];
- results[roles.length] = role;
- roles = results;
- }
+ public void stop() throws LifecycleException {
+ // Perform normal superclass finalization
+ super.stop();
- /**
- * Return the name of this Principal.
- */
- public String getName() {
- return (username);
- }
-
+ // Close any open DB connection
+ close();
- /**
- * Return the password of this Principal.
- */
- public String getPassword() {
- return (password);
}
-
- /**
- * Does this user have the specified role assigned?
- * @param role The role to be checked
- */
- boolean hasRole(String role) {
- if (role == null)
- return (false);
- for (int i = 0; i < roles.length; i++) {
- if (role.equals(roles[i]))
- return (true);
- }
- return (false);
- }
}
1.3 +17 -23 jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/realm/LocalStrings.properties
Index: LocalStrings.properties
===================================================================
RCS file: /home/cvs/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/realm/LocalStrings.properties,v
retrieving revision 1.2
retrieving revision 1.3
diff -u -r1.2 -r1.3
--- LocalStrings.properties 2000/12/31 16:30:35 1.2
+++ LocalStrings.properties 2001/04/11 01:46:09 1.3
@@ -1,28 +1,22 @@
-# $Id: LocalStrings.properties,v 1.2 2000/12/31 16:30:35 nacho Exp $
+# $Id: LocalStrings.properties,v 1.3 2001/04/11 01:46:09 craigmcc Exp $
# language
# package org.apache.catalina.realm
-
-memoryRealm.alreadyStarted=This Realm has already been started
-memoryRealm.authenticateFailure=Authentication unsuccessful for user {0}
-memoryRealm.authenticateSuccess=Authentication successful for user {0}
-memoryRealm.hasRoleFailure=User {0} does NOT have role {1}
-memoryRealm.hasRoleNone=No user has role {0}
-memoryRealm.hasRoleSuccess=User {0} has role {1}
-memoryRealm.hasRoleUser=User {0} is not in this realm database
-memoryRealm.loadExist=Memory realm file {0} does not exist
-memoryRealm.loadPath=Loading memory realm file {0}
-memoryRealm.notStarted=This Realm has not yet been started
-jdbcRealm.alreadyStarted=This Realm has already been started
-jdbcRealm.authenticateFailure=Authentication unsuccessful for user {0}
-jdbcRealm.authenticateSuccess=Authentication successful for user {0}
-jdbcRealm.authenticateSQLException=There was an SQLException while in authenticate: {0}
-jdbcRealm.hasRoleFailure=User {0} does NOT have role {1}
-jdbcRealm.hasRoleSuccess=User {0} has role {1}
-jdbcRealm.hasRoleSQLException=There was an SQLException while in hasRole: {0}
-jdbcRealm.notStarted=This Realm has not yet been started
-jdbcRealm.checkConnectionDBClosed=The database connection is null or was found to be closed. Trying to re-open it.
-jdbcRealm.checkConnectionDBReOpenFail=The re-open on the database failed. The database could be down.
-jdbcRealm.checkConnectionClassNotFoundException=JDBC driver class not found {0}
+jdbcRealm.authenticateFailure=Username {0} NOT successfully authenticated
+jdbcRealm.authenticateSuccess=Username {0} successfully authenticated
+jdbcRealm.close=Exception closing database connection
+jdbcRealm.exception=Exception performing authentication
+jdbcRealm.open=Exception opening database connection
+memoryRealm.authenticateFailure=Username {0} NOT successfully authenticated
+memoryRealm.authenticateSuccess=Username {0} successfully authenticated
+memoryRealm.loadExist=Memory database file {0} cannot be read
+memoryRealm.loadPath=Loading users from memory database file {0}
+memoryRealm.readXml=Exception while reading memory database file
+realmBase.algorithm=Invalid message digest algorithm {0} specified
+realmBase.alreadyStarted=This Realm has already been started
+realmBase.digest=Error digesting user credentials
+realmBase.hasRoleFailure=Username {0} does NOT have role {1}
+realmBase.hasRoleSuccess=Username {0} has role {1}
+realmBase.notStarted=This Realm has not yet been started
1.4 +49 -60 jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/realm/MemoryRealm.java
Index: MemoryRealm.java
===================================================================
RCS file: /home/cvs/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/realm/MemoryRealm.java,v
retrieving revision 1.3
retrieving revision 1.4
diff -u -r1.3 -r1.4
--- MemoryRealm.java 2001/04/01 02:29:46 1.3
+++ MemoryRealm.java 2001/04/11 01:46:09 1.4
@@ -1,7 +1,7 @@
/*
- * $Header: /home/cvs/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/realm/MemoryRealm.java,v 1.3 2001/04/01 02:29:46 craigmcc Exp $
- * $Revision: 1.3 $
- * $Date: 2001/04/01 02:29:46 $
+ * $Header: /home/cvs/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/realm/MemoryRealm.java,v 1.4 2001/04/11 01:46:09 craigmcc Exp $
+ * $Revision: 1.4 $
+ * $Date: 2001/04/11 01:46:09 $
*
* ====================================================================
*
@@ -65,8 +65,6 @@
package org.apache.catalina.realm;
-import java.beans.PropertyChangeListener;
-import java.beans.PropertyChangeSupport;
import java.security.Principal;
import java.io.File;
import java.util.ArrayList;
@@ -97,7 +95,7 @@
* synchronization is performed around accesses to the principals collection.
*
* @author Craig R. McClanahan
- * @version $Revision: 1.3 $ $Date: 2001/04/01 02:29:46 $
+ * @version $Revision: 1.4 $ $Date: 2001/04/11 01:46:09 $
*/
public final class MemoryRealm
@@ -167,17 +165,30 @@
}
+
/**
- * Return short name of this Realm implementation
+ * Return the pathname of our XML file containing user definitions.
*/
- public String getName() {
+ public String getPathname() {
- return name;
+ return pathname;
}
+ /**
+ * Set the pathname of our XML file containing user definitions. If a
+ * relative pathname is specified, it is resolved against "catalina.home".
+ *
+ * @param pathname The new pathname
+ */
+ public void setPathname(String pathname) {
+
+ this.pathname = pathname;
+ }
+
+
// --------------------------------------------------------- Public Methods
@@ -194,12 +205,12 @@
GenericPrincipal principal =
(GenericPrincipal) principals.get(username);
if ((principal != null) &&
- (credentials.equals(principal.getPassword()))) {
- if (debug > 1)
+ (digest(credentials).equals(principal.getPassword()))) {
+ if (debug >= 2)
log(sm.getString("memoryRealm.authenticateSuccess", username));
return (principal);
} else {
- if (debug > 1)
+ if (debug >= 2)
log(sm.getString("memoryRealm.authenticateFailure", username));
return (null);
}
@@ -207,35 +218,6 @@
}
- /**
- * Return <code>true</code> if the specified Principal has the specified
- * security role, within the context of this Realm; otherwise return
- * <code>false</code>.
- *
- * @param principal Principal for whom the role is to be checked
- * @param role Security role to be checked
- */
- public boolean hasRole(Principal principal, String role) {
-
- if ((principal == null) || (role == null) ||
- !(principal instanceof GenericPrincipal))
- return (false);
- GenericPrincipal gp = (GenericPrincipal) principal;
- if (!(gp.getRealm() == this))
- return (false);
- boolean result = gp.hasRole(role);
- if (debug > 1) {
- String name = principal.getName();
- if (result)
- log(sm.getString("memoryRealm.hasRoleSuccess", name, role));
- else
- log(sm.getString("memoryRealm.hasRoleFailure", name, role));
- }
- return (result);
-
- }
-
-
// -------------------------------------------------------- Package Methods
@@ -272,9 +254,20 @@
/**
+ * Return a short name for this Realm implementation.
+ */
+ protected String getName() {
+
+ return (this.name);
+
+ }
+
+
+ /**
* Return the password associated with the given principal's user name.
*/
protected String getPassword(String username) {
+
GenericPrincipal principal =
(GenericPrincipal) principals.get(username);
if (principal != null) {
@@ -282,6 +275,7 @@
} else {
return (null);
}
+
}
@@ -289,9 +283,10 @@
* Return the Principal associated with the given user name.
*/
protected Principal getPrincipal(String username) {
+
return (Principal) principals.get(username);
- }
+ }
// ------------------------------------------------------ Lifecycle Methods
@@ -307,33 +302,31 @@
*/
public synchronized void start() throws LifecycleException {
- // Validate and update our current component state
- if (started)
- throw new LifecycleException
- (sm.getString("memoryRealm.alreadyStarted"));
- lifecycle.fireLifecycleEvent(START_EVENT, null);
- started = true;
-
// Validate the existence of our database file
File file = new File(pathname);
if (!file.isAbsolute())
file = new File(System.getProperty("catalina.home") +
File.separator + pathname);
- if (!file.exists())
+ if (!file.exists() || !file.canRead())
throw new LifecycleException
- (sm.getString("memoryRealm.loadExist", pathname));
+ (sm.getString("memoryRealm.loadExist",
+ file.getAbsolutePath()));
// Load the contents of the database file
- if (debug > 0)
- log(sm.getString("memoryRealm.loadPath", pathname));
+ if (debug >= 1)
+ log(sm.getString("memoryRealm.loadPath",
+ file.getAbsolutePath()));
XmlMapper mapper = new XmlMapper();
mapper.addRule("tomcat-users/user", new MemoryRealmUserAction());
try {
mapper.readXml(file, this);
} catch (Exception e) {
- throw new LifecycleException("MemoryRealm.start.readXml: " + e, e);
+ throw new LifecycleException("memoryRealm.readXml", e);
}
+ // Perform normal superclass initialization
+ super.start();
+
}
@@ -346,12 +339,8 @@
*/
public synchronized void stop() throws LifecycleException {
- // Validate and update our current component state
- if (!started)
- throw new LifecycleException
- (sm.getString("memoryRealm.notStarted"));
- lifecycle.fireLifecycleEvent(STOP_EVENT, null);
- started = false;
+ // Perform normal superclass finalization
+ super.stop();
// No shutdown activities required
1.3 +148 -50 jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/realm/RealmBase.java
Index: RealmBase.java
===================================================================
RCS file: /home/cvs/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/realm/RealmBase.java,v
retrieving revision 1.2
retrieving revision 1.3
diff -u -r1.2 -r1.3
--- RealmBase.java 2000/12/31 16:30:35 1.2
+++ RealmBase.java 2001/04/11 01:46:09 1.3
@@ -1,7 +1,7 @@
/*
- * $Header: /home/cvs/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/realm/RealmBase.java,v 1.2 2000/12/31 16:30:35 nacho Exp $
- * $Revision: 1.2 $
- * $Date: 2000/12/31 16:30:35 $
+ * $Header: /home/cvs/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/realm/RealmBase.java,v 1.3 2001/04/11 01:46:09 craigmcc Exp $
+ * $Revision: 1.3 $
+ * $Date: 2001/04/11 01:46:09 $
*
* ====================================================================
*
@@ -78,6 +78,7 @@
import org.apache.catalina.LifecycleListener;
import org.apache.catalina.Logger;
import org.apache.catalina.Realm;
+import org.apache.catalina.util.HexUtils;
import org.apache.catalina.util.LifecycleSupport;
import org.apache.catalina.util.StringManager;
import org.apache.catalina.util.MD5Encoder;
@@ -93,7 +94,7 @@
* location) are identical to those currently supported by Tomcat 3.X.
*
* @author Craig R. McClanahan
- * @version $Revision: 1.2 $ $Date: 2000/12/31 16:30:35 $
+ * @version $Revision: 1.3 $ $Date: 2001/04/11 01:46:09 $
*/
public abstract class RealmBase
@@ -116,6 +117,15 @@
/**
+ * Digest algorithm used in storing passwords in a non-plaintext format.
+ * Valid values are those accepted for the algorithm name by the
+ * MessageDigest class, or <code>null</code> if no digesting should
+ * be performed.
+ */
+ protected String digest = null;
+
+
+ /**
* Descriptive information about this Realm implementation.
*/
protected static final String info =
@@ -123,45 +133,46 @@
/**
- * Short name for this Realm Implementation
+ * The lifecycle event support for this component.
*/
- protected static final String name = "RealmBase";
+ protected LifecycleSupport lifecycle = new LifecycleSupport(this);
+
/**
- * The lifecycle event support for this component.
+ * The MessageDigest object for digesting user credentials (passwords).
*/
- protected LifecycleSupport lifecycle = new LifecycleSupport(this);
+ protected MessageDigest md = null;
/**
- * The string manager for this package.
+ * The MD5 helper object for this class.
*/
- protected static StringManager sm =
- StringManager.getManager(Constants.Package);
+ protected static final MD5Encoder md5Encoder = new MD5Encoder();
/**
- * Has this component been started?
+ * MD5 message digest provider.
*/
- protected boolean started = false;
+ protected static MessageDigest md5Helper;
/**
- * The property change support for this component.
+ * The string manager for this package.
*/
- protected PropertyChangeSupport support = new PropertyChangeSupport(this);
+ protected static StringManager sm =
+ StringManager.getManager(Constants.Package);
/**
- * MD5 message digest provider.
+ * Has this component been started?
*/
- protected static MessageDigest md5Helper;
+ protected boolean started = false;
/**
- * The MD5 helper object for this class.
+ * The property change support for this component.
*/
- protected static final MD5Encoder md5Encoder = new MD5Encoder();
+ protected PropertyChangeSupport support = new PropertyChangeSupport(this);
// ------------------------------------------------------------- Properties
@@ -194,7 +205,9 @@
* Return the debugging detail level for this component.
*/
public int getDebug() {
+
return (this.debug);
+
}
@@ -204,30 +217,46 @@
* @param debug The new debugging detail level
*/
public void setDebug(int debug) {
+
this.debug = debug;
+
}
/**
- * Return descriptive information about this Realm implementation and
- * the corresponding version number, in the format
- * <code><description>/<version></code>.
+ * Return the digest algorithm used for storing credentials.
*/
- public String getInfo() {
+ public String getDigest() {
- return info;
+ return digest;
}
+
/**
- * Return short name of this Realm implementation
+ * Set the digest algorithm used for storing credentials.
+ *
+ * @param digest The new digest algorithm
*/
- public String getName() {
+ public void setDigest(String digest) {
- return name;
+ this.digest = digest;
}
+
+ /**
+ * Return descriptive information about this Realm implementation and
+ * the corresponding version number, in the format
+ * <code><description>/<version></code>.
+ */
+ public String getInfo() {
+
+ return info;
+
+ }
+
+
// --------------------------------------------------------- Public Methods
@@ -332,14 +361,35 @@
/**
* Return <code>true</code> if the specified Principal has the specified
* security role, within the context of this Realm; otherwise return
- * <code>false</code>.
+ * <code>false</code>. This method can be overridden by Realm
+ * implementations, but the default is adequate when an instance of
+ * <code>GenericPrincipal</code> is used to represent authenticated
+ * Principals from this Realm.
*
* @param principal Principal for whom the role is to be checked
* @param role Security role to be checked
*/
- public abstract boolean hasRole(Principal principal, String role);
+ public boolean hasRole(Principal principal, String role) {
+ if ((principal == null) || (role == null) ||
+ !(principal instanceof GenericPrincipal))
+ return (false);
+ GenericPrincipal gp = (GenericPrincipal) principal;
+ if (!(gp.getRealm() == this))
+ return (false);
+ boolean result = gp.hasRole(role);
+ if (debug >= 2) {
+ String name = principal.getName();
+ if (result)
+ log(sm.getString("realmBase.hasRoleSuccess", name, role));
+ else
+ log(sm.getString("realmBase.hasRoleFailure", name, role));
+ }
+ return (result);
+
+ }
+
/**
* Remove a property change listener from this component.
*
@@ -390,16 +440,25 @@
* @exception LifecycleException if this component detects a fatal error
* that prevents this component from being used
*/
- public void start()
- throws LifecycleException {
+ public void start() throws LifecycleException {
// Validate and update our current component state
if (started)
- throw new LifecycleException
- (sm.getString("memoryRealm.alreadyStarted"));
+ throw new IllegalStateException
+ (sm.getString("realmBase.alreadyStarted"));
lifecycle.fireLifecycleEvent(START_EVENT, null);
started = true;
+ // Create a MessageDigest instance for credentials, if desired
+ if (digest != null) {
+ try {
+ md = MessageDigest.getInstance(digest);
+ } catch (NoSuchAlgorithmException e) {
+ throw new LifecycleException
+ (sm.getString("realmBase.algorithm", digest), e);
+ }
+ }
+
}
@@ -418,11 +477,14 @@
// Validate and update our current component state
if (!started)
- throw new LifecycleException
- (sm.getString("memoryRealm.notStarted"));
+ throw new IllegalStateException
+ (sm.getString("realmBase.notStarted"));
lifecycle.fireLifecycleEvent(STOP_EVENT, null);
started = false;
+ // Clean up allocated resources
+ md = null;
+
}
@@ -430,6 +492,39 @@
/**
+ * Digest the password using the specified algorithm and
+ * convert the result to a corresponding hexadecimal string.
+ * If exception, the plain credentials string is returned.
+ *
+ * <strong>IMPLEMENTATION NOTE</strong> - This implementation is
+ * synchronized because it reuses the MessageDigest instance.
+ * This should be faster than cloning the instance on every request.
+ *
+ * @param credentials Password or other credentials to use in
+ * authenticating this username
+ */
+ protected String digest(String credentials) {
+
+ // If no MessageDigest instance is specified, return unchanged
+ if (md == null)
+ return (credentials);
+
+ // Digest the user credentials and return as hexadecimal
+ synchronized (this) {
+ try {
+ md.reset();
+ md.update(credentials.getBytes());
+ return (HexUtils.convert(md.digest()));
+ } catch (Exception e) {
+ log(sm.getString("realmBase.digest"), e);
+ return (credentials);
+ }
+ }
+
+ }
+
+
+ /**
* Return the digest associated with given principal's user name.
*/
protected String getDigest(String username, String realmName) {
@@ -450,6 +545,13 @@
/**
+ * Return a short name for this Realm implementation, for use in
+ * log messages.
+ */
+ protected abstract String getName();
+
+
+ /**
* Return the password associated with the given principal's user name.
*/
protected abstract String getPassword(String username);
@@ -467,21 +569,20 @@
* @param message Message to be logged
*/
protected void log(String message) {
- Logger logger = null;
+ Logger logger = null;
+ String name = null;
if (container != null) {
logger = container.getLogger();
+ name = container.getName();
}
if (logger != null) {
- logger.log(getName()+"[" + container.getName() + "]: " + message);
+ logger.log(getName()+"[" + name + "]: " + message);
} else {
- String containerName = null;
- if (container != null) {
- containerName = container.getName();
- }
- System.out.println(getName()+"[" + containerName + "]: " + message);
+ System.out.println(getName()+"[" + name + "]: " + message);
}
+
}
@@ -492,21 +593,18 @@
* @param throwable Associated exception
*/
protected void log(String message, Throwable throwable) {
- Logger logger = null;
+ Logger logger = null;
+ String name = null;
if (container != null) {
logger = container.getLogger();
+ name = container.getName();
}
if (logger != null) {
- logger.log(getName()+"[" + container.getName() + "]: " + message, throwable);
+ logger.log(getName()+"[" + name + "]: " + message, throwable);
} else {
- String containerName = null;
- if (container != null) {
- containerName = container.getName();
- }
- System.out.println(getName()+"[" + containerName + "]: " + message);
- System.out.println("" + throwable);
+ System.out.println(getName()+"[" + name + "]: " + message);
throwable.printStackTrace(System.out);
}
}
Re: Tabs vs. spaces (was: cvs commit: blah blah blah)
Posted by Kief Morris <ki...@bitbull.com>.
cmanolache@yahoo.com typed the following on 09:11 AM 4/12/2001 -0700
>Changing a file from 09 ( TAB ) to spaces ( or reverse ) is _bad_, it is
>(IMHO) lack of respect for the people who wrote the original code.
I take your point about respect for the people who wrote the original code,
(and the other developers working on it) which is why I brought it up - not
because I wanted to resurrect an old holy war on an extremely dull topic.
The only files I'm changing are the ones I'm doing a lot of work on, the
session classes in Catalina 4. The original author, Craig has said he doesn't
mind. I have no interest in mucking with any other files, I can deal with whatever
spacing they already have.
Anyway, I don't want to carry this thread on, but figured I ought to justify
the changes I made, and reassure that I'm not going to go nuts mangling
other files.
Kief
RE: Tabs vs. spaces (was: cvs commit: blah blah blah)
Posted by T J Morgan <tm...@cinci.rr.com>.
I believe this topic has been hashed out enough. Can we move on.
Todd
-----Original Message-----
From: cmanolache@yahoo.com [mailto:cmanolache@yahoo.com]
Sent: Thursday, April 12, 2001 12:11 PM
To: tomcat-dev@jakarta.apache.org
Subject: Re: Tabs vs. spaces (was: cvs commit: blah blah blah)
Please, don't start this ( again ). You can read the archives ( every six
months some smart people ask this question ), or check other mailing lists
for the same question.
Use whatever other people are using in that project. In tomcat you'll
notice the 4 space indentation, and 09 ( TAB ) is used in many
(most) files.
Changing a file from 09 ( TAB ) to spaces ( or reverse ) is _bad_, it is
(IMHO) lack of respect for the people who wrote the original code.
It may be better to use only spaces for new code you write - but at least
in tomcat this is not required ( I do use 09 in most code I write for
example, and so far nobody -1 a commit I made for this reason ).
Costin
On Thu, 12 Apr 2001, Kief Morris wrote:
> Jon Stevens typed the following on 06:50 PM 4/10/2001 -0700
> >
> >Craig, does this mean you (finally) aren't using tabs anymore? :-)
> >
>
> So, are spaces kosher? The Sun coding standards document (which is the
> official Jakarta guideline?) says either is OK, but the mixed tabs and
spaces
> format I've found in the Catalina code I've mucked with is a PITA. Can I
just
> set my editor to use 4 spaces for tabs and reformat files I work with
> accordingly, without spawning a jihad?
>
> Kief
>
Re: Tabs vs. spaces (was: cvs commit: blah blah blah)
Posted by cm...@yahoo.com.
Please, don't start this ( again ). You can read the archives ( every six
months some smart people ask this question ), or check other mailing lists
for the same question.
Use whatever other people are using in that project. In tomcat you'll
notice the 4 space indentation, and 09 ( TAB ) is used in many
(most) files.
Changing a file from 09 ( TAB ) to spaces ( or reverse ) is _bad_, it is
(IMHO) lack of respect for the people who wrote the original code.
It may be better to use only spaces for new code you write - but at least
in tomcat this is not required ( I do use 09 in most code I write for
example, and so far nobody -1 a commit I made for this reason ).
Costin
On Thu, 12 Apr 2001, Kief Morris wrote:
> Jon Stevens typed the following on 06:50 PM 4/10/2001 -0700
> >
> >Craig, does this mean you (finally) aren't using tabs anymore? :-)
> >
>
> So, are spaces kosher? The Sun coding standards document (which is the
> official Jakarta guideline?) says either is OK, but the mixed tabs and spaces
> format I've found in the Catalina code I've mucked with is a PITA. Can I just
> set my editor to use 4 spaces for tabs and reformat files I work with
> accordingly, without spawning a jihad?
>
> Kief
>
Re: Tabs vs. spaces (was: cvs commit: blah blah blah)
Posted by Jon Stevens <jo...@latchkey.com>.
on 4/12/01 8:43 AM, "Kief Morris" <ki...@bitbull.com> wrote:
> So, are spaces kosher? The Sun coding standards document (which is the
> official Jakarta guideline?) says either is OK, but the mixed tabs and spaces
> format I've found in the Catalina code I've mucked with is a PITA. Can I just
> set my editor to use 4 spaces for tabs and reformat files I work with
> accordingly, without spawning a jihad?
>
> Kief
You can read what it says on the website...
<http://jakarta.apache.org/site/source.html>
-jon
Re: Tabs vs. spaces (was: cvs commit: blah blah blah)
Posted by "Craig R. McClanahan" <cr...@apache.org>.
On Thu, 12 Apr 2001, Kief Morris wrote:
> Jon Stevens typed the following on 06:50 PM 4/10/2001 -0700
> >
> >Craig, does this mean you (finally) aren't using tabs anymore? :-)
> >
>
> So, are spaces kosher? The Sun coding standards document (which is the
> official Jakarta guideline?) says either is OK, but the mixed tabs and spaces
> format I've found in the Catalina code I've mucked with is a PITA. Can I just
> set my editor to use 4 spaces for tabs and reformat files I work with
> accordingly, without spawning a jihad?
>
> Kief
>
>
One more note on this, even if you are using spaces for indentation, the
indentation interval for Tomcat code should be four spaces.
Craig
Re: Tabs vs. spaces (was: cvs commit: blah blah blah)
Posted by "Craig R. McClanahan" <cr...@apache.org>.
On Thu, 12 Apr 2001, Kief Morris wrote:
> Jon Stevens typed the following on 06:50 PM 4/10/2001 -0700
> >
> >Craig, does this mean you (finally) aren't using tabs anymore? :-)
> >
>
> So, are spaces kosher? The Sun coding standards document (which is the
> official Jakarta guideline?) says either is OK, but the mixed tabs and spaces
> format I've found in the Catalina code I've mucked with is a PITA. Can I just
> set my editor to use 4 spaces for tabs and reformat files I work with
> accordingly, without spawning a jihad?
>
> Kief
>
>
Spaces are definitely kosher for Tomcat code.
The coding standards document does say either is OK, and prior to about
nine months ago I didn't care (Emacs took care of the details for me when
I pressed the TAB key once). At that point, I switched to space-fill --
but of course lots of the Catalina code preceeded that change.
If you want to change a source file that you're currently working on, I
would say go for it. A suggestion, though, would be to do the untabify
change as a single commit (with a message like "cosmetic change only, no
functional change") so that we can still clearly identify functional
changes in the other commits.
Craig
RE: Tabs vs. spaces (was: cvs commit: blah blah blah)
Posted by Nick Bauman <ni...@cortexity.com>.
My company is, for a team of about 15+ people. We use JIndent on .java
files. Works great. If it's doing damage, then it misconfigured.
> True; not using a beautifier or CVS (now we're using
> Perforce). I find that beautifiers do more damage
> than good, but I'm happy to be enlightened; is anyone
> actually doing this in practice?
>
> -tom
>
> -----Original Message-----
> From: Nick Bauman [mailto:nick@cortexity.com]
> Sent: Thursday, April 12, 2001 9:34 AM
> To: tomcat-dev@jakarta.apache.org
> Subject: RE: Tabs vs. spaces (was: cvs commit: blah blah blah)
>
>
> This issue would be moot if you frontended your CVS checkins with a
> beautifier.
>
>> Here's an edited version of a comment on tabs and spaces I sent to our
>> development team that might be useful.
>>
>> ---cut---
>
> --
> Nick Bauman
> Software Developer
> 3023 Lynn #22
> Minneapolis, MN
> 55416
> Mobile Phone: (612) 810-7406
--
Nick Bauman
Software Developer
3023 Lynn #22
Minneapolis, MN
55416
Mobile Phone: (612) 810-7406
RE: Tabs vs. spaces (was: cvs commit: blah blah blah)
Posted by Tomas Rokicki <ro...@instantis.com>.
True; not using a beautifier or CVS (now we're using
Perforce). I find that beautifiers do more damage
than good, but I'm happy to be enlightened; is anyone
actually doing this in practice?
-tom
-----Original Message-----
From: Nick Bauman [mailto:nick@cortexity.com]
Sent: Thursday, April 12, 2001 9:34 AM
To: tomcat-dev@jakarta.apache.org
Subject: RE: Tabs vs. spaces (was: cvs commit: blah blah blah)
This issue would be moot if you frontended your CVS checkins with a
beautifier.
> Here's an edited version of a comment on tabs and spaces I sent to our
> development team that might be useful.
>
> ---cut---
--
Nick Bauman
Software Developer
3023 Lynn #22
Minneapolis, MN
55416
Mobile Phone: (612) 810-7406
RE: Tabs vs. spaces (was: cvs commit: blah blah blah)
Posted by Nick Bauman <ni...@cortexity.com>.
This issue would be moot if you frontended your CVS checkins with a beautifier.
> Here's an edited version of a comment on tabs and spaces I sent to our
> development team that might be useful.
>
> ---cut---
--
Nick Bauman
Software Developer
3023 Lynn #22
Minneapolis, MN
55416
Mobile Phone: (612) 810-7406
RE: Tabs vs. spaces (was: cvs commit: blah blah blah)
Posted by Tomas Rokicki <ro...@instantis.com>.
Here's an edited version of a comment on tabs and spaces I sent to
our development team that might be useful.
---cut---
Okay, we've had some discussions this morning, and we've got to
deal with tabs and indentation better than we have been.
Some files are simply unviewable right now in various editors
with various settings, and it is time to clean it all up and do
it right.
First, there is a distinction between *tab stops* and *indentation*.
The tab stops define how the editor interprets a tab character in
the file---how many space-equivalents that tab character gives.
The indentation defines how many spaces a nested level should be
indented compared to the previous level. Most editors (including
vim and emacs) use the *indentation*, not the tab stops, in
interpreting how much whitespace to introduce when the user hits
the tab key. That is, when the user hits the tab key, some number
of spaces or tabs will be inserted to match the indentation.
Most modern editors (including emacs and vi) support both concepts.
Unfortunately, most modern programmers (including most of us)
only understand the tab stop concept. This is broken.
Finally, many programs, printers, and so forth are hard-wired to use
8-character tab stops, and there is often no way to work around
this.
There is a clean solution. It's simple and it's elegant and it's
easy.
The rules are as follows.
1. Never ever set your tab stops to something other than 8. This
means, never do (setq tab-width 4) in emacs or set tabstop=4
in vim. If you do, you will create files that will be unusable
by others.
2. Instead, set your indentation to whatever you want, as the
author of a file. This is usually done with
(setq c-basic-indent 4) in emacs, or set softtabstop=4
in vim. This is what you want to do, and if you do this, your
files will work well with everyone.
3. If you want, in your files, add a line such as
/* -*- mode: java; c-basic-indent: 4; -*- */
to force all other emacs users to get *your* desired indentation
so that if they edit your files, their new lines will match your
indentation.
4. I would prefer that none of our source files contain any tab
characters at all since this is what causes the files to display
incorrectly in the different editors, but I'm not going to insist
on this. To do this, in emacs use (setq indent-tabs-mode nil);
in vim use set expandtab. If everyone does as in 1 above, there
won't be a problem in any case.
If you encounter a file that just doesn't display properly for you,
it's probably got a mixture of tabs and spaces and your tab stop is
set differently than (one of the) authors'. This is the crux of
the problem. To fix this, you can set the tab stop of your editor
until it looks most proper, then untabify it (in emacs, untabify;
in vi, set expandtab) and save out the file with no tabs. Then fix
up any remaining indentation problems by hand and check it in.
Unfortunately, the changes will be massive by perforce standards, but
it's probably best to deal with this as early as possible, set the
standard now, and totally prevent problems in the future.
For further reference, consult
http://www.jwz.org/doc/tabs-vs-spaces.html
If anyone has any questions or further discussion is needed, contact
me.
-tom
-----Original Message-----
From: Kief Morris [mailto:kief@bitbull.com]
Sent: Thursday, April 12, 2001 8:43 AM
To: tomcat-dev@jakarta.apache.org
Subject: Tabs vs. spaces (was: cvs commit: blah blah blah)
Jon Stevens typed the following on 06:50 PM 4/10/2001 -0700
>
>Craig, does this mean you (finally) aren't using tabs anymore? :-)
>
So, are spaces kosher? The Sun coding standards document (which is the
official Jakarta guideline?) says either is OK, but the mixed tabs and
spaces
format I've found in the Catalina code I've mucked with is a PITA. Can I
just
set my editor to use 4 spaces for tabs and reformat files I work with
accordingly, without spawning a jihad?
Kief
Tabs vs. spaces (was: cvs commit: blah blah blah)
Posted by Kief Morris <ki...@bitbull.com>.
Jon Stevens typed the following on 06:50 PM 4/10/2001 -0700
>
>Craig, does this mean you (finally) aren't using tabs anymore? :-)
>
So, are spaces kosher? The Sun coding standards document (which is the
official Jakarta guideline?) says either is OK, but the mixed tabs and spaces
format I've found in the Catalina code I've mucked with is a PITA. Can I just
set my editor to use 4 spaces for tabs and reformat files I work with
accordingly, without spawning a jihad?
Kief
Re: cvs commit:
Posted by Nick Bauman <ni...@cortexity.com>.
Yeah, I don't see what the big wup is about formatting. We run a beautifier
against all commits to CVS. Works great and no more "format" wars.
> on 4/10/01 6:46 PM, "craigmcc@apache.org" <cr...@apache.org> wrote:
>> +
>> /**
>> + * Close any database connection that is currently open.
>> + */
>> + protected void close() {
>> +
>> + // Do nothing if the database connection is already closed +
>> if (dbConnection == null)
>> + return;
>> +
>> + // Close our prepared statements (if any)
>> + try {
>> + preparedCredentials.close();
>> + } catch (Throwable f) {
>> + ;
>> + }
>> + try {
>> + preparedRoles.close();
>> + } catch (Throwable f) {
>> + ;
>> + }
>> +
>> + // Close this database connection, and log any errors
>> + try {
>> + dbConnection.close();
>> + } catch (SQLException e) {
>> + log(sm.getString("jdbcRealm.close"), e); // Just log it
>> here + }
>> +
>> + // Release resources associated with the closed connection +
>> dbConnection = null;
>> + preparedCredentials = null;
>> + preparedRoles = null;
>> +
>> + }
>
> Craig, does this mean you (finally) aren't using tabs anymore? :-)
> Craig, does this mean you (finally) aren't using tabs anymore? :-)
> -jon
--
Nick Bauman
Software Developer
3023 Lynn #22
Minneapolis, MN
55416
Mobile Phone: (612) 810-7406
Re: cvs commit: jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/realm
GenericPrincipal.java JDBCRealm.java LocalStrings.properties MemoryRealm.java
RealmBase.java
Posted by "Craig R. McClanahan" <cr...@apache.org>.
On Tue, 10 Apr 2001, Jon Stevens wrote:
> on 4/10/01 6:46 PM, "craigmcc@apache.org" <cr...@apache.org> wrote:
>
> > +
> > /**
> > + * Close any database connection that is currently open.
> > + */
> > + protected void close() {
> > +
> > + // Do nothing if the database connection is already closed
> > + if (dbConnection == null)
> > + return;
> > +
> > + // Close our prepared statements (if any)
> > + try {
> > + preparedCredentials.close();
> > + } catch (Throwable f) {
> > + ;
> > + }
> > + try {
> > + preparedRoles.close();
> > + } catch (Throwable f) {
> > + ;
> > + }
> > +
> > + // Close this database connection, and log any errors
> > + try {
> > + dbConnection.close();
> > + } catch (SQLException e) {
> > + log(sm.getString("jdbcRealm.close"), e); // Just log it here
> > + }
> > +
> > + // Release resources associated with the closed connection
> > + dbConnection = null;
> > + preparedCredentials = null;
> > + preparedRoles = null;
> > +
> > + }
>
> Craig, does this mean you (finally) aren't using tabs anymore? :-)
>
I haven't been using tabs for quite a long while :-), although when
temporarily borrowing someone else's computer to do edits they might sneak
in.
You'll note that all the modified code above is nicely lined up, courtesy
of emacs being set to change tabs to spaces.
> -jon
>
>
Re: cvs commit:
jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/realm
GenericPrincipal.java JDBCRealm.java LocalStrings.properties
MemoryRealm.java RealmBase.java
Posted by Jon Stevens <jo...@latchkey.com>.
on 4/10/01 6:46 PM, "craigmcc@apache.org" <cr...@apache.org> wrote:
> +
> /**
> + * Close any database connection that is currently open.
> + */
> + protected void close() {
> +
> + // Do nothing if the database connection is already closed
> + if (dbConnection == null)
> + return;
> +
> + // Close our prepared statements (if any)
> + try {
> + preparedCredentials.close();
> + } catch (Throwable f) {
> + ;
> + }
> + try {
> + preparedRoles.close();
> + } catch (Throwable f) {
> + ;
> + }
> +
> + // Close this database connection, and log any errors
> + try {
> + dbConnection.close();
> + } catch (SQLException e) {
> + log(sm.getString("jdbcRealm.close"), e); // Just log it here
> + }
> +
> + // Release resources associated with the closed connection
> + dbConnection = null;
> + preparedCredentials = null;
> + preparedRoles = null;
> +
> + }
Craig, does this mean you (finally) aren't using tabs anymore? :-)
-jon