YARN-5246. NMWebAppFilter web redirects drop query parameters. Contributed by Varun Vasudev.

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/webapp/util/WebAppUtils.java

@@ -23,6 +23,7 @@ import java.io.IOException;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.UnknownHostException;
+import java.nio.charset.Charset;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -30,6 +31,7 @@ import java.util.List;
 import org.apache.hadoop.classification.InterfaceAudience.Private;
 import org.apache.hadoop.classification.InterfaceStability.Evolving;
 import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.http.HtmlQuoting;
 import org.apache.hadoop.http.HttpConfig.Policy;
 import org.apache.hadoop.http.HttpServer2;
 import org.apache.hadoop.net.NetUtils;
@@ -42,6 +44,10 @@ import org.apache.hadoop.yarn.util.ConverterUtils;
 import org.apache.hadoop.yarn.util.RMHAUtils;
 import org.apache.hadoop.yarn.webapp.BadRequestException;
 import org.apache.hadoop.yarn.webapp.NotFoundException;
+import org.apache.http.NameValuePair;
+import org.apache.http.client.utils.URLEncodedUtils;
+
+import javax.servlet.http.HttpServletRequest;
 
 @Private
 @Evolving
@@ -418,4 +424,49 @@ public class WebAppUtils {
   public static List<String> listSupportedLogContentType() {
     return Arrays.asList("text", "octet-stream");
   }
+
+  private static String getURLEncodedQueryString(HttpServletRequest request) {
+    String queryString = request.getQueryString();
+    if (queryString != null && !queryString.isEmpty()) {
+      String reqEncoding = request.getCharacterEncoding();
+      if (reqEncoding == null || reqEncoding.isEmpty()) {
+        reqEncoding = "ISO-8859-1";
+      }
+      Charset encoding = Charset.forName(reqEncoding);
+      List<NameValuePair> params = URLEncodedUtils.parse(queryString, encoding);
+      return URLEncodedUtils.format(params, encoding);
+    }
+    return null;
+  }
+
+  /**
+   * Get a HTML escaped uri with the query parameters of the request.
+   * @param request HttpServletRequest with the request details
+   * @return HTML escaped uri with the query paramters
+   */
+  public static String getHtmlEscapedURIWithQueryString(
+      HttpServletRequest request) {
+    String urlEncodedQueryString = getURLEncodedQueryString(request);
+    if (urlEncodedQueryString != null) {
+      return HtmlQuoting.quoteHtmlChars(
+          request.getRequestURI() + "?" + urlEncodedQueryString);
+    }
+    return HtmlQuoting.quoteHtmlChars(request.getRequestURI());
+  }
+
+  /**
+   * Add the query params from a HttpServletRequest to the target uri passed.
+   * @param request HttpServletRequest with the request details
+   * @param targetUri the uri to which the query params must be added
+   * @return URL encoded string containing the targetUri + "?" + query string
+   */
+  public static String appendQueryParams(HttpServletRequest request,
+      String targetUri) {
+    String ret = targetUri;
+    String urlEncodedQueryString = getURLEncodedQueryString(request);
+    if (urlEncodedQueryString != null) {
+      ret += "?" + urlEncodedQueryString;
+    }
+    return ret;
+  }
 }

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/webapp/util/TestWebAppUtils.java

@@ -39,6 +39,9 @@ import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Assert;
 import org.junit.Test;
