From 7af9b8ad1e993ef791aa38740b6aabc4c233a30f Mon Sep 17 00:00:00 2001 From: Arun Suresh Date: Thu, 9 Nov 2017 15:10:21 -0800 Subject: [PATCH] Fixing Job History Server Configuration parsing. (Jason Lowe via asuresh) --- .../org/apache/hadoop/conf/Configuration.java | 123 ++++++++++++++---- .../mapreduce/v2/hs/HistoryFileManager.java | 2 +- 2 files changed, 102 insertions(+), 23 deletions(-) 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 17d0dccebe2..253257080c3 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; @@ -68,6 +69,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; @@ -88,6 +90,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; @@ -196,19 +199,31 @@ public class Configuration implements Iterable>, 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(){ @@ -218,11 +233,28 @@ public class Configuration implements Iterable>, 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; + } } /** @@ -244,7 +276,7 @@ public class Configuration implements Iterable>, new ConcurrentHashMap()); private boolean loadDefaults = true; - + /** * Configuration objects */ @@ -752,6 +784,7 @@ public class Configuration implements Iterable>, this.overlay = (Properties)other.overlay.clone(); } + this.restrictSystemProps = other.restrictSystemProps; if (other.updatingResource != null) { this.updatingResource = new ConcurrentHashMap( other.updatingResource); @@ -798,6 +831,14 @@ public class Configuration implements Iterable>, } } + public static void setRestrictSystemPropertiesDefault(boolean val) { + restrictSystemPropsDefault = val; + } + + public void setRestrictSystemProperties(boolean val) { + this.restrictSystemProps = val; + } + /** * Add a configuration resource. * @@ -811,6 +852,10 @@ public class Configuration implements Iterable>, addResourceObject(new Resource(name)); } + public void addResource(String name, boolean restrictedParser) { + addResourceObject(new Resource(name, restrictedParser)); + } + /** * Add a configuration resource. * @@ -825,6 +870,10 @@ public class Configuration implements Iterable>, addResourceObject(new Resource(url)); } + public void addResource(URL url, boolean restrictedParser) { + addResourceObject(new Resource(url, restrictedParser)); + } + /** * Add a configuration resource. * @@ -839,6 +888,10 @@ public class Configuration implements Iterable>, addResourceObject(new Resource(file)); } + public void addResource(Path file, boolean restrictedParser) { + addResourceObject(new Resource(file, restrictedParser)); + } + /** * Add a configuration resource. * @@ -856,6 +909,10 @@ public class Configuration implements Iterable>, addResourceObject(new Resource(in)); } + public void addResource(InputStream in, boolean restrictedParser) { + addResourceObject(new Resource(in, restrictedParser)); + } + /** * Add a configuration resource. * @@ -869,7 +926,12 @@ public class Configuration implements Iterable>, 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. * @@ -879,7 +941,7 @@ public class Configuration implements Iterable>, * @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)); } @@ -899,6 +961,7 @@ public class Configuration implements Iterable>, private synchronized void addResourceObject(Resource resource) { resources.add(resource); // add to resources + restrictSystemProps |= resource.isParserRestricted(); reloadConfiguration(); } @@ -997,10 +1060,12 @@ public class Configuration implements Iterable>, final String var = eval.substring(varBounds[SUB_START_IDX], varBounds[SUB_END_IDX]); String val = null; - try { - val = System.getProperty(var); - } catch(SecurityException se) { - LOG.warn("Unexpected SecurityException in Configuration", se); + if (!restrictSystemProps) { + try { + val = System.getProperty(var); + } catch (SecurityException se) { + LOG.warn("Unexpected SecurityException in Configuration", se); + } } if (val == null) { val = getRaw(var); @@ -1051,6 +1116,10 @@ public class Configuration implements Iterable>, this.allowNullValueProperties = val; } + public void setRestrictSystemProps(boolean val) { + this.restrictSystemProps = val; + } + /** * Return existence of the name property, but only for * names which have no valid value, usually non-existent or commented @@ -2614,7 +2683,7 @@ public class Configuration implements Iterable>, return configMap; } - private XMLStreamReader parse(URL url) + private XMLStreamReader parse(URL url, boolean restricted) throws IOException, XMLStreamException { if (!quietmode) { if (LOG.isDebugEnabled()) { @@ -2631,11 +2700,11 @@ public class Configuration implements Iterable>, // 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); } @@ -2643,9 +2712,12 @@ public class Configuration implements Iterable>, 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, @@ -2653,7 +2725,7 @@ public class Configuration implements Iterable>, boolean quiet) { if(loadDefaults) { for (String resource : defaultResources) { - loadResource(properties, new Resource(resource), quiet); + loadResource(properties, new Resource(resource, false), quiet); } } @@ -2673,12 +2745,13 @@ public class Configuration implements Iterable>, 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. @@ -2689,10 +2762,12 @@ public class Configuration implements Iterable>, 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); @@ -2768,6 +2843,10 @@ public class Configuration implements Iterable>, 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 @@ -2872,7 +2951,7 @@ public class Configuration implements Iterable>, if (returnCachedProperties) { overlay(properties, toAddTo); - return new Resource(toAddTo, name); + return new Resource(toAddTo, name, wrapper.isParserRestricted()); } return null; } catch (IOException e) { 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 0789c35d016..045f0ee4909 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 @@ -511,7 +511,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; }