From 1ba9704eec22c75f8aec653ee15eb6767b5a7f4b Mon Sep 17 00:00:00 2001 From: Steve Loughran Date: Thu, 25 May 2017 14:59:33 +0100 Subject: [PATCH] HADOOP-14399. Configuration does not correctly XInclude absolute file URIs. Contributed by Jonathan Eagles --- .../org/apache/hadoop/conf/Configuration.java | 37 ++++++++++++------- .../apache/hadoop/conf/TestConfiguration.java | 23 ++++++++++-- 2 files changed, 42 insertions(+), 18 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 2ac52cb394..1a6679b185 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 @@ -2714,6 +2714,7 @@ private Resource loadResource(Properties properties, StringBuilder token = new StringBuilder(); String confName = null; String confValue = null; + String confInclude = null; boolean confFinal = false; boolean fallbackAllowed = false; boolean fallbackEntered = false; @@ -2757,7 +2758,7 @@ private Resource loadResource(Properties properties, break; case "include": // Determine href for xi:include - String confInclude = null; + confInclude = null; attrCount = reader.getAttributeCount(); for (int i = 0; i < attrCount; i++) { String attrName = reader.getAttributeLocalName(i); @@ -2776,18 +2777,25 @@ private Resource loadResource(Properties properties, Resource classpathResource = new Resource(include, name); loadResource(properties, classpathResource, quiet); } else { - File href = new File(confInclude); - if (!href.isAbsolute()) { - // Included resources are relative to the current resource - File baseFile = new File(name).getParentFile(); - href = new File(baseFile, href.getPath()); + URL url; + try { + url = new URL(confInclude); + url.openConnection().connect(); + } catch (IOException ioe) { + File href = new File(confInclude); + if (!href.isAbsolute()) { + // Included resources are relative to the current resource + File baseFile = new File(name).getParentFile(); + href = new File(baseFile, href.getPath()); + } + if (!href.exists()) { + // Resource errors are non-fatal iff there is 1 xi:fallback + fallbackAllowed = true; + break; + } + url = href.toURI().toURL(); } - if (!href.exists()) { - // Resource errors are non-fatal iff there is 1 xi:fallback - fallbackAllowed = true; - break; - } - Resource uriResource = new Resource(href.toURI().toURL(), name); + Resource uriResource = new Resource(url, name); loadResource(properties, uriResource, quiet); } break; @@ -2828,8 +2836,9 @@ private Resource loadResource(Properties properties, break; case "include": if (fallbackAllowed && !fallbackEntered) { - throw new IOException("Fetch fail on include with no " - + "fallback while loading '" + name + "'"); + throw new IOException("Fetch fail on include for '" + + confInclude + "' with no fallback while loading '" + + name + "'"); } fallbackAllowed = false; fallbackEntered = false; diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java index 6f4c26cf86..0c66470567 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java @@ -29,6 +29,7 @@ import java.io.StringWriter; import java.net.InetAddress; import java.net.InetSocketAddress; +import java.net.URI; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -61,6 +62,9 @@ public class TestConfiguration extends TestCase { final static String CONFIG = new File("./test-config-TestConfiguration.xml").getAbsolutePath(); final static String CONFIG2 = new File("./test-config2-TestConfiguration.xml").getAbsolutePath(); final static String CONFIG_FOR_ENUM = new File("./test-config-enum-TestConfiguration.xml").getAbsolutePath(); + final static String CONFIG_FOR_URI = "file://" + + new File("./test-config-uri-TestConfiguration.xml").getAbsolutePath(); + private static final String CONFIG_MULTI_BYTE = new File( "./test-config-multi-byte-TestConfiguration.xml").getAbsolutePath(); private static final String CONFIG_MULTI_BYTE_SAVED = new File( @@ -85,6 +89,7 @@ protected void tearDown() throws Exception { new File(CONFIG).delete(); new File(CONFIG2).delete(); new File(CONFIG_FOR_ENUM).delete(); + new File(new URI(CONFIG_FOR_URI)).delete(); new File(CONFIG_MULTI_BYTE).delete(); new File(CONFIG_MULTI_BYTE_SAVED).delete(); } @@ -516,13 +521,21 @@ public void testIncludes() throws Exception { appendProperty("a","b"); appendProperty("c","d"); endConfig(); + File fileUri = new File(new URI(CONFIG_FOR_URI)); + out=new BufferedWriter(new FileWriter(fileUri)); + startConfig(); + appendProperty("e", "f"); + appendProperty("g", "h"); + endConfig(); out=new BufferedWriter(new FileWriter(CONFIG)); startConfig(); startInclude(CONFIG2); endInclude(); - appendProperty("e","f"); - appendProperty("g","h"); + startInclude(CONFIG_FOR_URI); + endInclude(); + appendProperty("i", "j"); + appendProperty("k", "l"); endConfig(); // verify that the includes file contains all properties @@ -530,8 +543,10 @@ public void testIncludes() throws Exception { conf.addResource(fileResource); assertEquals(conf.get("a"), "b"); assertEquals(conf.get("c"), "d"); - assertEquals(conf.get("e"), "f"); - assertEquals(conf.get("g"), "h"); + assertEquals(conf.get("e"), "f"); + assertEquals(conf.get("g"), "h"); + assertEquals(conf.get("i"), "j"); + assertEquals(conf.get("k"), "l"); tearDown(); }