HADOOP-15808. Harden Token service loader use. Contributed by Steve Loughran.
Note that the patch for TestToken is much simpler than the 3.2+ patch. In later branches,it modifies a test case which is not present in this branch. (cherry picked from commit 202926ac3301298753abd0e6e1f324caf0202ec6) Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/1192fad0 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/1192fad0 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/1192fad0 Branch: refs/heads/branch-3.1 Commit: 1192fad0d2b453064cdad86b26cffd7c35c6542c Parents: 14ecdb6 Author: Steve Loughran <ste...@apache.org> Authored: Tue Dec 11 17:38:56 2018 +0000 Committer: Steve Loughran <ste...@apache.org> Committed: Tue Dec 11 17:39:55 2018 +0000 ---------------------------------------------------------------------- .../hadoop/security/token/DtFileOperations.java | 25 +++++++++-- .../org/apache/hadoop/security/token/Token.java | 44 +++++++++++++++----- .../java/org/apache/hadoop/ipc/TestSaslRPC.java | 4 +- .../apache/hadoop/security/token/TestToken.java | 9 ++-- 4 files changed, 61 insertions(+), 21 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/1192fad0/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/DtFileOperations.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/DtFileOperations.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/DtFileOperations.java index d36ad9b..c332ddf 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/DtFileOperations.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/DtFileOperations.java @@ -24,6 +24,8 @@ import java.io.PrintStream; import java.text.DateFormat; import java.util.ArrayList; import java.util.Date; +import java.util.Iterator; +import java.util.ServiceConfigurationError; import java.util.ServiceLoader; import org.apache.commons.lang.StringUtils; @@ -130,7 +132,7 @@ public final class DtFileOperations { * @param creds the Credentials object to be printed out. * @param alias print only tokens matching alias (null matches all). * @param out print to this stream. - * @throws IOException + * @throws IOException failure to unmarshall a token identifier. */ public static void printCredentials( Credentials creds, Text alias, PrintStream out) @@ -145,8 +147,13 @@ public final class DtFileOperations { out.println(StringUtils.repeat("-", 80)); tokenHeader = false; } - AbstractDelegationTokenIdentifier id = - (AbstractDelegationTokenIdentifier) token.decodeIdentifier(); + AbstractDelegationTokenIdentifier id; + try { + id = (AbstractDelegationTokenIdentifier) token.decodeIdentifier(); + } catch (IllegalStateException e) { + LOG.debug("Failed to decode token identifier", e); + id = null; + } out.printf(fmt, token.getKind(), token.getService(), (id != null) ? id.getRenewer() : NA_STRING, (id != null) ? formatDate(id.getMaxDate()) : NA_STRING, @@ -172,7 +179,17 @@ public final class DtFileOperations { Credentials creds = tokenFile.exists() ? Credentials.readTokenStorageFile(tokenFile, conf) : new Credentials(); ServiceLoader<DtFetcher> loader = ServiceLoader.load(DtFetcher.class); - for (DtFetcher fetcher : loader) { + Iterator<DtFetcher> iterator = loader.iterator(); + while (iterator.hasNext()) { + DtFetcher fetcher; + try { + fetcher = iterator.next(); + } catch (ServiceConfigurationError e) { + // failure to load a token implementation + // log at debug and continue. + LOG.debug("Failed to load token fetcher implementation", e); + continue; + } if (matchService(fetcher, service, url)) { if (!fetcher.isTokenRequired()) { String message = "DtFetcher for service '" + service + http://git-wip-us.apache.org/repos/asf/hadoop/blob/1192fad0/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/Token.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/Token.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/Token.java index 33cb9ec..3a981a9 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/Token.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/Token.java @@ -34,7 +34,9 @@ import org.slf4j.LoggerFactory; import java.io.*; import java.util.Arrays; +import java.util.Iterator; import java.util.Map; +import java.util.ServiceConfigurationError; import java.util.ServiceLoader; import java.util.UUID; @@ -146,14 +148,25 @@ public class Token<T extends TokenIdentifier> implements Writable { synchronized (Token.class) { if (tokenKindMap == null) { tokenKindMap = Maps.newHashMap(); - for (TokenIdentifier id : ServiceLoader.load(TokenIdentifier.class)) { - tokenKindMap.put(id.getKind(), id.getClass()); + // start the service load process; it's only in the "next()" calls + // where implementations are loaded. + final Iterator<TokenIdentifier> tokenIdentifiers = + ServiceLoader.load(TokenIdentifier.class).iterator(); + while (tokenIdentifiers.hasNext()) { + try { + TokenIdentifier id = tokenIdentifiers.next(); + tokenKindMap.put(id.getKind(), id.getClass()); + } catch (ServiceConfigurationError e) { + // failure to load a token implementation + // log at debug and continue. + LOG.debug("Failed to load token identifier implementation", e); + } } } cls = tokenKindMap.get(kind); } if (cls == null) { - LOG.debug("Cannot find class for token kind " + kind); + LOG.debug("Cannot find class for token kind {}", kind); return null; } return cls; @@ -162,8 +175,9 @@ public class Token<T extends TokenIdentifier> implements Writable { /** * Get the token identifier object, or null if it could not be constructed * (because the class could not be loaded, for example). - * @return the token identifier, or null - * @throws IOException + * @return the token identifier, or null if there was no class found for it + * @throws IOException failure to unmarshall the data + * @throws RuntimeException if the token class could not be instantiated. */ @SuppressWarnings("unchecked") public T decodeIdentifier() throws IOException { @@ -262,7 +276,7 @@ public class Token<T extends TokenIdentifier> implements Writable { assert !publicToken.isPrivate(); publicService = publicToken.service; if (LOG.isDebugEnabled()) { - LOG.debug("Cloned private token " + this + " from " + publicToken); + LOG.debug("Cloned private token {} from {}", this, publicToken); } } @@ -460,14 +474,22 @@ public class Token<T extends TokenIdentifier> implements Writable { } renewer = TRIVIAL_RENEWER; synchronized (renewers) { - for (TokenRenewer canidate : renewers) { - if (canidate.handleKind(this.kind)) { - renewer = canidate; - return renewer; + Iterator<TokenRenewer> it = renewers.iterator(); + while (it.hasNext()) { + try { + TokenRenewer candidate = it.next(); + if (candidate.handleKind(this.kind)) { + renewer = candidate; + return renewer; + } + } catch (ServiceConfigurationError e) { + // failure to load a token implementation + // log at debug and continue. + LOG.debug("Failed to load token renewer implementation", e); } } } - LOG.warn("No TokenRenewer defined for token kind " + this.kind); + LOG.warn("No TokenRenewer defined for token kind {}", kind); return renewer; } http://git-wip-us.apache.org/repos/asf/hadoop/blob/1192fad0/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestSaslRPC.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestSaslRPC.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestSaslRPC.java index 0b463a5..42baa12 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestSaslRPC.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestSaslRPC.java @@ -926,7 +926,9 @@ public class TestSaslRPC extends TestRpcBase { private static void assertAuthEquals(Pattern expect, String actual) { // this allows us to see the regexp and the value it didn't match if (!expect.matcher(actual).matches()) { - fail(); // it failed + // it failed + fail(String.format("\"%s\" did not match pattern %s", + actual, expect)); } } http://git-wip-us.apache.org/repos/asf/hadoop/blob/1192fad0/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/token/TestToken.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/token/TestToken.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/token/TestToken.java index f6e5133..a8b47d6 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/token/TestToken.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/token/TestToken.java @@ -85,13 +85,12 @@ public class TestToken { "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLM" + "NOPQRSTUVWXYZ01234567890!@#$%^&*()-=_+[]{}|;':,./<>?"}; Token<AbstractDelegationTokenIdentifier> orig; - Token<AbstractDelegationTokenIdentifier> copy = - new Token<AbstractDelegationTokenIdentifier>(); + Token<AbstractDelegationTokenIdentifier> copy = new Token<>(); // ensure that for each string the input and output values match for(int i=0; i< values.length; ++i) { String val = values[i]; - System.out.println("Input = " + val); - orig = new Token<AbstractDelegationTokenIdentifier>(val.getBytes(), + Token.LOG.info("Input = {}", val); + orig = new Token<>(val.getBytes(), val.getBytes(), new Text(val), new Text(val)); String encode = orig.encodeToUrlString(); copy.decodeFromUrlString(encode); @@ -109,7 +108,7 @@ public class TestToken { new Text("owner"), new Text("renewer"), new Text("realUser")); Token<TestDelegationTokenIdentifier> token = - new Token<TestDelegationTokenIdentifier>(id, secretManager); + new Token<>(id, secretManager); TokenIdentifier idCopy = token.decodeIdentifier(); assertNotSame(id, idCopy); --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org
