Repository: hadoop Updated Branches: refs/heads/trunk dd07038ff -> 5eb7dbe9b
Fixing Job History Server Configuration parsing. (Jason Lowe via asuresh) Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/5eb7dbe9 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/5eb7dbe9 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/5eb7dbe9 Branch: refs/heads/trunk Commit: 5eb7dbe9b31a45f57f2e1623aa1c9ce84a56c4d1 Parents: dd07038 Author: Arun Suresh <asur...@apache.org> Authored: Thu Nov 9 15:15:51 2017 -0800 Committer: Arun Suresh <asur...@apache.org> Committed: Thu Nov 9 15:15:51 2017 -0800 ---------------------------------------------------------------------- .../org/apache/hadoop/conf/Configuration.java | 163 ++++++++++++++----- .../mapreduce/v2/hs/HistoryFileManager.java | 2 +- 2 files changed, 122 insertions(+), 43 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/5eb7dbe9/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java index f94eba6..dfbeec7 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java @@ -18,6 +18,7 @@ package org.apache.hadoop.conf; +import com.ctc.wstx.api.ReaderConfig; import com.ctc.wstx.io.StreamBootstrapper; import com.ctc.wstx.io.SystemId; import com.ctc.wstx.stax.WstxInputFactory; @@ -70,6 +71,7 @@ import java.util.concurrent.atomic.AtomicReference; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; +import javax.xml.stream.XMLInputFactory; import javax.xml.stream.XMLStreamConstants; import javax.xml.stream.XMLStreamException; import javax.xml.stream.XMLStreamReader; @@ -91,6 +93,7 @@ import org.apache.hadoop.fs.CommonConfigurationKeys; import org.apache.hadoop.io.Writable; import org.apache.hadoop.io.WritableUtils; import org.apache.hadoop.net.NetUtils; +import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.security.alias.CredentialProvider; import org.apache.hadoop.security.alias.CredentialProvider.CredentialEntry; import org.apache.hadoop.security.alias.CredentialProviderFactory; @@ -206,19 +209,31 @@ public class Configuration implements Iterable<Map.Entry<String,String>>, private static final String DEFAULT_STRING_CHECK = "testingforemptydefaultvalue"; + private static boolean restrictSystemPropsDefault = false; + private boolean restrictSystemProps = restrictSystemPropsDefault; private boolean allowNullValueProperties = false; private static class Resource { private final Object resource; private final String name; + private final boolean restrictParser; public Resource(Object resource) { this(resource, resource.toString()); } - + + public Resource(Object resource, boolean useRestrictedParser) { + this(resource, resource.toString(), useRestrictedParser); + } + public Resource(Object resource, String name) { + this(resource, name, getRestrictParserDefault(resource)); + } + + public Resource(Object resource, String name, boolean restrictParser) { this.resource = resource; this.name = name; + this.restrictParser = restrictParser; } public String getName(){ @@ -228,11 +243,28 @@ public class Configuration implements Iterable<Map.Entry<String,String>>, public Object getResource() { return resource; } - + + public boolean isParserRestricted() { + return restrictParser; + } + @Override public String toString() { return name; } + + private static boolean getRestrictParserDefault(Object resource) { + if (resource instanceof String) { + return false; + } + UserGroupInformation user; + try { + user = UserGroupInformation.getCurrentUser(); + } catch (IOException e) { + throw new RuntimeException("Unable to determine current user", e); + } + return user.getRealUser() != null; + } } /** @@ -254,7 +286,7 @@ public class Configuration implements Iterable<Map.Entry<String,String>>, new ConcurrentHashMap<String, Boolean>()); private boolean loadDefaults = true; - + /** * Configuration objects */ @@ -777,6 +809,7 @@ public class Configuration implements Iterable<Map.Entry<String,String>>, this.overlay = (Properties)other.overlay.clone(); } + this.restrictSystemProps = other.restrictSystemProps; if (other.updatingResource != null) { this.updatingResource = new ConcurrentHashMap<String, String[]>( other.updatingResource); @@ -825,6 +858,14 @@ public class Configuration implements Iterable<Map.Entry<String,String>>, } } + public static void setRestrictSystemPropertiesDefault(boolean val) { + restrictSystemPropsDefault = val; + } + + public void setRestrictSystemProperties(boolean val) { + this.restrictSystemProps = val; + } + /** * Add a configuration resource. * @@ -838,6 +879,10 @@ public class Configuration implements Iterable<Map.Entry<String,String>>, addResourceObject(new Resource(name)); } + public void addResource(String name, boolean restrictedParser) { + addResourceObject(new Resource(name, restrictedParser)); + } + /** * Add a configuration resource. * @@ -852,6 +897,10 @@ public class Configuration implements Iterable<Map.Entry<String,String>>, addResourceObject(new Resource(url)); } + public void addResource(URL url, boolean restrictedParser) { + addResourceObject(new Resource(url, restrictedParser)); + } + /** * Add a configuration resource. * @@ -866,6 +915,10 @@ public class Configuration implements Iterable<Map.Entry<String,String>>, addResourceObject(new Resource(file)); } + public void addResource(Path file, boolean restrictedParser) { + addResourceObject(new Resource(file, restrictedParser)); + } + /** * Add a configuration resource. * @@ -883,6 +936,10 @@ public class Configuration implements Iterable<Map.Entry<String,String>>, addResourceObject(new Resource(in)); } + public void addResource(InputStream in, boolean restrictedParser) { + addResourceObject(new Resource(in, restrictedParser)); + } + /** * Add a configuration resource. * @@ -896,7 +953,12 @@ public class Configuration implements Iterable<Map.Entry<String,String>>, public void addResource(InputStream in, String name) { addResourceObject(new Resource(in, name)); } - + + public void addResource(InputStream in, String name, + boolean restrictedParser) { + addResourceObject(new Resource(in, name, restrictedParser)); + } + /** * Add a configuration resource. * @@ -906,7 +968,7 @@ public class Configuration implements Iterable<Map.Entry<String,String>>, * @param conf Configuration object from which to load properties */ public void addResource(Configuration conf) { - addResourceObject(new Resource(conf.getProps())); + addResourceObject(new Resource(conf.getProps(), conf.restrictSystemProps)); } @@ -926,6 +988,7 @@ public class Configuration implements Iterable<Map.Entry<String,String>>, private synchronized void addResourceObject(Resource resource) { resources.add(resource); // add to resources + restrictSystemProps |= resource.isParserRestricted(); reloadConfiguration(); } @@ -1034,34 +1097,36 @@ public class Configuration implements Iterable<Map.Entry<String,String>>, final String var = eval.substring(varBounds[SUB_START_IDX], varBounds[SUB_END_IDX]); String val = null; - try { - if (var.startsWith("env.") && 4 < var.length()) { - String v = var.substring(4); - int i = 0; - for (; i < v.length(); i++) { - char c = v.charAt(i); - if (c == ':' && i < v.length() - 1 && v.charAt(i + 1) == '-') { - val = getenv(v.substring(0, i)); - if (val == null || val.length() == 0) { - val = v.substring(i + 2); - } - break; - } else if (c == '-') { - val = getenv(v.substring(0, i)); - if (val == null) { - val = v.substring(i + 1); + if (!restrictSystemProps) { + try { + if (var.startsWith("env.") && 4 < var.length()) { + String v = var.substring(4); + int i = 0; + for (; i < v.length(); i++) { + char c = v.charAt(i); + if (c == ':' && i < v.length() - 1 && v.charAt(i + 1) == '-') { + val = getenv(v.substring(0, i)); + if (val == null || val.length() == 0) { + val = v.substring(i + 2); + } + break; + } else if (c == '-') { + val = getenv(v.substring(0, i)); + if (val == null) { + val = v.substring(i + 1); + } + break; } - break; } + if (i == v.length()) { + val = getenv(v); + } + } else { + val = getProperty(var); } - if (i == v.length()) { - val = getenv(v); - } - } else { - val = getProperty(var); + } catch (SecurityException se) { + LOG.warn("Unexpected SecurityException in Configuration", se); } - } catch(SecurityException se) { - LOG.warn("Unexpected SecurityException in Configuration", se); } if (val == null) { val = getRaw(var); @@ -1128,6 +1193,10 @@ public class Configuration implements Iterable<Map.Entry<String,String>>, this.allowNullValueProperties = val; } + public void setRestrictSystemProps(boolean val) { + this.restrictSystemProps = val; + } + /** * Return existence of the <code>name</code> property, but only for * names which have no valid value, usually non-existent or commented @@ -2718,7 +2787,7 @@ public class Configuration implements Iterable<Map.Entry<String,String>>, return configMap; } - private XMLStreamReader parse(URL url) + private XMLStreamReader parse(URL url, boolean restricted) throws IOException, XMLStreamException { if (!quietmode) { if (LOG.isDebugEnabled()) { @@ -2735,11 +2804,11 @@ public class Configuration implements Iterable<Map.Entry<String,String>>, // with other users. connection.setUseCaches(false); } - return parse(connection.getInputStream(), url.toString()); + return parse(connection.getInputStream(), url.toString(), restricted); } - private XMLStreamReader parse(InputStream is, String systemIdStr) - throws IOException, XMLStreamException { + private XMLStreamReader parse(InputStream is, String systemIdStr, + boolean restricted) throws IOException, XMLStreamException { if (!quietmode) { LOG.debug("parsing input stream " + is); } @@ -2747,9 +2816,12 @@ public class Configuration implements Iterable<Map.Entry<String,String>>, return null; } SystemId systemId = SystemId.construct(systemIdStr); - return XML_INPUT_FACTORY.createSR(XML_INPUT_FACTORY.createPrivateConfig(), - systemId, StreamBootstrapper.getInstance(null, systemId, is), false, - true); + ReaderConfig readerConfig = XML_INPUT_FACTORY.createPrivateConfig(); + if (restricted) { + readerConfig.setProperty(XMLInputFactory.SUPPORT_DTD, false); + } + return XML_INPUT_FACTORY.createSR(readerConfig, systemId, + StreamBootstrapper.getInstance(null, systemId, is), false, true); } private void loadResources(Properties properties, @@ -2757,7 +2829,7 @@ public class Configuration implements Iterable<Map.Entry<String,String>>, boolean quiet) { if(loadDefaults) { for (String resource : defaultResources) { - loadResource(properties, new Resource(resource), quiet); + loadResource(properties, new Resource(resource, false), quiet); } } @@ -2777,12 +2849,13 @@ public class Configuration implements Iterable<Map.Entry<String,String>>, name = wrapper.getName(); XMLStreamReader2 reader = null; boolean returnCachedProperties = false; + boolean isRestricted = wrapper.isParserRestricted(); if (resource instanceof URL) { // an URL resource - reader = (XMLStreamReader2)parse((URL)resource); + reader = (XMLStreamReader2)parse((URL)resource, isRestricted); } else if (resource instanceof String) { // a CLASSPATH resource URL url = getResource((String)resource); - reader = (XMLStreamReader2)parse(url); + reader = (XMLStreamReader2)parse(url, isRestricted); } else if (resource instanceof Path) { // a file resource // Can't use FileSystem API or we get an infinite loop // since FileSystem uses Configuration API. Use java.io.File instead. @@ -2793,10 +2866,12 @@ public class Configuration implements Iterable<Map.Entry<String,String>>, LOG.debug("parsing File " + file); } reader = (XMLStreamReader2)parse(new BufferedInputStream( - new FileInputStream(file)), ((Path)resource).toString()); + new FileInputStream(file)), ((Path)resource).toString(), + isRestricted); } } else if (resource instanceof InputStream) { - reader = (XMLStreamReader2)parse((InputStream)resource, null); + reader = (XMLStreamReader2)parse((InputStream)resource, null, + isRestricted); returnCachedProperties = true; } else if (resource instanceof Properties) { overlay(properties, (Properties)resource); @@ -2878,6 +2953,10 @@ public class Configuration implements Iterable<Map.Entry<String,String>>, if (confInclude == null) { break; } + if (isRestricted) { + throw new RuntimeException("Error parsing resource " + wrapper + + ": XInclude is not supported for restricted resources"); + } // Determine if the included resource is a classpath resource // otherwise fallback to a file resource // xi:include are treated as inline and retain current source @@ -2992,7 +3071,7 @@ public class Configuration implements Iterable<Map.Entry<String,String>>, if (returnCachedProperties) { overlay(properties, toAddTo); - return new Resource(toAddTo, name); + return new Resource(toAddTo, name, wrapper.isParserRestricted()); } return null; } catch (IOException e) { http://git-wip-us.apache.org/repos/asf/hadoop/blob/5eb7dbe9/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/HistoryFileManager.java ---------------------------------------------------------------------- diff --git a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/HistoryFileManager.java b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/HistoryFileManager.java index 61c750e..b418db7 100644 --- a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/HistoryFileManager.java +++ b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/HistoryFileManager.java @@ -508,7 +508,7 @@ public class HistoryFileManager extends AbstractService { public synchronized Configuration loadConfFile() throws IOException { FileContext fc = FileContext.getFileContext(confFile.toUri(), conf); Configuration jobConf = new Configuration(false); - jobConf.addResource(fc.open(confFile), confFile.toString()); + jobConf.addResource(fc.open(confFile), confFile.toString(), true); return jobConf; } --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org
