From 6f2a41e37d0b36cdafcfff75125165f212c612a6 Mon Sep 17 00:00:00 2001 From: Gera Shegalov Date: Tue, 30 Jun 2015 14:44:48 -0700 Subject: [PATCH] YARN-3768. ArrayIndexOutOfBoundsException with empty environment variables. (Zhihai Xu via gera) --- .../java/org/apache/hadoop/util/Shell.java | 6 +- hadoop-yarn-project/CHANGES.txt | 3 + .../org/apache/hadoop/yarn/util/Apps.java | 19 ++++-- .../org/apache/hadoop/yarn/util/TestApps.java | 61 +++++++++++++++++++ 4 files changed, 81 insertions(+), 8 deletions(-) create mode 100644 hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/TestApps.java diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java index 45c15885e61..ed83e8d1e5e 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java @@ -221,10 +221,12 @@ abstract public class Shell { new String[] { "kill", "-" + code, isSetsidAvailable ? "-" + pid : pid }; } + public static final String ENV_NAME_REGEX = "[A-Za-z_][A-Za-z0-9_]*"; /** Return a regular expression string that match environment variables */ public static String getEnvironmentVariableRegex() { - return (WINDOWS) ? "%([A-Za-z_][A-Za-z0-9_]*?)%" : - "\\$([A-Za-z_][A-Za-z0-9_]*)"; + return (WINDOWS) + ? "%(" + ENV_NAME_REGEX + "?)%" + : "\\$(" + ENV_NAME_REGEX + ")"; } /** diff --git a/hadoop-yarn-project/CHANGES.txt b/hadoop-yarn-project/CHANGES.txt index 8461f69aaaa..94bc8fcbacf 100644 --- a/hadoop-yarn-project/CHANGES.txt +++ b/hadoop-yarn-project/CHANGES.txt @@ -574,6 +574,9 @@ Release 2.8.0 - UNRELEASED YARN-3770. SerializedException should also handle java.lang.Error on de-serialization. (Lavkesh Lahngir via jianhe) + YARN-3768. ArrayIndexOutOfBoundsException with empty environment variables. + (Zhihai Xu via gera) + Release 2.7.2 - UNRELEASED INCOMPATIBLE CHANGES diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/Apps.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/Apps.java index cf3940fd9c7..9235e7dcaa8 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/Apps.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/Apps.java @@ -44,6 +44,14 @@ import org.apache.hadoop.yarn.exceptions.YarnRuntimeException; public class Apps { public static final String APP = "application"; public static final String ID = "ID"; + private static final Pattern VAR_SUBBER = + Pattern.compile(Shell.getEnvironmentVariableRegex()); + private static final Pattern VARVAL_SPLITTER = Pattern.compile( + "(?<=^|,)" // preceded by ',' or line begin + + '(' + Shell.ENV_NAME_REGEX + ')' // var group + + '=' + + "([^,]*)" // val group + ); public static ApplicationId toAppID(String aid) { Iterator it = _split(aid).iterator(); @@ -73,11 +81,10 @@ public class Apps { public static void setEnvFromInputString(Map env, String envString, String classPathSeparator) { if (envString != null && envString.length() > 0) { - String childEnvs[] = envString.split(","); - Pattern p = Pattern.compile(Shell.getEnvironmentVariableRegex()); - for (String cEnv : childEnvs) { - String[] parts = cEnv.split("="); // split on '=' - Matcher m = p.matcher(parts[1]); + Matcher varValMatcher = VARVAL_SPLITTER.matcher(envString); + while (varValMatcher.find()) { + String envVar = varValMatcher.group(1); + Matcher m = VAR_SUBBER.matcher(varValMatcher.group(2)); StringBuffer sb = new StringBuffer(); while (m.find()) { String var = m.group(1); @@ -93,7 +100,7 @@ public class Apps { m.appendReplacement(sb, Matcher.quoteReplacement(replace)); } m.appendTail(sb); - addToEnvironment(env, parts[0], sb.toString(), classPathSeparator); + addToEnvironment(env, envVar, sb.toString(), classPathSeparator); } } } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/TestApps.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/TestApps.java new file mode 100644 index 00000000000..66d2d231344 --- /dev/null +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/TestApps.java @@ -0,0 +1,61 @@ +/** +* Licensed to the Apache Software Foundation (ASF) under one +* or more contributor license agreements. See the NOTICE file +* distributed with this work for additional information +* regarding copyright ownership. The ASF licenses this file +* to you under the Apache License, Version 2.0 (the +* "License"); you may not use this file except in compliance +* with the License. You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ +package org.apache.hadoop.yarn.util; + +import org.apache.hadoop.util.Shell; +import org.junit.Test; + +import java.io.File; +import java.util.HashMap; +import java.util.Map; + +import static org.junit.Assert.assertEquals; + +public class TestApps { + @Test + public void testSetEnvFromInputString() { + Map environment = new HashMap(); + environment.put("JAVA_HOME", "/path/jdk"); + String goodEnv = "a1=1,b_2=2,_c=3,d=4,e=,f_win=%JAVA_HOME%" + + ",g_nix=$JAVA_HOME"; + Apps.setEnvFromInputString(environment, goodEnv, File.pathSeparator); + assertEquals("1", environment.get("a1")); + assertEquals("2", environment.get("b_2")); + assertEquals("3", environment.get("_c")); + assertEquals("4", environment.get("d")); + assertEquals("", environment.get("e")); + if (Shell.WINDOWS) { + assertEquals("$JAVA_HOME", environment.get("g_nix")); + assertEquals("/path/jdk", environment.get("f_win")); + } else { + assertEquals("/path/jdk", environment.get("g_nix")); + assertEquals("%JAVA_HOME%", environment.get("f_win")); + } + String badEnv = "1,,2=a=b,3=a=,4==,5==a,==,c-3=3,="; + environment.clear(); + Apps.setEnvFromInputString(environment, badEnv, File.pathSeparator); + assertEquals(environment.size(), 0); + + // Test "=" in the value part + environment.clear(); + Apps.setEnvFromInputString(environment, "b1,e1==,e2=a1=a2,b2", + File.pathSeparator); + assertEquals("=", environment.get("e1")); + assertEquals("a1=a2", environment.get("e2")); + } +}