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

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

@@ -84,6 +84,7 @@
 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;
@@ -194,19 +195,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(){
@@ -216,11 +232,28 @@ public String getName(){
     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;
+    }
   }
   
   /**
@@ -242,7 +275,7 @@ public String toString() {
       new ConcurrentHashMap<String, Boolean>());
   
   private boolean loadDefaults = true;
-  
+
   /**
    * Configuration objects
    */
@@ -741,6 +774,7 @@ public Configuration(Configuration other) {
        this.overlay = (Properties)other.overlay.clone();
      }
 
+     this.restrictSystemProps = other.restrictSystemProps;
      this.updatingResource = new ConcurrentHashMap<String, String[]>(
          other.updatingResource);
      this.finalParameters = Collections.newSetFromMap(
@@ -785,6 +819,14 @@ public static synchronized void addDefaultResource(String name) {
     }
   }
 
+  public static void setRestrictSystemPropertiesDefault(boolean val) {
+    restrictSystemPropsDefault = val;
+  }
+
+  public void setRestrictSystemProperties(boolean val) {
+    this.restrictSystemProps = val;
+  }
+
   /**
    * Add a configuration resource. 
    * 
@@ -798,6 +840,10 @@ public void addResource(String name) {
     addResourceObject(new Resource(name));
   }
 
+  public void addResource(String name, boolean restrictedParser) {
+    addResourceObject(new Resource(name, restrictedParser));
+  }
+
   /**
    * Add a configuration resource. 
    * 
@@ -812,6 +858,10 @@ public void addResource(URL url) {
     addResourceObject(new Resource(url));
   }
 
+  public void addResource(URL url, boolean restrictedParser) {
+    addResourceObject(new Resource(url, restrictedParser));
+  }
+
   /**
    * Add a configuration resource. 
    * 
@@ -826,6 +876,10 @@ public void addResource(Path file) {
     addResourceObject(new Resource(file));
   }
 
+  public void addResource(Path file, boolean restrictedParser) {
+    addResourceObject(new Resource(file, restrictedParser));
+  }
+
   /**
    * Add a configuration resource. 
    * 
@@ -843,6 +897,10 @@ public void addResource(InputStream in) {
     addResourceObject(new Resource(in));
   }
 
+  public void addResource(InputStream in, boolean restrictedParser) {
+    addResourceObject(new Resource(in, restrictedParser));
+  }
+
   /**
    * Add a configuration resource. 
    * 
@@ -856,7 +914,12 @@ public void addResource(InputStream in) {
   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.
    *
@@ -886,6 +949,7 @@ public synchronized void reloadConfiguration() {
   
   private synchronized void addResourceObject(Resource resource) {
     resources.add(resource);                      // add to resources
+    restrictSystemProps |= resource.isParserRestricted();
     reloadConfiguration();
   }
 
@@ -984,10 +1048,12 @@ private String substituteVars(String expr) {
       final String var = eval.substring(varBounds[SUB_START_IDX],
           varBounds[SUB_END_IDX]);
       String val = null;
-      try {
+      if (!restrictSystemProps) {
-        val = System.getProperty(var);
+        try {
-      } catch(SecurityException se) {
+          val = System.getProperty(var);
-        LOG.warn("Unexpected SecurityException in Configuration", se);
+        } catch (SecurityException se) {
+          LOG.warn("Unexpected SecurityException in Configuration", se);
+        }
       }
       if (val == null) {
         val = getRaw(var);
@@ -1038,6 +1104,10 @@ public void setAllowNullValueProperties( boolean val ) {
     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
@@ -2623,12 +2693,12 @@ private void loadResources(Properties properties,
                              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);
       }
     }
     
@@ -2653,13 +2723,16 @@ private Resource loadResource(Properties properties, Resource wrapper, boolean q
 
       //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 "
+        LOG.error("Failed to set setXIncludeAware(" + useXInclude
-                + docBuilderFactory
+                + ") for parser " + docBuilderFactory, e);
-                + ":" + e,
+      }
-                e);
+      if (wrapper.isParserRestricted()) {
+        docBuilderFactory.setFeature(
+            "http://apache.org/xml/features/disallow-doctype-decl", true);
       }
       DocumentBuilder builder = docBuilderFactory.newDocumentBuilder();
       Document doc = null;
@@ -2715,11 +2788,19 @@ private Resource loadResource(Properties properties, Resource wrapper, boolean q
           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()))
+        if (!"property".equals(prop.getTagName())) {
-          LOG.warn("bad conf file: element not <property>");
+          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;
@@ -2765,7 +2846,7 @@ private Resource loadResource(Properties properties, Resource wrapper, boolean q
       
       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

@@ -495,7 +495,7 @@ public synchronized Path getConfFile() {
     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;
     }
   }