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

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<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(){
@@ -218,11 +233,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;
+    }
   }
   
   /**
@@ -244,7 +276,7 @@ public class Configuration implements Iterable<Map.Entry<String,String>>,
       new ConcurrentHashMap<String, Boolean>());
   
   private boolean loadDefaults = true;
-  
+
   /**
    * Configuration objects
    */
@@ -752,6 +784,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);
@@ -798,6 +831,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. 
    * 
@@ -811,6 +852,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. 
    * 
@@ -825,6 +870,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. 
    * 
@@ -839,6 +888,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. 
    * 
@@ -856,6 +909,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. 
    * 
@@ -869,7 +926,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.
    *
@@ -879,7 +941,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));
   }
 
   
@@ -899,6 +961,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();
   }
 
@@ -997,10 +1060,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 {
+      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);
@@ -1051,6 +1116,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
@@ -2614,7 +2683,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()) {
@@ -2631,11 +2700,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)
+  private XMLStreamReader parse(InputStream is, String systemIdStr,
-      throws IOException, XMLStreamException {
+      boolean restricted) throws IOException, XMLStreamException {
     if (!quietmode) {
       LOG.debug("parsing input stream " + is);
     }
@@ -2643,9 +2712,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(),
+    ReaderConfig readerConfig = XML_INPUT_FACTORY.createPrivateConfig();
-        systemId, StreamBootstrapper.getInstance(null, systemId, is), false,
+    if (restricted) {
-        true);
+      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<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);
       }
     }
     
@@ -2673,12 +2745,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.
@@ -2689,10 +2762,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);
@@ -2768,6 +2843,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
@@ -2872,7 +2951,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

@@ -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;
     }