Fixing Job History Server Configuration parsing. (Jason Lowe via asuresh)

(cherry picked from commit b3df744fb651d480050cf355f7004e50e30f22ff)

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java

@@ -82,6 +82,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;
@@ -185,19 +186,34 @@ public class Configuration implements Iterable<Map.Entry<String,String>>,
   private static final String DEFAULT_STRING_CHECK =
     "testingforemptydefaultvalue";
 
+  private static final String XINCLUDE_NS_URI =
+      "http://www.w3.org/2001/XInclude";
+
+  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(){
@@ -207,11 +223,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;
+    }
   }
   
   /**
@@ -233,7 +266,7 @@ public class Configuration implements Iterable<Map.Entry<String,String>>,
       new ConcurrentHashMap<String, Boolean>());
   
   private boolean loadDefaults = true;
-  
+
   /**
    * Configuration objects
    */
@@ -710,6 +743,7 @@ public class Configuration implements Iterable<Map.Entry<String,String>>,
        this.overlay = (Properties)other.overlay.clone();
      }
 
+     this.restrictSystemProps = other.restrictSystemProps;
      this.updatingResource = new ConcurrentHashMap<String, String[]>(
          other.updatingResource);
      this.finalParameters = Collections.newSetFromMap(
@@ -741,6 +775,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. 
    * 
@@ -754,6 +796,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. 
    * 
@@ -768,6 +814,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. 
    * 
@@ -782,6 +832,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. 
    * 
@@ -799,6 +853,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. 
    * 
@@ -812,7 +870,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.
    *
@@ -842,6 +905,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();
   }
 
@@ -940,10 +1004,12 @@ 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 {
-        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);
@@ -994,6 +1060,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
@@ -2491,12 +2561,12 @@ 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);
       }
     
       //support the hadoop-site.xml as a deprecated case
       if(getResource("hadoop-site.xml")!=null) {
-        loadResource(properties, new Resource("hadoop-site.xml"), quiet);
+        loadResource(properties, new Resource("hadoop-site.xml", false), quiet);
       }
     }
     
@@ -2521,13 +2591,16 @@ public class Configuration implements Iterable<Map.Entry<String,String>>,
 
       //allow includes in the xml file
       docBuilderFactory.setNamespaceAware(true);
+      boolean useXInclude = !wrapper.isParserRestricted();
       try {
-          docBuilderFactory.setXIncludeAware(true);
+        docBuilderFactory.setXIncludeAware(useXInclude);
       } catch (UnsupportedOperationException e) {
-        LOG.error("Failed to set setXIncludeAware(true) for parser "
-                + docBuilderFactory
-                + ":" + e,
-                e);
+        LOG.error("Failed to set setXIncludeAware(" + useXInclude
+                + ") for parser " + docBuilderFactory, e);
+      }
+      if (wrapper.isParserRestricted()) {
+        docBuilderFactory.setFeature(
+            "http://apache.org/xml/features/disallow-doctype-decl", true);
       }
       DocumentBuilder builder = docBuilderFactory.newDocumentBuilder();
       Document doc = null;
@@ -2583,11 +2656,19 @@ public class Configuration implements Iterable<Map.Entry<String,String>>,
           continue;
         Element prop = (Element)propNode;
         if ("configuration".equals(prop.getTagName())) {
-          loadResource(toAddTo, new Resource(prop, name), quiet);
+          loadResource(toAddTo,
+              new Resource(prop, name, wrapper.isParserRestricted()), quiet);
           continue;
         }
-        if (!"property".equals(prop.getTagName()))
-          LOG.warn("bad conf file: element not <property>");
+        if (!"property".equals(prop.getTagName())) {
+          if (wrapper.isParserRestricted()
+              && XINCLUDE_NS_URI.equals(prop.getNamespaceURI())) {
+            throw new RuntimeException("Error parsing resource " + wrapper
+                + ": XInclude is not supported for restricted resources");
+          }
+          LOG.warn("Unexpected tag in conf file " + wrapper
+              + ": expected <property> but found <" + prop.getTagName() + ">");
+        }
         NodeList fields = prop.getChildNodes();
         String attr = null;
         String value = null;
@@ -2633,7 +2714,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) {

hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/HistoryFileManager.java

@@ -497,7 +497,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;
     }
   }