You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by st...@apache.org on 2021/06/08 21:11:50 UTC
[hadoop] 01/02: HADOOP-17631. Configuration ${env.VAR:-FALLBACK} to
eval FALLBACK when restrictSystemProps=true (#2977)
This is an automated email from the ASF dual-hosted git repository.
stevel pushed a commit to branch branch-3.3
in repository https://gitbox.apache.org/repos/asf/hadoop.git
commit 4ac9123619e043414c7f2a31f5919bd616fe4209
Author: Steve Loughran <st...@cloudera.com>
AuthorDate: Tue Jun 8 21:56:40 2021 +0100
HADOOP-17631. Configuration ${env.VAR:-FALLBACK} to eval FALLBACK when restrictSystemProps=true (#2977)
Contributed by Steve Loughran.
Change-Id: I9b82109eddeb659c01896152cf603d458e2a04cd
---
.../java/org/apache/hadoop/conf/Configuration.java | 79 ++++++++++++++--------
.../org/apache/hadoop/conf/TestConfiguration.java | 56 +++++++++++++++
2 files changed, 106 insertions(+), 29 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 aedde6b..364521c 100755
--- 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
@@ -1139,36 +1139,37 @@ 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;
- if (!restrictSystemProps) {
- try {
- if (var.startsWith("env.") && 4 < var.length()) {
- String v = var.substring(4);
- int i = 0;
- for (; i < v.length(); i++) {
- char c = v.charAt(i);
- if (c == ':' && i < v.length() - 1 && v.charAt(i + 1) == '-') {
- val = getenv(v.substring(0, i));
- if (val == null || val.length() == 0) {
- val = v.substring(i + 2);
- }
- break;
- } else if (c == '-') {
- val = getenv(v.substring(0, i));
- if (val == null) {
- val = v.substring(i + 1);
- }
- break;
+ try {
+ // evaluate system properties or environment variables even when
+ // the configuration is restricted -the restrictions are enforced
+ // in the getenv/getProperty calls
+ if (var.startsWith("env.") && 4 < var.length()) {
+ String v = var.substring(4);
+ int i = 0;
+ for (; i < v.length(); i++) {
+ char c = v.charAt(i);
+ if (c == ':' && i < v.length() - 1 && v.charAt(i + 1) == '-') {
+ val = getenv(v.substring(0, i));
+ if (val == null || val.length() == 0) {
+ val = v.substring(i + 2);
}
+ break;
+ } else if (c == '-') {
+ val = getenv(v.substring(0, i));
+ if (val == null) {
+ val = v.substring(i + 1);
+ }
+ break;
}
- if (i == v.length()) {
- val = getenv(v);
- }
- } else {
- val = getProperty(var);
}
- } catch (SecurityException se) {
- LOG.warn("Unexpected SecurityException in Configuration", se);
+ if (i == v.length()) {
+ val = getenv(v);
+ }
+ } else {
+ val = getProperty(var);
}
+ } catch (SecurityException se) {
+ LOG.warn("Unexpected SecurityException in Configuration", se);
}
if (val == null) {
val = getRaw(var);
@@ -1194,13 +1195,33 @@ public class Configuration implements Iterable<Map.Entry<String,String>>,
throw new IllegalStateException("Variable substitution depth too large: "
+ MAX_SUBST + " " + expr);
}
-
+
+ /**
+ * Get the environment variable value if
+ * {@link #restrictSystemProps} does not block this.
+ * @param name environment variable name.
+ * @return the value or null if either it is unset or access forbidden.
+ */
String getenv(String name) {
- return System.getenv(name);
+ if (!restrictSystemProps) {
+ return System.getenv(name);
+ } else {
+ return null;
+ }
}
+ /**
+ * Get a system property value if
+ * {@link #restrictSystemProps} does not block this.
+ * @param key property key
+ * @return the value or null if either it is unset or access forbidden.
+ */
String getProperty(String key) {
- return System.getProperty(key);
+ if (!restrictSystemProps) {
+ return System.getProperty(key);
+ } else {
+ return null;
+ }
}
/**
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 a2273ef..263e40c 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
@@ -490,6 +490,62 @@ public class TestConfiguration {
}
}
+ /**
+ * Verify that when a configuration is restricted, environment
+ * variables and system properties will be unresolved.
+ * The fallback patterns for the variables are still parsed.
+ */
+ @Test
+ public void testRestrictedEnv() throws IOException {
+ // this test relies on env.PATH being set on all platforms a
+ // test run will take place on, and the java.version sysprop
+ // set in all JVMs.
+ // Restricted configurations will not get access to these values, so
+ // will either be unresolved or, for env vars with fallbacks: the fallback
+ // values.
+
+ conf.setRestrictSystemProperties(true);
+
+ out = new BufferedWriter(new FileWriter(CONFIG));
+ startConfig();
+ // a simple property to reference
+ declareProperty("d", "D", "D");
+
+ // system property evaluation stops working completely
+ declareProperty("system1", "${java.version}", "${java.version}");
+
+ // the env variable does not resolve
+ declareProperty("secret1", "${env.PATH}", "${env.PATH}");
+
+ // but all the fallback options do work
+ declareProperty("secret2", "${env.PATH-a}", "a");
+ declareProperty("secret3", "${env.PATH:-b}", "b");
+ declareProperty("secret4", "${env.PATH:-}", "");
+ declareProperty("secret5", "${env.PATH-}", "");
+ // special case
+ declareProperty("secret6", "${env.PATH:}", "${env.PATH:}");
+ // safety check
+ declareProperty("secret7", "${env.PATH:--}", "-");
+
+ // recursive eval of the fallback
+ declareProperty("secret8", "${env.PATH:-${d}}", "D");
+
+ // if the fallback doesn't resolve, the result is the whole variable raw.
+ declareProperty("secret9", "${env.PATH:-$d}}", "${env.PATH:-$d}}");
+
+ endConfig();
+ Path fileResource = new Path(CONFIG);
+ conf.addResource(fileResource);
+
+ for (Prop p : props) {
+ System.out.println("p=" + p.name);
+ String gotVal = conf.get(p.name);
+ String gotRawVal = conf.getRaw(p.name);
+ assertEq(p.val, gotRawVal);
+ assertEq(p.expectEval, gotVal);
+ }
+ }
+
@Test
public void testFinalParam() throws IOException {
out=new BufferedWriter(new FileWriter(CONFIG));
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org