1. I do not like that you need to convert chunks to Strings to call that API.
Though in this particular case the strings should be short enough for it to do not matter. 2. I think we should not blindly cache authentication strings, even if it costs some performance. I think such things should not be kept in memory longer than necessary. 3. Looking into implementation of this JAXB DatatypeConverter class, it serves as a facade, delegating all calls to an instance of converter class. The converter class can be changed via DatatypeConverter.setDatatypeConverter(...). It should not be a security issue though, as otherwise all JAXB were compromised. (In my understanding, the converter can be replaced only in Java 7. The 7u17 implementation of setDatatypeConverter() does check necessary permissions via security manager. The 6u43 one is strange and in my understanding it is noop, as "theConverter == null" condition there is always false). 2013/3/21 <ma...@apache.org>: > Author: markt > Date: Wed Mar 20 20:22:42 2013 > New Revision: 1459031 > > URL: http://svn.apache.org/r1459031 > Log: > Deprecate Base64 and switch to DatatypeConverter > > Modified: > > tomcat/trunk/java/org/apache/catalina/authenticator/BasicAuthenticator.java > > tomcat/trunk/java/org/apache/catalina/authenticator/SpnegoAuthenticator.java > tomcat/trunk/java/org/apache/catalina/realm/JNDIRealm.java > tomcat/trunk/java/org/apache/catalina/util/Base64.java > > Modified: > tomcat/trunk/java/org/apache/catalina/authenticator/BasicAuthenticator.java > URL: > http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/authenticator/BasicAuthenticator.java?rev=1459031&r1=1459030&r2=1459031&view=diff > ============================================================================== > --- > tomcat/trunk/java/org/apache/catalina/authenticator/BasicAuthenticator.java > (original) > +++ > tomcat/trunk/java/org/apache/catalina/authenticator/BasicAuthenticator.java > Wed Mar 20 20:22:42 2013 > @@ -24,13 +24,13 @@ import java.security.Principal; > > import javax.servlet.http.HttpServletRequest; > import javax.servlet.http.HttpServletResponse; > +import javax.xml.bind.DatatypeConverter; > > import org.apache.catalina.connector.Request; > -import org.apache.catalina.util.Base64; > import org.apache.juli.logging.Log; > import org.apache.juli.logging.LogFactory; > +import org.apache.tomcat.util.buf.B2CConverter; > import org.apache.tomcat.util.buf.ByteChunk; > -import org.apache.tomcat.util.buf.CharChunk; > import org.apache.tomcat.util.buf.MessageBytes; > > > @@ -113,18 +113,28 @@ public class BasicAuthenticator > // FIXME: Add trimming > // authorizationBC.trim(); > > - CharChunk authorizationCC = authorization.getCharChunk(); > - Base64.decode(authorizationBC, authorizationCC); > + // Use the StringCache as these will be the same between > + // requests > + String encoded = authorizationBC.toString(); > + byte[] decoded = > DatatypeConverter.parseBase64Binary(encoded); > > // Get username and password > - int colon = authorizationCC.indexOf(':'); > + int colon = -1; > + for (int i = 0; i < decoded.length; i++) { > + if (decoded[i] == ':') { > + colon = i; > + break; > + } > + } > + > if (colon < 0) { > - username = authorizationCC.toString(); > + username = new String(decoded, B2CConverter.ISO_8859_1); > } else { > - char[] buf = authorizationCC.getBuffer(); > - username = new String(buf, 0, colon); > - password = new String(buf, colon + 1, > - authorizationCC.getEnd() - colon - 1); > + username = new String( > + decoded, 0, colon, B2CConverter.ISO_8859_1); > + password = new String( > + decoded, colon + 1, decoded.length - colon - 1, > + B2CConverter.ISO_8859_1); > } > > authorizationBC.setOffset(authorizationBC.getOffset() - 6); > > Modified: > tomcat/trunk/java/org/apache/catalina/authenticator/SpnegoAuthenticator.java > URL: > http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/authenticator/SpnegoAuthenticator.java?rev=1459031&r1=1459030&r2=1459031&view=diff > ============================================================================== > --- > tomcat/trunk/java/org/apache/catalina/authenticator/SpnegoAuthenticator.java > (original) > +++ > tomcat/trunk/java/org/apache/catalina/authenticator/SpnegoAuthenticator.java > Wed Mar 20 20:22:42 2013 > @@ -31,9 +31,9 @@ import javax.xml.bind.DatatypeConverter; > > import org.apache.catalina.LifecycleException; > import org.apache.catalina.connector.Request; > -import org.apache.catalina.util.Base64; > import org.apache.juli.logging.Log; > import org.apache.juli.logging.LogFactory; > +import org.apache.tomcat.util.buf.B2CConverter; > import org.apache.tomcat.util.buf.ByteChunk; > import org.apache.tomcat.util.buf.MessageBytes; > import org.ietf.jgss.GSSContext; > @@ -190,10 +190,15 @@ public class SpnegoAuthenticator extends > // FIXME: Add trimming > // authorizationBC.trim(); > > - ByteChunk decoded = new ByteChunk(); > - Base64.decode(authorizationBC, decoded); > + // Create the String directly as this will change on each request > and we > + // don't want to use the StringCache > + String encoded = new String(authorizationBC.getBuffer(), > + authorizationBC.getOffset(), > + authorizationBC.getLength(), B2CConverter.ISO_8859_1); > > - if (decoded.getLength() == 0) { > + byte[] decoded = DatatypeConverter.parseBase64Binary(encoded); > + > + if (decoded.length == 0) { > if (log.isDebugEnabled()) { > log.debug(sm.getString( > "spnegoAuthenticator.authHeaderNoToken")); > @@ -232,8 +237,7 @@ public class SpnegoAuthenticator extends > }; > gssContext = manager.createContext(Subject.doAs(lc.getSubject(), > action)); > > - outToken = gssContext.acceptSecContext(decoded.getBytes(), > - decoded.getOffset(), decoded.getLength()); > + outToken = gssContext.acceptSecContext(decoded, 0, > decoded.length); > > if (outToken == null) { > if (log.isDebugEnabled()) { > > 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=1459031&r1=1459030&r2=1459031&view=diff > ============================================================================== > --- tomcat/trunk/java/org/apache/catalina/realm/JNDIRealm.java (original) > +++ tomcat/trunk/java/org/apache/catalina/realm/JNDIRealm.java Wed Mar 20 > 20:22:42 2013 > @@ -17,7 +17,6 @@ > > package org.apache.catalina.realm; > > -import java.io.IOException; > import java.net.URI; > import java.net.URISyntaxException; > import java.security.Principal; > @@ -54,10 +53,7 @@ import javax.naming.directory.SearchResu > import javax.xml.bind.DatatypeConverter; > > import org.apache.catalina.LifecycleException; > -import org.apache.catalina.util.Base64; > import org.apache.tomcat.util.buf.B2CConverter; > -import org.apache.tomcat.util.buf.ByteChunk; > -import org.apache.tomcat.util.buf.CharChunk; > import org.ietf.jgss.GSSCredential; > > /** > @@ -1571,29 +1567,15 @@ public class JNDIRealm extends RealmBase > md.update(credentials.getBytes(B2CConverter.ISO_8859_1)); > > // Decode stored password. > - ByteChunk pwbc = new ByteChunk(password.length()); > - try { > - > pwbc.append(password.getBytes(B2CConverter.ISO_8859_1), > - 0, password.length()); > - } catch (IOException e) { > - // Should never happen > - containerLog.error("Could not append password bytes > to chunk: ", e); > - } > - > - CharChunk decoded = new CharChunk(); > - Base64.decode(pwbc, decoded); > - char[] pwarray = decoded.getBuffer(); > + byte[] decoded = > + DatatypeConverter.parseBase64Binary(password); > > // Split decoded password into hash and salt. > final int saltpos = 20; > byte[] hash = new byte[saltpos]; > - for (int i=0; i< hash.length; i++) { > - hash[i] = (byte) pwarray[i]; > - } > - > - byte[] salt = new byte[pwarray.length - saltpos]; > - for (int i=0; i< salt.length; i++) > - salt[i] = (byte)pwarray[i+saltpos]; > + System.arraycopy(decoded, 0, hash, 0, saltpos); > + byte[] salt = new byte[decoded.length - saltpos]; > + System.arraycopy(decoded, saltpos, salt, 0, salt.length); > > md.update(salt); > byte[] dp = md.digest(); > > Modified: tomcat/trunk/java/org/apache/catalina/util/Base64.java > URL: > http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/util/Base64.java?rev=1459031&r1=1459030&r2=1459031&view=diff > ============================================================================== > --- tomcat/trunk/java/org/apache/catalina/util/Base64.java (original) > +++ tomcat/trunk/java/org/apache/catalina/util/Base64.java Wed Mar 20 > 20:22:42 2013 > @@ -29,7 +29,12 @@ import org.apache.tomcat.util.buf.CharCh > * > * @author Jeffrey Rodriguez > * @version $Id$ > + * > + * @deprecated Use {@link > + * javax.xml.bind.DatatypeConverter#parseBase64Binary(String)}. > + * This class will be removed in Tomcat 8. > */ > +@Deprecated > public final class Base64 > { > private static final int BASELENGTH = 255; > > > > --------------------------------------------------------------------- > 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