+import org.mockito.Mockito;
+
+import javax.servlet.http.HttpServletRequest;
 
 public class TestWebAppUtils {
   private static final String RM1_NODE_ID = "rm1";
@@ -176,6 +179,45 @@ public class TestWebAppUtils {
     return conf;
   }
 
+  @Test
+  public void testAppendQueryParams() throws Exception {
+    HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
+    String targetUri = "/test/path";
+    Mockito.when(request.getCharacterEncoding()).thenReturn(null);
+    Map<String, String> paramResultMap = new HashMap<>();
+    paramResultMap.put("param1=x", targetUri + "?" + "param1=x");
+    paramResultMap
+        .put("param1=x&param2=y", targetUri + "?" + "param1=x&param2=y");
+    paramResultMap.put("param1=x&param2=y&param3=x+y",
+        targetUri + "?" + "param1=x&param2=y&param3=x+y");
+
+    for (Map.Entry<String, String> entry : paramResultMap.entrySet()) {
+      Mockito.when(request.getQueryString()).thenReturn(entry.getKey());
+      String uri = WebAppUtils.appendQueryParams(request, targetUri);
+      Assert.assertEquals(entry.getValue(), uri);
+    }
+  }
+
+  @Test
+  public void testGetHtmlEscapedURIWithQueryString() throws Exception {
+    HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
+    String targetUri = "/test/path";
+    Mockito.when(request.getCharacterEncoding()).thenReturn(null);
+    Mockito.when(request.getRequestURI()).thenReturn(targetUri);
+    Map<String, String> paramResultMap = new HashMap<>();
+    paramResultMap.put("param1=x", targetUri + "?" + "param1=x");
+    paramResultMap
+        .put("param1=x&param2=y", targetUri + "?" + "param1=x&amp;param2=y");
+    paramResultMap.put("param1=x&param2=y&param3=x+y",
+        targetUri + "?" + "param1=x&amp;param2=y&amp;param3=x+y");
+
+    for (Map.Entry<String, String> entry : paramResultMap.entrySet()) {
+      Mockito.when(request.getQueryString()).thenReturn(entry.getKey());
+      String uri = WebAppUtils.getHtmlEscapedURIWithQueryString(request);
+      Assert.assertEquals(entry.getValue(), uri);
+    }
+  }
+
   public class TestBuilder extends HttpServer2.Builder {
     public String keypass;
     public String keystorePassword;

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/NMWebAppFilter.java

@@ -38,6 +38,7 @@ import org.apache.hadoop.yarn.server.nodemanager.containermanager.application.Ap
 import org.apache.hadoop.yarn.webapp.Controller.RequestContext;
 import com.google.inject.Injector;
 import com.sun.jersey.guice.spi.container.servlet.GuiceContainer;
+import org.apache.hadoop.yarn.webapp.util.WebAppUtils;
 
 @Singleton
 public class NMWebAppFilter extends GuiceContainer{
@@ -58,8 +59,7 @@ public class NMWebAppFilter extends GuiceContainer{
   public void doFilter(HttpServletRequest request,
       HttpServletResponse response, FilterChain chain) throws IOException,
       ServletException {
-    String uri = HtmlQuoting.quoteHtmlChars(request.getRequestURI());
+    String redirectPath = containerLogPageRedirectPath(request);
-    String redirectPath = containerLogPageRedirectPath(uri);
     if (redirectPath != null) {
       String redirectMsg =
           "Redirecting to log server" + " : " + redirectPath;
@@ -72,7 +72,8 @@ public class NMWebAppFilter extends GuiceContainer{
     super.doFilter(request, response, chain);
   }
 
-  private String containerLogPageRedirectPath(String uri) {
+  private String containerLogPageRedirectPath(HttpServletRequest request) {
+    String uri = HtmlQuoting.quoteHtmlChars(request.getRequestURI());
     String redirectPath = null;
     if (!uri.contains("/ws/v1/node") && uri.contains("/containerlogs")) {
       String[] parts = uri.split("/");
@@ -105,7 +106,8 @@ public class NMWebAppFilter extends GuiceContainer{
             sb.append(containerIdStr);
             sb.append("/");
             sb.append(appOwner);
-            redirectPath = sb.toString();
+            redirectPath =
+                WebAppUtils.appendQueryParams(request, sb.toString());
           } else {
             injector.getInstance(RequestContext.class).set(
               ContainerLogsPage.REDIRECT_URL, "false");

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/RMWebAppFilter.java

@@ -25,8 +25,6 @@ import java.io.PrintWriter;
 import java.net.InetSocketAddress;
 import java.net.URI;
 import java.net.URISyntaxException;
-import java.nio.charset.Charset;
-import java.util.List;
 import java.util.Random;
 import java.util.Set;
 
@@ -50,8 +48,6 @@ import org.apache.hadoop.yarn.util.Apps;
 import org.apache.hadoop.yarn.util.ConverterUtils;
 import org.apache.hadoop.yarn.webapp.YarnWebParams;
 import org.apache.hadoop.yarn.webapp.util.WebAppUtils;
-import org.apache.http.NameValuePair;
-import org.apache.http.client.utils.URLEncodedUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -116,22 +112,10 @@ public class RMWebAppFilter extends GuiceContainer {
       htmlEscapedUri = "/";
     }
 
-    String uriWithQueryString = htmlEscapedUri;
+    String uriWithQueryString =
-    String htmlEscapedUriWithQueryString = htmlEscapedUri;
+        WebAppUtils.appendQueryParams(request, htmlEscapedUri);
-
+    String htmlEscapedUriWithQueryString =
-    String queryString = request.getQueryString();
+        WebAppUtils.getHtmlEscapedURIWithQueryString(request);
-    if (queryString != null && !queryString.isEmpty()) {
-      String reqEncoding = request.getCharacterEncoding();
-      if (reqEncoding == null || reqEncoding.isEmpty()) {
-        reqEncoding = "ISO-8859-1";
-      }
-      Charset encoding = Charset.forName(reqEncoding);
-      List<NameValuePair> params = URLEncodedUtils.parse(queryString, encoding);
-      String urlEncodedQueryString = URLEncodedUtils.format(params, encoding);
-      uriWithQueryString += "?" + urlEncodedQueryString;
-      htmlEscapedUriWithQueryString = HtmlQuoting.quoteHtmlChars(
-          request.getRequestURI() + "?" + urlEncodedQueryString);
-    }
 
     RMWebApp rmWebApp = injector.getInstance(RMWebApp.class);
     rmWebApp.checkIfStandbyRM();